drivers/media/dvb/frontends/dib8000.c:1837 dib8000_tune() warn: variable dereferenced before check 'state' (see line 1834)
Hi Patrick,
FYI, the below commit triggered a smatch warning:
77e2c0f V4L/DVB (12900): DiB8000: added support for DiBcom ISDB-T/ISDB-Tsb
demodulator DiB8000
+ drivers/media/dvb/frontends/dib8000.c:1837 dib8000_tune() warn: variable
dereferenced before check 'state' (see line 1834)
vim +1837 drivers/media/dvb/frontends/dib8000.c
77e2c0f5 Patrick Boettcher 2009-08-17 1830 static int dib8000_tune(struct
dvb_frontend *fe)
77e2c0f5 Patrick Boettcher 2009-08-17 1831 {
77e2c0f5 Patrick Boettcher 2009-08-17 1832 struct dib8000_state *state =
fe-demodulator_priv;
77e2c0f5 Patrick Boettcher 2009-08-17 1833 int ret = 0;
77e2c0f5 Patrick Boettcher 2009-08-17 1834 u16 value, mode =
fft_to_mode(state);
77e2c0f5 Patrick Boettcher 2009-08-17 1835
77e2c0f5 Patrick Boettcher 2009-08-17 1836 // we are already tuned - just
resuming from suspend
77e2c0f5 Patrick Boettcher 2009-08-17 @1837 if (state == NULL)
77e2c0f5 Patrick Boettcher 2009-08-17 1838 return -EINVAL;
77e2c0f5 Patrick Boettcher 2009-08-17 1839
---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation
--
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_build broken by [PATCH RFC v3 5/5] m5mols: Implement .get_frame_desc subdev callback
On Sun October 7 2012 03:19:48 Michael West wrote: This patch changes versions.txt and disables VIDEO_M5MOLS which fixed the build for my 3.2 kernel but looking at the logs it looks like this is not the way to fix it as it's not just a 3.6+ problem as it does not build on 3.6 as well... So probably best to find why it doesn't build on the current kernel first. FYI, I'll be fixing this and other build problems in media_build on Monday. Regards, Hans --- v4l/versions.txt |2 ++ 1 file changed, 2 insertions(+) diff --git a/v4l/versions.txt b/v4l/versions.txt index 328651e..349695c 100644 --- a/v4l/versions.txt +++ b/v4l/versions.txt @@ -4,6 +4,8 @@ [3.6.0] # needs devm_clk_get, clk_enable, clk_disable VIDEO_CODA +# broken add reason here +VIDEO_M5MOLS [3.4.0] # needs devm_regulator_bulk_get On 10/06/2012 08:43 PM, Jan Hoogenraad wrote: Thanks. I see several drivers disabled for lower kernel versions in my Kconfig file. I am not sure how this is accomplished, but it would be helpful if the Fujitsu M-5MOLS 8MP sensor support is automatically disabled for kernel 3.6 I fixed it in my version by replacing SZ_1M by (1024*1024). I did not need the driver, but at least it compiled ... A patch for v4l/versions.txt is needed [1]. I'll see if I can prepare that. http://git.linuxtv.org/media_build.git/history/5d00dba6aaf0f91a742d90fd1e12e0fb2d36253e:/v4l/versions.txt -- 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: [PATCHv9 00/25] Integration of videobuf2 with DMABUF
Hi all!
On Tue October 2 2012 16:27:11 Tomasz Stanislawski wrote:
Hello everyone,
This patchset adds support for DMABUF [2] importing and exporting to V4L2
stack.
I see that there is a lot of interest to getting this into 3.7. I understand
that, but IMHO this is a bit too soon. I would like to see the review comments
I made for v9 addressed, and more importantly, I would *really* like to see
mem2mem_testdev.c be dmabuf aware. One of the problems today is that it is
hard to test this, but if vivi and mem2mem_testdev can do dmabuf, then it is
easy to setup a pipeline that everyone can use for testing and application
development without requiring special hardware.
My suggestion would be to get a v10 out, and merge that as soon as the for_v3.8
branch opens. Then get mem2mem_testdev ready for dmabuf as well for 3.8.
Regards,
Hans
v9:
- rebase on 3.6
- change type for fs to __s32
- add support for vb2_ioctl_expbuf
- remove patch 'v4l: vb2: add support for DMA_ATTR_NO_KERNEL_MAPPING',
it will be posted as a separate patch
- fix typos and style in Documentation (from Hans Verkuil)
- only vb2-core and vb2-dma-contig selects DMA_SHARED_BUFFER in Kconfig
- use data_offset and length while queueing DMABUF
- return the most recently used fd at VIDIOC_DQBUF
- use (buffer-type, index, plane) tuple instead of mem_offset
to identify a for exporting (from Hans Verkuil)
- support for DMABUF mmap in vb2-dma-contig (from Laurent Pinchart)
- add testing alignment of vaddr and size while verifying userptr
against DMA capabilities (from Laurent Pinchart)
- substitute VM_DONTDUMP with VM_RESERVED
- simplify error handling in vb2_dc_get_dmabuf (from Laurent Pinchart)
v8:
- rebased on 3.6-rc1
- merged importer and exporter patchsets
- fixed missing fields in v4l2_plane32 and v4l2_buffer32 structs
- fixed typos/style in documentation
- significant reduction of warnings from checkpatch.pl
- fixed STREAMOFF issues reported by Dima Zavin [4] by adding
__vb2_dqbuf helper to vb2-core
- DC fails if userptr is not correctly aligned
- add support for DMA attributes in DC
- add support for buffers with no kernel mapping
- add reference counting on device from allocator context
- dummy support for mmap
- use dma_get_sgtable, drop vb2_dc_kaddr_to_pages hack and
vb2_dc_get_base_sgt helper
v7:
- support for V4L2_MEMORY_DMABUF in v4l2-compact-ioctl32.c
- cosmetic fixes to the documentation
- added importing for vmalloc because vmap support in dmabuf for 3.5
was pull-requested
- support for dmabuf importing for VIVI
- resurrect allocation of dma-contig context
- remove reference of alloc_ctx in dma-contig buffer
- use sg_alloc_table_from_pages
- fix DMA scatterlist calls to use orig_nents instead of nents
- fix memleak in vb2_dc_sgt_foreach_page (use orig_nents instead of nents)
v6:
- fixed missing entry in v4l2_memory_names
- fixed a bug occuring after get_user_pages failure
- fixed a bug caused by using invalid vma for get_user_pages
- prepare/finish no longer call dma_sync for dmabuf buffers
v5:
- removed change of importer/exporter behaviour
- fixes vb2_dc_pages_to_sgt basing on Laurent's hints
- changed pin/unpin words to lock/unlock in Doc
v4:
- rebased on mainline 3.4-rc2
- included missing importing support for s5p-fimc and s5p-tv
- added patch for changing map/unmap for importers
- fixes to Documentation part
- coding style fixes
- pairing {map/unmap}_dmabuf in vb2-core
- fixing variable types and semantic of arguments in videobufb2-dma-contig.c
v3:
- rebased on mainline 3.4-rc1
- split 'code refactor' patch to multiple smaller patches
- squashed fixes to Sumit's patches
- patchset is no longer dependant on 'DMA mapping redesign'
- separated path for handling IO and non-IO mappings
- add documentation for DMABUF importing to V4L
- removed all DMABUF exporter related code
- removed usage of dma_get_pages extension
v2:
- extended VIDIOC_EXPBUF argument from integer memoffset to struct
v4l2_exportbuffer
- added patch that breaks DMABUF spec on (un)map_atachment callcacks but
allows
to work with existing implementation of DMABUF prime in DRM
- all dma-contig code refactoring patches were squashed
- bugfixes
v1: List of changes since [1].
- support for DMA api extension dma_get_pages, the function is used to
retrieve
pages used to create DMA mapping.
- small fixes/code cleanup to videobuf2
- added prepare and finish callbacks to vb2 allocators, it is used keep
consistency between dma-cpu acess to the memory (by Marek Szyprowski)
- support for exporting of DMABUF buffer in V4L2 and Videobuf2, originated
from
[3].
- support for dma-buf exporting in vb2-dma-contig allocator
- support for DMABUF for s5p-tv and s5p-fimc (capture interface) drivers,
originated from [3]
- changed handling for userptr buffers (by Marek Szyprowski, Andrzej
Pietrasiewicz)
- let mmap method to use
Re: Media_build broken by [PATCH RFC v3 5/5] m5mols: Implement .get_frame_desc subdev callback
On 10/07/2012 03:19 AM, Michael West wrote: This patch changes versions.txt and disables VIDEO_M5MOLS which fixed the build for my 3.2 kernel but looking at the logs it looks like this is not the way to fix it as it's not just a 3.6+ problem as it does not build on 3.6 as well... So probably best to find why it doesn't build on the current kernel first. To fix the build on kernels 3.6+ linux/sizes.h just needs to be inclcuded in m5mols.h. This is what my patch from previous message in this thread does. But this will break again on kernel versions _3.5 and lower_ where linux/sizes.h doesn't exist. I thought originally it could have been simply replaced there with asm/sizes.h, but not all architectures have it $ git grep #define SZ_1M v2.6.32 v2.6.32:arch/arm/include/asm/sizes.h:#define SZ_1M 0x0010 v2.6.32:arch/sh/include/asm/sizes.h:#define SZ_1M 0x0010 $ git grep #define SZ_1M v3.6-rc5 v3.6-rc5:drivers/base/dma-contiguous.c:#define SZ_1M (1 20) v3.6-rc5:include/linux/sizes.h:#define SZ_1M 0x0010 Let's just use the below patch to solve this build break, this way there is no need to touch anything at media_build. From 11adc6956f3fe87c897aa6add08f8437422969a8 Mon Sep 17 00:00:00 2001 From: Sylwester Nawrocki sylvester.nawro...@gmail.com Date: Sun, 7 Oct 2012 13:04:37 +0200 Subject: [PATCH] m5mols: Replace SZ_1M with explicit value SZ_1M macro definition was introduced in commit ab7ef22419927 [media] m5mols: Implement .get_frame_desc subdev callback but required linux/sizes.h header was not included. To prevent build errors with older kernels where linux/sizes.h doesn't exist use explicit value rather than SZ_1M. Reported-by: Jan Hoogenraad jan-conceptro...@hoogenraad.net Signed-off-by: Sylwester Nawrocki sylvester.nawro...@gmail.com --- drivers/media/i2c/m5mols/m5mols.h |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/i2c/m5mols/m5mols.h b/drivers/media/i2c/m5mols/m5mols.h index 4ab8b37..30654f5 100644 --- a/drivers/media/i2c/m5mols/m5mols.h +++ b/drivers/media/i2c/m5mols/m5mols.h @@ -24,7 +24,7 @@ * determined by CAPP_JPEG_SIZE_MAX register. */ #define M5MOLS_JPEG_TAGS_SIZE 0x2 -#define M5MOLS_MAIN_JPEG_SIZE_MAX (5 * SZ_1M) +#define M5MOLS_MAIN_JPEG_SIZE_MAX (5 * 1024 * 1024) extern int m5mols_debug; -- 1.7.4.1 --- v4l/versions.txt |2 ++ 1 file changed, 2 insertions(+) diff --git a/v4l/versions.txt b/v4l/versions.txt index 328651e..349695c 100644 --- a/v4l/versions.txt +++ b/v4l/versions.txt @@ -4,6 +4,8 @@ [3.6.0] # needs devm_clk_get, clk_enable, clk_disable VIDEO_CODA +# broken add reason here +VIDEO_M5MOLS This was supposed to be under [3.5.0]. [3.4.0] # needs devm_regulator_bulk_get -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] drivers/media: Remove unnecessary semicolon
Hi Peter, Em Thu, 27 Sep 2012 13:55:06 +0200 Peter Senna Tschudin peter.se...@gmail.com escreveu: Remove unnecessary semicolon And: drivers/media/dvb-frontends/stv0900_core.c: remove unnecessary whitespace before a quoted newline (answering about your v3 here, instead of at your v3 email as it got lost - I had subscribed from vger - you likely sent the patch during that period of time - fortunately, patchwork got it) This patch doesn't apply as-is: Applying patch patches/lmml_14721_v3_drivers_media_remove_unnecessary_semicolon.patch patching file drivers/media/dvb-core/dvb_frontend.c Hunk #1 succeeded at 2309 (offset 21 lines). patching file drivers/media/dvb-frontends/a8293.c patching file drivers/media/dvb-frontends/af9013.c patching file drivers/media/dvb-frontends/bcm3510.c patching file drivers/media/dvb-frontends/cx24110.c patching file drivers/media/dvb-frontends/drxd_hard.c patching file drivers/media/dvb-frontends/isl6405.c patching file drivers/media/dvb-frontends/isl6421.c patching file drivers/media/dvb-frontends/itd1000.c Hunk #1 FAILED at 231. 1 out of 1 hunk FAILED -- rejects in file drivers/media/dvb-frontends/itd1000.c patching file drivers/media/dvb-frontends/lnbp21.c patching file drivers/media/dvb-frontends/lnbp22.c patching file drivers/media/dvb-frontends/si21xx.c patching file drivers/media/dvb-frontends/sp8870.c Hunk #1 FAILED at 188. Hunk #2 FAILED at 207. Hunk #3 FAILED at 229. 3 out of 3 hunks FAILED -- rejects in file drivers/media/dvb-frontends/sp8870.c patching file drivers/media/dvb-frontends/sp887x.c patching file drivers/media/dvb-frontends/stv0299.c patching file drivers/media/dvb-frontends/stv0900_core.c patching file drivers/media/dvb-frontends/tda8083.c patching file drivers/media/i2c/cx25840/cx25840-core.c patching file drivers/media/pci/bt8xx/dst_ca.c patching file drivers/media/pci/cx23885/altera-ci.c patching file drivers/media/pci/cx23885/cimax2.c patching file drivers/media/pci/cx88/cx88-blackbird.c Hunk #1 FAILED at 721. Hunk #2 FAILED at 739. Hunk #3 FAILED at 755. 3 out of 3 hunks FAILED -- rejects in file drivers/media/pci/cx88/cx88-blackbird.c patching file drivers/media/pci/cx88/cx88-dvb.c patching file drivers/media/pci/cx88/cx88-mpeg.c patching file drivers/media/pci/cx88/cx88-tvaudio.c patching file drivers/media/pci/cx88/cx88-video.c patching file drivers/media/pci/saa7134/saa7134-video.c patching file drivers/media/platform/exynos-gsc/gsc-regs.c patching file drivers/media/radio/si470x/radio-si470x-i2c.c Hunk #1 succeeded at 308 (offset 11 lines). patching file drivers/media/radio/si470x/radio-si470x-usb.c patching file drivers/media/radio/si4713-i2c.c Hunk #1 FAILED at 1009. Hunk #2 FAILED at 1081. Hunk #3 FAILED at 1130. Hunk #4 FAILED at 1420. Hunk #5 FAILED at 1473. Hunk #6 FAILED at 1698. 6 out of 6 hunks FAILED -- rejects in file drivers/media/radio/si4713-i2c.c patching file drivers/media/usb/dvb-usb-v2/af9015.c patching file drivers/media/usb/dvb-usb-v2/af9035.c Hunk #1 succeeded at 520 (offset 1 line). As this patch is trivial enough, I'll just discard the affected hunks with quilt push -f, applying it partially. 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 6/7] [media] ds3000: add module parameter to force firmware upload
Em Wed, 03 Oct 2012 03:38:19 +0300
Antti Palosaari cr...@iki.fi escreveu:
On 09/28/2012 03:59 PM, Rémi Cardona wrote:
Signed-off-by: Rémi Cardona remi.card...@smartjog.com
Next time, please provide a better comment: why such change is
needed?
Reviewed-by: Antti Palosaari cr...@iki.fi
---
drivers/media/dvb-frontends/ds3000.c |6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/media/dvb-frontends/ds3000.c
b/drivers/media/dvb-frontends/ds3000.c
index 59184a8..c66d731 100644
--- a/drivers/media/dvb-frontends/ds3000.c
+++ b/drivers/media/dvb-frontends/ds3000.c
@@ -30,6 +30,7 @@
#include ds3000.h
static int debug;
+static int force_fw_upload;
#define dprintk(args...) \
do { \
@@ -396,7 +397,7 @@ static int ds3000_firmware_ondemand(struct dvb_frontend
*fe)
dprintk(%s()\n, __func__);
ret = ds3000_readreg(state, 0xb2);
- if (ret == 0) {
+ if (ret == 0 force_fw_upload == 0) {
This hunk got a conflict. I solved it manually and applied. See below.
Regards,
Mauro
-
[PATCH] [media] ds3000: add module parameter to force firmware upload
From: Rémi Cardona remi.card...@smartjog.com
[mche...@redhat.com: Fix a merge conflict]
Signed-off-by: Rémi Cardona remi.card...@smartjog.com
Reviewed-by: Antti Palosaari cr...@iki.fi
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
diff --git a/drivers/media/dvb-frontends/ds3000.c
b/drivers/media/dvb-frontends/ds3000.c
index 4c8ac26..5b63908 100644
--- a/drivers/media/dvb-frontends/ds3000.c
+++ b/drivers/media/dvb-frontends/ds3000.c
@@ -30,6 +30,7 @@
#include ds3000.h
static int debug;
+static int force_fw_upload;
#define dprintk(args...) \
do { \
@@ -392,11 +393,13 @@ static int ds3000_firmware_ondemand(struct dvb_frontend
*fe)
dprintk(%s()\n, __func__);
- if (ds3000_readreg(state, 0xb2) = 0)
+ ret = ds3000_readreg(state, 0xb2);
+ if (ret 0)
return ret;
- if (state-skip_fw_load)
- return 0;
+ if (state-skip_fw_load || !force_fw_upload)
+ return 0; /* Firmware already uploaded, skipping */
+
/* Load firmware */
/* request the firmware, this will block until someone uploads it */
printk(KERN_INFO %s: Waiting for firmware upload (%s)...\n, __func__,
@@ -1306,6 +1309,9 @@ static struct dvb_frontend_ops ds3000_ops = {
module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, Activates frontend debugging (default:0));
+module_param(force_fw_upload, int, 0644);
+MODULE_PARM_DESC(force_fw_upload, Force firmware upload (default:0));
+
MODULE_DESCRIPTION(DVB Frontend module for Montage Technology
DS3000/TS2020 hardware);
MODULE_AUTHOR(Konstantin Dimitrov);
--
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] tda18271-common: hold the I2C adapter during write transfers
Em Sat, 29 Sep 2012 15:20:23 -0400
Michael Krufky mkru...@linuxtv.org escreveu:
On Fri, Sep 28, 2012 at 2:56 PM, Michael Krufky mkru...@linuxtv.org wrote:
On Fri, Sep 28, 2012 at 2:31 PM, Antti Palosaari cr...@iki.fi wrote:
Hello,
Did not fix the issue. Problem remains same. With the sleep + that patch it
works still.
On 09/28/2012 06:04 PM, Mauro Carvalho Chehab wrote:
The tda18271 datasheet says:
The image rejection calibration and RF tracking filter
calibration must be launched exactly as described in the
flowchart, otherwise bad calibration or even blocking of the
TDA18211HD can result making it impossible to communicate
via the I2C-bus.
(yeah, tda18271 refers there to tda18211 - likely a typo at their
datasheets)
tda18211 is just same than tda18271 but without a analog.
That likely explains why sometimes tda18271 stops answering. That
is now happening more often on designs with drx-k chips, as the
firmware is now loaded asyncrousnly there.
While the above text doesn't explicitly tell that the I2C bus
couldn't be used by other devices during such initialization,
that seems to be a requirement there.
So, let's explicitly use the I2C lock there, avoiding I2C bus
share during those critical moments.
Compile-tested only. Please test.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
drivers/media/tuners/tda18271-common.c | 104
++---
1 file changed, 71 insertions(+), 33 deletions(-)
diff --git a/drivers/media/tuners/tda18271-common.c
b/drivers/media/tuners/tda18271-common.c
index 221171e..18c77af 100644
--- a/drivers/media/tuners/tda18271-common.c
+++ b/drivers/media/tuners/tda18271-common.c
@@ -187,7 +187,8 @@ int tda18271_read_extended(struct dvb_frontend *fe)
return (ret == 2 ? 0 : ret);
}
-int tda18271_write_regs(struct dvb_frontend *fe, int idx, int len)
+static int __tda18271_write_regs(struct dvb_frontend *fe, int idx, int
len,
+ bool lock_i2c)
{
struct tda18271_priv *priv = fe-tuner_priv;
unsigned char *regs = priv-tda18271_regs;
@@ -198,7 +199,6 @@ int tda18271_write_regs(struct dvb_frontend *fe, int
idx, int len)
BUG_ON((len == 0) || (idx + len sizeof(buf)));
-
switch (priv-small_i2c) {
case TDA18271_03_BYTE_CHUNK_INIT:
max = 3;
@@ -214,7 +214,19 @@ int tda18271_write_regs(struct dvb_frontend *fe, int
idx, int len)
max = 39;
}
- tda18271_i2c_gate_ctrl(fe, 1);
+
+ /*
+* If lock_i2c is true, it will take the I2C bus for tda18271
private
+* usage during the entire write ops, as otherwise, bad things
could
+* happen.
+* During device init, several write operations will happen. So,
+* tda18271_init_regs controls the I2C lock directly,
+* disabling lock_i2c here.
+*/
+ if (lock_i2c) {
+ tda18271_i2c_gate_ctrl(fe, 1);
+ i2c_lock_adapter(priv-i2c_props.adap);
+ }
while (len) {
if (max len)
max = len;
@@ -226,14 +238,17 @@ int tda18271_write_regs(struct dvb_frontend *fe, int
idx, int len)
msg.len = max + 1;
/* write registers */
- ret = i2c_transfer(priv-i2c_props.adap, msg, 1);
+ ret = __i2c_transfer(priv-i2c_props.adap, msg, 1);
if (ret != 1)
break;
idx += max;
len -= max;
}
- tda18271_i2c_gate_ctrl(fe, 0);
+ if (lock_i2c) {
+ i2c_unlock_adapter(priv-i2c_props.adap);
+ tda18271_i2c_gate_ctrl(fe, 0);
+ }
if (ret != 1)
tda_err(ERROR: idx = 0x%x, len = %d,
@@ -242,10 +257,16 @@ int tda18271_write_regs(struct dvb_frontend *fe, int
idx, int len)
return (ret == 1 ? 0 : ret);
}
+int tda18271_write_regs(struct dvb_frontend *fe, int idx, int len)
+{
+ return __tda18271_write_regs(fe, idx, len, true);
+}
+
/*-*/
-int tda18271_charge_pump_source(struct dvb_frontend *fe,
- enum tda18271_pll pll, int force)
+static int __tda18271_charge_pump_source(struct dvb_frontend *fe,
+enum tda18271_pll pll, int force,
+bool lock_i2c)
{
struct tda18271_priv *priv = fe-tuner_priv;
unsigned char *regs = priv-tda18271_regs;
@@ -255,9 +276,16 @@ int tda18271_charge_pump_source(struct dvb_frontend
*fe,
regs[r_cp] = ~0x20;
regs[r_cp] |= ((force 1) 5);
- return
Re: [patch] [media] cx25821: testing the wrong variable
Em Sat, 29 Sep 2012 10:12:53 +0300
Dan Carpenter dan.carpen...@oracle.com escreveu:
-input_filename could be NULL here. The intent was to test
-_filename.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
I'm not totally convinced that using /root/vid411.yuv is the right idea.
Agreed.
Palash, Sri,
Why do we need those files?
Regards,
Mauro
diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream.c
b/drivers/media/pci/cx25821/cx25821-video-upstream.c
index 52c13e0..6759fff 100644
--- a/drivers/media/pci/cx25821/cx25821-video-upstream.c
+++ b/drivers/media/pci/cx25821/cx25821-video-upstream.c
@@ -808,7 +808,7 @@ int cx25821_vidupstream_init_ch1(struct cx25821_dev *dev,
int channel_select,
}
/* Default if filename is empty string */
- if (strcmp(dev-input_filename, ) == 0) {
+ if (strcmp(dev-_filename, ) == 0) {
if (dev-_isNTSC) {
dev-_filename =
(dev-_pixel_format == PIXEL_FRMT_411) ?
diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
index c8c94fb..d33fc1a 100644
--- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
+++ b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
@@ -761,7 +761,7 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev,
int channel_select,
}
/* Default if filename is empty string */
- if (strcmp(dev-input_filename_ch2, ) == 0) {
+ if (strcmp(dev-_filename_ch2, ) == 0) {
if (dev-_isNTSC_ch2) {
dev-_filename_ch2 = (dev-_pixel_format_ch2 ==
PIXEL_FRMT_411) ? /root/vid411.yuv :
--
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
[PATCH] rc-msi-digivox-ii: Add full scan keycodes - Was: Re: v4l
Em Sun, 30 Sep 2012 05:49:26 +0200
Wolfgang Bail wolfgang.b...@t-online.de escreveu:
Hello,
the ir-rc from my msi DigiVox mini II Version 3 (af9015) will not work since
kernel 3.2.x (kubuntu 12.04), same with s2-liplianin or v4l.
sudo ir-keytable -t shows:
Testing events. Please, press CTRL-C to abort.
1348890734.303273: event MSC: scancode = 317
1348890734.303280: event key down: KEY_POWER (0x0074)
1348890734.303282: event sync
1348890734.553961: event key up: KEY_POWER (0x0074)
1348890734.553963: event sync
1348890741.303451: event MSC: scancode = 30d
1348890741.303457: event key down: KEY_DOWN (0x006c)
1348890741.303459: event sync
^[[B1348890741.553956: event key up: KEY_DOWN (0x006c)
So I changed in rc-msi-digivox-ii.c { 0x0002, KEY_2 }, to { 0x0302, KEY_2 },
and so on. And now it works well.
I hope, my mini patch is standard, the first I made.
Well, you should have using a subject like:
[PATCH] rc-msi-digivox-ii: Add full scan keycodes
And your signed-off-by. There are some pages at linuxtv.org wiki that points
how to write a patch.
Yet, as this is a really trivial one, I'll accept it without your Signed-off-by.
I don't know, whether
there are different variants of remote controls. But I don't believe it,
because it was ok with kernel 2.6.x.
No, this seems just yet-another-regression caused by some patch that changed
the code that gets IR scancode to report the 16-bit keycode, instead of
just the last 8 bits.
Thanks for it.
@Mauro, thank you for the reply.
Regards,
Mauro
-
FYI, this is how I applied it.
From: Wolfgang Bail wolfgang.b...@t-online.de
Date: Sat, 29 Sep 2012 23:49:26 -0300
Subject: [PATCH] [media] rc-msi-digivox-ii: Add full scan keycodes
The ir-rc from my MSI DigiVox mini II Version 3 (af9015) will not work since
kernel 3.2.x.
sudo ir-keytable -t shows:
1348890734.303273: event MSC: scancode = 317
1348890734.303280: event key down: KEY_POWER (0x0074)
1348890734.303282: event sync
1348890734.553961: event key up: KEY_POWER (0x0074)
1348890734.553963: event sync
1348890741.303451: event MSC: scancode = 30d
1348890741.303457: event key down: KEY_DOWN (0x006c)
1348890741.303459: event sync
1348890741.553956: event key up: KEY_DOWN (0x006c)
So I changed in rc-msi-digivox-ii.c { 0x0002, KEY_2 }, to { 0x0302, KEY_2 },
and so on. And now it works well.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
diff --git a/drivers/media/rc/keymaps/rc-msi-digivox-ii.c
b/drivers/media/rc/keymaps/rc-msi-digivox-ii.c
index c64e9e3..2fa71d0 100644
--- a/drivers/media/rc/keymaps/rc-msi-digivox-ii.c
+++ b/drivers/media/rc/keymaps/rc-msi-digivox-ii.c
@@ -22,24 +22,24 @@
#include linux/module.h
static struct rc_map_table msi_digivox_ii[] = {
- { 0x0002, KEY_2 },
- { 0x0003, KEY_UP }, /* up */
- { 0x0004, KEY_3 },
- { 0x0005, KEY_CHANNELDOWN },
- { 0x0008, KEY_5 },
- { 0x0009, KEY_0 },
- { 0x000b, KEY_8 },
- { 0x000d, KEY_DOWN },/* down */
- { 0x0010, KEY_9 },
- { 0x0011, KEY_7 },
- { 0x0014, KEY_VOLUMEUP },
- { 0x0015, KEY_CHANNELUP },
- { 0x0016, KEY_OK },
- { 0x0017, KEY_POWER2 },
- { 0x001a, KEY_1 },
- { 0x001c, KEY_4 },
- { 0x001d, KEY_6 },
- { 0x001f, KEY_VOLUMEDOWN },
+ { 0x0302, KEY_2 },
+ { 0x0303, KEY_UP }, /* up */
+ { 0x0304, KEY_3 },
+ { 0x0305, KEY_CHANNELDOWN },
+ { 0x0308, KEY_5 },
+ { 0x0309, KEY_0 },
+ { 0x030b, KEY_8 },
+ { 0x030d, KEY_DOWN },/* down */
+ { 0x0310, KEY_9 },
+ { 0x0311, KEY_7 },
+ { 0x0314, KEY_VOLUMEUP },
+ { 0x0315, KEY_CHANNELUP },
+ { 0x0316, KEY_OK },
+ { 0x0317, KEY_POWER2 },
+ { 0x031a, KEY_1 },
+ { 0x031c, KEY_4 },
+ { 0x031d, KEY_6 },
+ { 0x031f, KEY_VOLUMEDOWN },
};
static struct rc_map_list msi_digivox_ii_map = {
--
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] libv4lconvert: clarify the behavior and resulting restrictions of v4lconvert_convert()
Hi Frank, Thanks for all your work on this. I'm afraid that atm I'm very busy with work, so I don't have time to review your patches. I hope to find some time for this next weekend... Regards, Hans On 10/03/2012 06:48 PM, Frank Schäfer wrote: Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- lib/include/libv4lconvert.h | 20 ++-- 1 Datei geändert, 18 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-) diff --git a/lib/include/libv4lconvert.h b/lib/include/libv4lconvert.h index 167b57d..509655e 100644 --- a/lib/include/libv4lconvert.h +++ b/lib/include/libv4lconvert.h @@ -89,8 +89,24 @@ LIBV4L_PUBLIC int v4lconvert_needs_conversion(struct v4lconvert_data *data, const struct v4l2_format *src_fmt, /* in */ const struct v4l2_format *dest_fmt); /* in */ -/* return value of -1 on error, otherwise the amount of bytes written to - dest */ +/* This function does the following conversions: +- format conversion +- cropping + if enabled: +- processing (auto whitebalance, auto gain, gamma correction) +- horizontal/vertical flipping +- 90 degree (clockwise) rotation + + NOTE: the last 3 steps are enabled/disabled depending on +- the internal device list +- the state of the (software emulated) image controls + + Therefore this function should +- not be used when getting the frames from libv4l +- be called only once per frame + Otherwise this may result in unintended double conversions ! + + Returns the amount of bytes written to dest an -1 on error */ LIBV4L_PUBLIC int v4lconvert_convert(struct v4lconvert_data *data, const struct v4l2_format *src_fmt, /* in */ const struct v4l2_format *dest_fmt, /* in */ -- 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/5] [media] omap3isp: Fix compilation error in ispreg.h
Em Tue, 2 Oct 2012 09:31:58 -0700 Tony Lindgren t...@atomide.com escreveu: * Ido Yariv i...@wizery.com [121001 15:48]: Commit c49f34bc (ARM: OMAP2+ Move SoC specific headers to be local to mach-omap2) moved omap34xx.h to mach-omap2. This broke omap3isp, as it includes omap34xx.h. Instead of moving omap34xx to platform_data, simply add the two definitions the driver needs and remove the include altogether. Signed-off-by: Ido Yariv i...@wizery.com I'm assuming that Mauro picks this one up, sorry for breaking it. Picked, thanks. With regards to the other patches in this series, IMHO, it makes more sense to go through arm omap tree, so, for the patches on this series that touch at drivers/media/platform/*: Acked-by: Mauro Carvalho Chehab mche...@redhat.com Acked-by: Tony Lindgren t...@atomide.com --- drivers/media/platform/omap3isp/ispreg.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/media/platform/omap3isp/ispreg.h b/drivers/media/platform/omap3isp/ispreg.h index 084ea77..e2c57f3 100644 --- a/drivers/media/platform/omap3isp/ispreg.h +++ b/drivers/media/platform/omap3isp/ispreg.h @@ -27,13 +27,13 @@ #ifndef OMAP3_ISP_REG_H #define OMAP3_ISP_REG_H -#include plat/omap34xx.h - - #define CM_CAM_MCLK_HZ 17280 /* Hz */ /* ISP Submodules offset */ +#define L4_34XX_BASE 0x4800 +#define OMAP3430_ISP_BASE (L4_34XX_BASE + 0xBC000) + #define OMAP3ISP_REG_BASE OMAP3430_ISP_BASE #define OMAP3ISP_REG(offset) (OMAP3ISP_REG_BASE + (offset)) -- 1.7.11.4 -- To unsubscribe from this list: send the line unsubscribe linux-omap 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 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] tda18271-common: hold the I2C adapter during write transfers
On Sun, Oct 7, 2012 at 8:42 AM, Mauro Carvalho Chehab
mche...@redhat.com wrote:
Em Sat, 29 Sep 2012 15:20:23 -0400
Michael Krufky mkru...@linuxtv.org escreveu:
On Fri, Sep 28, 2012 at 2:56 PM, Michael Krufky mkru...@linuxtv.org wrote:
On Fri, Sep 28, 2012 at 2:31 PM, Antti Palosaari cr...@iki.fi wrote:
Hello,
Did not fix the issue. Problem remains same. With the sleep + that patch
it
works still.
On 09/28/2012 06:04 PM, Mauro Carvalho Chehab wrote:
The tda18271 datasheet says:
The image rejection calibration and RF tracking filter
calibration must be launched exactly as described in the
flowchart, otherwise bad calibration or even blocking of the
TDA18211HD can result making it impossible to communicate
via the I2C-bus.
(yeah, tda18271 refers there to tda18211 - likely a typo at their
datasheets)
tda18211 is just same than tda18271 but without a analog.
That likely explains why sometimes tda18271 stops answering. That
is now happening more often on designs with drx-k chips, as the
firmware is now loaded asyncrousnly there.
While the above text doesn't explicitly tell that the I2C bus
couldn't be used by other devices during such initialization,
that seems to be a requirement there.
So, let's explicitly use the I2C lock there, avoiding I2C bus
share during those critical moments.
Compile-tested only. Please test.
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
---
drivers/media/tuners/tda18271-common.c | 104
++---
1 file changed, 71 insertions(+), 33 deletions(-)
diff --git a/drivers/media/tuners/tda18271-common.c
b/drivers/media/tuners/tda18271-common.c
index 221171e..18c77af 100644
--- a/drivers/media/tuners/tda18271-common.c
+++ b/drivers/media/tuners/tda18271-common.c
@@ -187,7 +187,8 @@ int tda18271_read_extended(struct dvb_frontend *fe)
return (ret == 2 ? 0 : ret);
}
-int tda18271_write_regs(struct dvb_frontend *fe, int idx, int len)
+static int __tda18271_write_regs(struct dvb_frontend *fe, int idx, int
len,
+ bool lock_i2c)
{
struct tda18271_priv *priv = fe-tuner_priv;
unsigned char *regs = priv-tda18271_regs;
@@ -198,7 +199,6 @@ int tda18271_write_regs(struct dvb_frontend *fe, int
idx, int len)
BUG_ON((len == 0) || (idx + len sizeof(buf)));
-
switch (priv-small_i2c) {
case TDA18271_03_BYTE_CHUNK_INIT:
max = 3;
@@ -214,7 +214,19 @@ int tda18271_write_regs(struct dvb_frontend *fe, int
idx, int len)
max = 39;
}
- tda18271_i2c_gate_ctrl(fe, 1);
+
+ /*
+* If lock_i2c is true, it will take the I2C bus for tda18271
private
+* usage during the entire write ops, as otherwise, bad things
could
+* happen.
+* During device init, several write operations will happen. So,
+* tda18271_init_regs controls the I2C lock directly,
+* disabling lock_i2c here.
+*/
+ if (lock_i2c) {
+ tda18271_i2c_gate_ctrl(fe, 1);
+ i2c_lock_adapter(priv-i2c_props.adap);
+ }
while (len) {
if (max len)
max = len;
@@ -226,14 +238,17 @@ int tda18271_write_regs(struct dvb_frontend *fe,
int
idx, int len)
msg.len = max + 1;
/* write registers */
- ret = i2c_transfer(priv-i2c_props.adap, msg, 1);
+ ret = __i2c_transfer(priv-i2c_props.adap, msg, 1);
if (ret != 1)
break;
idx += max;
len -= max;
}
- tda18271_i2c_gate_ctrl(fe, 0);
+ if (lock_i2c) {
+ i2c_unlock_adapter(priv-i2c_props.adap);
+ tda18271_i2c_gate_ctrl(fe, 0);
+ }
if (ret != 1)
tda_err(ERROR: idx = 0x%x, len = %d,
@@ -242,10 +257,16 @@ int tda18271_write_regs(struct dvb_frontend *fe,
int
idx, int len)
return (ret == 1 ? 0 : ret);
}
+int tda18271_write_regs(struct dvb_frontend *fe, int idx, int len)
+{
+ return __tda18271_write_regs(fe, idx, len, true);
+}
+
/*-*/
-int tda18271_charge_pump_source(struct dvb_frontend *fe,
- enum tda18271_pll pll, int force)
+static int __tda18271_charge_pump_source(struct dvb_frontend *fe,
+enum tda18271_pll pll, int
force,
+bool lock_i2c)
{
struct tda18271_priv *priv = fe-tuner_priv;
unsigned char *regs = priv-tda18271_regs;
@@ -255,9 +276,16 @@ int tda18271_charge_pump_source(struct dvb_frontend
*fe,
Re: [git:v4l-dvb/for_v3.7] [media] tda18271-common: hold the I2C adapter during write transfers
umm, again, i didn't actually ACK the patch, I verbally said ok, i guess
You shouldn't forge someone's signature, Mauro. :-(
On Sun, Oct 7, 2012 at 8:43 AM, Mauro Carvalho Chehab
mche...@redhat.com wrote:
This is an automatic generated email to let you know that the following patch
were queued at the
http://git.linuxtv.org/media_tree.git tree:
Subject: [media] tda18271-common: hold the I2C adapter during write transfers
Author: Mauro Carvalho Chehab mche...@redhat.com
Date:Fri Sep 28 11:04:21 2012 -0300
The tda18271 datasheet says:
The image rejection calibration and RF tracking filter
calibration must be launched exactly as described in the
flowchart, otherwise bad calibration or even blocking of the
TDA18211HD can result making it impossible to communicate
via the I2C-bus.
(yeah, tda18271 refers there to tda18211 - likely a typo at their
datasheets)
That likely explains why sometimes tda18271 stops answering. That
is now happening more often on designs with drx-k chips, as the
firmware is now loaded asyncrousnly there.
While the above text doesn't explicitly tell that the I2C bus
couldn't be used by other devices during such initialization,
that seems to be a requirement there.
So, let's explicitly use the I2C lock there, avoiding I2C bus
share during those critical moments.
Compile-tested only. Please test.
Acked-by: Michael Krufky mkru...@linuxtv.org
Signed-off-by: Mauro Carvalho Chehab mche...@redhat.com
drivers/media/tuners/tda18271-common.c | 104
++--
1 files changed, 71 insertions(+), 33 deletions(-)
---
http://git.linuxtv.org/media_tree.git?a=commitdiff;h=78ef81f6ea9649fd09d1fafcfa0ad68763172c42
diff --git a/drivers/media/tuners/tda18271-common.c
b/drivers/media/tuners/tda18271-common.c
index 221171e..18c77af 100644
--- a/drivers/media/tuners/tda18271-common.c
+++ b/drivers/media/tuners/tda18271-common.c
@@ -187,7 +187,8 @@ int tda18271_read_extended(struct dvb_frontend *fe)
return (ret == 2 ? 0 : ret);
}
-int tda18271_write_regs(struct dvb_frontend *fe, int idx, int len)
+static int __tda18271_write_regs(struct dvb_frontend *fe, int idx, int len,
+ bool lock_i2c)
{
struct tda18271_priv *priv = fe-tuner_priv;
unsigned char *regs = priv-tda18271_regs;
@@ -198,7 +199,6 @@ int tda18271_write_regs(struct dvb_frontend *fe, int idx,
int len)
BUG_ON((len == 0) || (idx + len sizeof(buf)));
-
switch (priv-small_i2c) {
case TDA18271_03_BYTE_CHUNK_INIT:
max = 3;
@@ -214,7 +214,19 @@ int tda18271_write_regs(struct dvb_frontend *fe, int
idx, int len)
max = 39;
}
- tda18271_i2c_gate_ctrl(fe, 1);
+
+ /*
+* If lock_i2c is true, it will take the I2C bus for tda18271 private
+* usage during the entire write ops, as otherwise, bad things could
+* happen.
+* During device init, several write operations will happen. So,
+* tda18271_init_regs controls the I2C lock directly,
+* disabling lock_i2c here.
+*/
+ if (lock_i2c) {
+ tda18271_i2c_gate_ctrl(fe, 1);
+ i2c_lock_adapter(priv-i2c_props.adap);
+ }
while (len) {
if (max len)
max = len;
@@ -226,14 +238,17 @@ int tda18271_write_regs(struct dvb_frontend *fe, int
idx, int len)
msg.len = max + 1;
/* write registers */
- ret = i2c_transfer(priv-i2c_props.adap, msg, 1);
+ ret = __i2c_transfer(priv-i2c_props.adap, msg, 1);
if (ret != 1)
break;
idx += max;
len -= max;
}
- tda18271_i2c_gate_ctrl(fe, 0);
+ if (lock_i2c) {
+ i2c_unlock_adapter(priv-i2c_props.adap);
+ tda18271_i2c_gate_ctrl(fe, 0);
+ }
if (ret != 1)
tda_err(ERROR: idx = 0x%x, len = %d,
@@ -242,10 +257,16 @@ int tda18271_write_regs(struct dvb_frontend *fe, int
idx, int len)
return (ret == 1 ? 0 : ret);
}
+int tda18271_write_regs(struct dvb_frontend *fe, int idx, int len)
+{
+ return __tda18271_write_regs(fe, idx, len, true);
+}
+
/*-*/
-int tda18271_charge_pump_source(struct dvb_frontend *fe,
- enum tda18271_pll pll, int force)
+static int __tda18271_charge_pump_source(struct dvb_frontend *fe,
+enum tda18271_pll pll, int force,
+bool lock_i2c)
{
struct tda18271_priv *priv = fe-tuner_priv;
unsigned char *regs = priv-tda18271_regs;
@@ -255,9 +276,16 @@ int tda18271_charge_pump_source(struct dvb_frontend *fe,
Re: [PATCH RFC v3] dvb: LNA implementation changes
Em Wed, 3 Oct 2012 11:28:56 +0300
Antti Palosaari cr...@iki.fi escreveu:
* use dvb property cache
* implement get (thus API minor++)
* PCTV 290e: 1=LNA ON, all the other values LNA OFF
Also fix PCTV 290e LNA comment, it is disabled by default
Hans and Mauro proposed use of cache implementation of get as they
were planning to extend LNA usage for analog side too.
Looks sane for me. I'll apply it, as Hans also acked.
Regards,
Mauro
Reported-by: Hans Verkuil hverk...@xs4all.nl
Reported-by: Mauro Carvalho Chehab mche...@redhat.com
Signed-off-by: Antti Palosaari cr...@iki.fi
Acked-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/dvb-core/dvb_frontend.c | 18 ++
drivers/media/dvb-core/dvb_frontend.h | 4 +++-
drivers/media/usb/em28xx/em28xx-dvb.c | 13 +++--
include/linux/dvb/version.h | 2 +-
4 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/drivers/media/dvb-core/dvb_frontend.c
b/drivers/media/dvb-core/dvb_frontend.c
index 8f58f24..246a3c5 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -966,6 +966,8 @@ static int dvb_frontend_clear_cache(struct dvb_frontend
*fe)
break;
}
+ c-lna = LNA_AUTO;
+
return 0;
}
@@ -1054,6 +1056,8 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1]
= {
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_B, 0, 0),
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_C, 0, 0),
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_D, 0, 0),
+
+ _DTV_CMD(DTV_LNA, 0, 0),
};
static void dtv_property_dump(struct dvb_frontend *fe, struct dtv_property
*tvp)
@@ -1440,6 +1444,10 @@ static int dtv_property_process_get(struct
dvb_frontend *fe,
tvp-u.data = fe-dtv_property_cache.atscmh_sccc_code_mode_d;
break;
+ case DTV_LNA:
+ tvp-u.data = c-lna;
+ break;
+
default:
return -EINVAL;
}
@@ -1731,10 +1739,6 @@ static int dtv_property_process_set(struct
dvb_frontend *fe,
case DTV_INTERLEAVING:
c-interleaving = tvp-u.data;
break;
- case DTV_LNA:
- if (fe-ops.set_lna)
- r = fe-ops.set_lna(fe, tvp-u.data);
- break;
/* ISDB-T Support here */
case DTV_ISDBT_PARTIAL_RECEPTION:
@@ -1806,6 +1810,12 @@ static int dtv_property_process_set(struct
dvb_frontend *fe,
fe-dtv_property_cache.atscmh_rs_frame_ensemble = tvp-u.data;
break;
+ case DTV_LNA:
+ c-lna = tvp-u.data;
+ if (fe-ops.set_lna)
+ r = fe-ops.set_lna(fe);
+ break;
+
default:
return -EINVAL;
}
diff --git a/drivers/media/dvb-core/dvb_frontend.h
b/drivers/media/dvb-core/dvb_frontend.h
index 44a445c..97112cd 100644
--- a/drivers/media/dvb-core/dvb_frontend.h
+++ b/drivers/media/dvb-core/dvb_frontend.h
@@ -303,7 +303,7 @@ struct dvb_frontend_ops {
int (*dishnetwork_send_legacy_command)(struct dvb_frontend* fe,
unsigned long cmd);
int (*i2c_gate_ctrl)(struct dvb_frontend* fe, int enable);
int (*ts_bus_ctrl)(struct dvb_frontend* fe, int acquire);
- int (*set_lna)(struct dvb_frontend *, int);
+ int (*set_lna)(struct dvb_frontend *);
/* These callbacks are for devices that implement their own
* tuning algorithms, rather than a simple swzigzag
@@ -391,6 +391,8 @@ struct dtv_frontend_properties {
u8 atscmh_sccc_code_mode_b;
u8 atscmh_sccc_code_mode_c;
u8 atscmh_sccc_code_mode_d;
+
+ u32 lna;
};
struct dvb_frontend {
diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c
b/drivers/media/usb/em28xx/em28xx-dvb.c
index 913e522..13ae821 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -574,18 +574,19 @@ static void pctv_520e_init(struct em28xx *dev)
i2c_master_send(dev-i2c_client, regs[i].r, regs[i].len);
};
-static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe, int val)
+static int em28xx_pctv_290e_set_lna(struct dvb_frontend *fe)
{
+ struct dtv_frontend_properties *c = fe-dtv_property_cache;
struct em28xx *dev = fe-dvb-priv;
#ifdef CONFIG_GPIOLIB
struct em28xx_dvb *dvb = dev-dvb;
int ret;
unsigned long flags;
- if (val)
- flags = GPIOF_OUT_INIT_LOW;
+ if (c-lna == 1)
+ flags = GPIOF_OUT_INIT_HIGH; /* enable LNA */
else
- flags = GPIOF_OUT_INIT_HIGH;
+ flags = GPIOF_OUT_INIT_LOW; /* disable LNA */
ret = gpio_request_one(dvb-lna_gpio, flags, NULL);
if (ret)
@@ -595,8 +596,8 @@ static int em28xx_pctv_290e_set_lna(struct dvb_frontend
*fe, int val)
return ret;
#else
-
Re: [PATCH RFC v3] dvb: LNA implementation changes
Em Wed, 3 Oct 2012 11:28:56 +0300
Antti Palosaari cr...@iki.fi escreveu:
* use dvb property cache
* implement get (thus API minor++)
* PCTV 290e: 1=LNA ON, all the other values LNA OFF
Also fix PCTV 290e LNA comment, it is disabled by default
Hans and Mauro proposed use of cache implementation of get as they
were planning to extend LNA usage for analog side too.
Reported-by: Hans Verkuil hverk...@xs4all.nl
Reported-by: Mauro Carvalho Chehab mche...@redhat.com
Signed-off-by: Antti Palosaari cr...@iki.fi
Acked-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/dvb-core/dvb_frontend.c | 18 ++
drivers/media/dvb-core/dvb_frontend.h | 4 +++-
drivers/media/usb/em28xx/em28xx-dvb.c | 13 +++--
include/linux/dvb/version.h | 2 +-
4 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/drivers/media/dvb-core/dvb_frontend.c
b/drivers/media/dvb-core/dvb_frontend.c
index 8f58f24..246a3c5 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -966,6 +966,8 @@ static int dvb_frontend_clear_cache(struct dvb_frontend
*fe)
break;
}
+ c-lna = LNA_AUTO;
+
return 0;
}
@@ -1054,6 +1056,8 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1]
= {
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_B, 0, 0),
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_C, 0, 0),
_DTV_CMD(DTV_ATSCMH_SCCC_CODE_MODE_D, 0, 0),
+
+ _DTV_CMD(DTV_LNA, 0, 0),
};
static void dtv_property_dump(struct dvb_frontend *fe, struct dtv_property
*tvp)
@@ -1440,6 +1444,10 @@ static int dtv_property_process_get(struct
dvb_frontend *fe,
tvp-u.data = fe-dtv_property_cache.atscmh_sccc_code_mode_d;
break;
+ case DTV_LNA:
+ tvp-u.data = c-lna;
+ break;
+
default:
return -EINVAL;
}
@@ -1731,10 +1739,6 @@ static int dtv_property_process_set(struct
dvb_frontend *fe,
case DTV_INTERLEAVING:
c-interleaving = tvp-u.data;
break;
- case DTV_LNA:
- if (fe-ops.set_lna)
- r = fe-ops.set_lna(fe, tvp-u.data);
- break;
/* ISDB-T Support here */
case DTV_ISDBT_PARTIAL_RECEPTION:
@@ -1806,6 +1810,12 @@ static int dtv_property_process_set(struct
dvb_frontend *fe,
fe-dtv_property_cache.atscmh_rs_frame_ensemble = tvp-u.data;
break;
+ case DTV_LNA:
+ c-lna = tvp-u.data;
+ if (fe-ops.set_lna)
+ r = fe-ops.set_lna(fe);
+ break;
Hmm... on a second thought, I think that the implementation there should not me
that
simple: during tuner sleep, and suspend/resume, you may need to force LNA to
off, in
order to save power and prevent device overheat.
Still, as the previous code weren't doing it, I'm still applying it, but I
think we
need to properly handle such cases.
Regards,
Mauro
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: [PATCHv9 20/25] v4l: vb2-dma-contig: add support for DMABUF exporting
Hi Tomasz,
Thanks for the patch.
On Tuesday 02 October 2012 16:27:31 Tomasz Stanislawski wrote:
This patch adds support for exporting a dma-contig buffer using
DMABUF interface.
Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/media/video/videobuf2-dma-contig.c | 200 +
1 file changed, 200 insertions(+)
diff --git a/drivers/media/video/videobuf2-dma-contig.c
b/drivers/media/video/videobuf2-dma-contig.c index 0e065ce..b138b5c 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -36,6 +36,7 @@ struct vb2_dc_buf {
/* MMAP related */
struct vb2_vmarea_handler handler;
atomic_trefcount;
+ struct sg_table *sgt_base;
/* USERPTR related */
struct vm_area_struct *vma;
@@ -142,6 +143,10 @@ static void vb2_dc_put(void *buf_priv)
if (!atomic_dec_and_test(buf-refcount))
return;
+ if (buf-sgt_base) {
+ sg_free_table(buf-sgt_base);
+ kfree(buf-sgt_base);
+ }
dma_free_coherent(buf-dev, buf-size, buf-vaddr, buf-dma_addr);
kfree(buf);
}
@@ -213,6 +218,200 @@ static int vb2_dc_mmap(void *buf_priv, struct
vm_area_struct *vma) }
/*/
+/* DMABUF ops for exporters */
+/*/
+
+struct vb2_dc_attachment {
+ struct sg_table sgt;
+ enum dma_data_direction dir;
+};
+
+static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device
*dev, + struct dma_buf_attachment *dbuf_attach)
+{
+ struct vb2_dc_attachment *attach;
+ unsigned int i;
+ struct scatterlist *rd, *wr;
+ struct sg_table *sgt;
+ struct vb2_dc_buf *buf = dbuf-priv;
+ int ret;
+
+ attach = kzalloc(sizeof(*attach), GFP_KERNEL);
+ if (!attach)
+ return -ENOMEM;
+
+ sgt = attach-sgt;
+ /* Copy the buf-base_sgt scatter list to the attachment, as we can't
+ * map the same scatter list to multiple attachments at the same time.
+ */
+ ret = sg_alloc_table(sgt, buf-sgt_base-orig_nents, GFP_KERNEL);
+ if (ret) {
+ kfree(attach);
+ return -ENOMEM;
+ }
+
+ rd = buf-sgt_base-sgl;
+ wr = sgt-sgl;
+ for (i = 0; i sgt-orig_nents; ++i) {
+ sg_set_page(wr, sg_page(rd), rd-length, rd-offset);
+ rd = sg_next(rd);
+ wr = sg_next(wr);
+ }
+
+ attach-dir = DMA_NONE;
+ dbuf_attach-priv = attach;
+
+ return 0;
+}
+
+static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf,
+ struct dma_buf_attachment *db_attach)
+{
+ struct vb2_dc_attachment *attach = db_attach-priv;
+ struct sg_table *sgt;
+
+ if (!attach)
+ return;
+
+ sgt = attach-sgt;
+
+ /* release the scatterlist cache */
+ if (attach-dir != DMA_NONE)
+ dma_unmap_sg(db_attach-dev, sgt-sgl, sgt-orig_nents,
+ attach-dir);
+ sg_free_table(sgt);
+ kfree(attach);
+ db_attach-priv = NULL;
+}
+
+static struct sg_table *vb2_dc_dmabuf_ops_map(
+ struct dma_buf_attachment *db_attach, enum dma_data_direction dir)
+{
+ struct vb2_dc_attachment *attach = db_attach-priv;
+ /* stealing dmabuf mutex to serialize map/unmap operations */
+ struct mutex *lock = db_attach-dmabuf-lock;
+ struct sg_table *sgt;
+ int ret;
+
+ mutex_lock(lock);
+
+ sgt = attach-sgt;
+ /* return previously mapped sg table */
+ if (attach-dir == dir) {
+ mutex_unlock(lock);
+ return sgt;
+ }
+
+ /* release any previous cache */
+ if (attach-dir != DMA_NONE) {
+ dma_unmap_sg(db_attach-dev, sgt-sgl, sgt-orig_nents,
+ attach-dir);
+ attach-dir = DMA_NONE;
+ }
+
+ /* mapping to the client with new direction */
+ ret = dma_map_sg(db_attach-dev, sgt-sgl, sgt-orig_nents, dir);
+ if (ret = 0) {
+ pr_err(failed to map scatterlist\n);
+ mutex_unlock(lock);
+ return ERR_PTR(-EIO);
+ }
+
+ attach-dir = dir;
+
+ mutex_unlock(lock);
+
+ return sgt;
+}
+
+static void vb2_dc_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
+ struct sg_table *sgt, enum dma_data_direction dir)
+{
+ /* nothing to be done here */
+}
+
+static void vb2_dc_dmabuf_ops_release(struct dma_buf *dbuf)
+{
+ /* drop reference obtained in vb2_dc_get_dmabuf */
+ vb2_dc_put(dbuf-priv);
+}
+
+static void *vb2_dc_dmabuf_ops_kmap(struct dma_buf *dbuf, unsigned long
pgnum) +{
+ struct vb2_dc_buf *buf = dbuf-priv;
+
+ return
Re: [PATCHv9 21/25] v4l: vb2-dma-contig: add reference counting for a device from allocator context
Hi Tomasz,
Thanks for the patch.
On Tuesday 02 October 2012 16:27:32 Tomasz Stanislawski wrote:
This patch adds taking reference to the device for MMAP buffers.
Such buffers, may be exported using DMABUF mechanism. If the driver that
created a queue is unloaded then the queue is released, the device might be
released too. However, buffers cannot be released if they are referenced by
DMABUF descriptor(s). The device pointer kept in a buffer must be valid for
the whole buffer's lifetime. Therefore MMAP buffers should take a reference
to the device to avoid risk of dangling pointers.
Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
---
drivers/media/video/videobuf2-dma-contig.c |5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/media/video/videobuf2-dma-contig.c
b/drivers/media/video/videobuf2-dma-contig.c index b138b5c..b4d287a 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -148,6 +148,7 @@ static void vb2_dc_put(void *buf_priv)
kfree(buf-sgt_base);
}
dma_free_coherent(buf-dev, buf-size, buf-vaddr, buf-dma_addr);
+ put_device(buf-dev);
kfree(buf);
}
@@ -161,9 +162,13 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned
long size) if (!buf)
return ERR_PTR(-ENOMEM);
+ /* prevent the device from release while the buffer is exported */
+ get_device(dev);
+
What about moving this below the dma_alloc_coherent() call ? You could then
avoid the put_device() call in the error path.
buf-vaddr = dma_alloc_coherent(dev, size, buf-dma_addr, GFP_KERNEL);
if (!buf-vaddr) {
dev_err(dev, dma_alloc_coherent of size %ld failed\n, size);
+ put_device(dev);
kfree(buf);
return ERR_PTR(-ENOMEM);
}
Something like
- buf-dev = dev;
+ buf-dev = get_device(dev);
buf-size = size
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v6] of: add helper to parse display timings
Hi Steffen,
On Friday 05 October 2012 18:38:58 Steffen Trumtrar wrote:
On Fri, Oct 05, 2012 at 10:21:37AM -0600, Stephen Warren wrote:
On 10/05/2012 10:16 AM, Steffen Trumtrar wrote:
On Thu, Oct 04, 2012 at 12:47:16PM -0600, Stephen Warren wrote:
On 10/04/2012 11:59 AM, Steffen Trumtrar wrote:
...
+ for_each_child_of_node(timings_np, entry) {
+ struct signal_timing *st;
+
+ st = of_get_display_timing(entry);
+
+ if (!st)
+ continue;
I wonder if that shouldn't be an error?
In the sense of a pr_err not a -EINVAL I presume?! It is a little bit
quiet in case of a faulty spec, that is right.
I did mean return an error; if we try to parse something and can't,
shouldn't we return an error?
I suppose it may be possible to limp on and use whatever subset of modes
could be parsed and drop the others, which is what this code does, but
the code after the loop would definitely return an error if zero timings
were parseable.
If a display supports multiple modes, I think it is better to have a working
mode (even if it is not the preferred one) than have none at all.
If there is no mode at all, that should be an error, right.
If we fail completely in case of an error, DT writers will notice their bugs.
If we ignore errors silently they won't, and we'll end up with buggy DTs (or,
to be accurate, even more buggy DTs :-)). I'd rather fail completely in the
first implementation and add workarounds later only if we need to.
--
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: [PATCHv9 18/25] v4l: add buffer exporting via dmabuf
Hi Hans,
On Friday 05 October 2012 10:55:40 Hans Verkuil wrote:
On Tue October 2 2012 16:27:29 Tomasz Stanislawski wrote:
This patch adds extension to V4L2 api. It allow to export a mmap buffer as
file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer
offset used by mmap and return a file descriptor on success.
Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
[snip]
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index e04a73e..f429b6a 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -688,6 +688,33 @@ struct v4l2_buffer {
#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0800
#define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000
+/**
+ * struct v4l2_exportbuffer - export of video buffer as DMABUF file
descriptor + *
+ * @fd:file descriptor associated with DMABUF (set by driver)
+ * @flags: flags for newly created file, currently only O_CLOEXEC is
+ * supported, refer to manual of open syscall for more details
+ * @index: id number of the buffer
+ * @type: enum v4l2_buf_type; buffer type (type == *_MPLANE for
+ * multiplanar buffers);
+ * @plane: index of the plane to be exported, 0 for single plane queues
+ *
+ * Contains data used for exporting a video buffer as DMABUF file
descriptor. + * The buffer is identified by a 'cookie' returned by
VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
userspace). All + * reserved fields must be set to zero. The field
reserved0 is expected to + * become a structure 'type' allowing an
alternative layout of the structure + * content. Therefore this field
should not be used for any other extensions. + */
+struct v4l2_exportbuffer {
+ __s32 fd;
+ __u32 flags;
+ __u32 type; /* enum v4l2_buf_type */
+ __u32 index;
+ __u32 plane;
As suggested in my comments in the previous patch, I think it is a more
natural order to have the type/index/plane fields first in this struct.
Actually, I think that flags should also come before fd:
struct v4l2_exportbuffer {
__u32 type; /* enum v4l2_buf_type */
__u32 index;
__u32 plane;
__u32 flags;
__s32 fd;
__u32 reserved[11];
};
It would indeed feel more natural, but putting them right before the reserved
fields allows creating an anonymous union around type, index and plane and
extending it with reserved fields if needed. That's (at least to my
understanding) the rationale behind the current structure layout.
+ __u32 reserved[11];
+};
+
/*
* O V E R L A Y P R E V I E W
*/
--
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: [PATCHv9 22/25] v4l: vb2-dma-contig: fail if user ptr buffer is not correctly aligned
Hi Tomasz,
Thanks for the patch.
On Tuesday 02 October 2012 16:27:33 Tomasz Stanislawski wrote:
From: Marek Szyprowski m.szyprow...@samsung.com
The DMA transfer must be aligned to a specific value. If userptr is not
aligned to DMA requirements then unexpected corruptions of the memory may
occur before or after a buffer. To prevent such situations, all unligned
userptr buffers are rejected at VIDIOC_QBUF.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/media/video/videobuf2-dma-contig.c |7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/media/video/videobuf2-dma-contig.c
b/drivers/media/video/videobuf2-dma-contig.c index b4d287a..55f8c80 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -494,6 +494,13 @@ static void *vb2_dc_get_userptr(void *alloc_ctx,
unsigned long vaddr, struct vm_area_struct *vma;
struct sg_table *sgt;
unsigned long contig_size;
+ unsigned long dma_align = dma_get_cache_alignment();
+
+ /* Only cache aligned DMA transfers are reliable */
+ if (!IS_ALIGNED(vaddr | size, dma_align)) {
+ pr_err(user data must be aligned to %lu bytes\n, dma_align);
+ return ERR_PTR(-EINVAL);
Do you think EFAULT would be more descriptive ? EINVAL is already used quite
extensively. We could then possibly turn pr_err() into pr_dbg() to avoid
flooding the kernel log.
+ }
buf = kzalloc(sizeof *buf, GFP_KERNEL);
if (!buf)
--
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: How to add support for the em2765 webcam Speedlink VAD Laplace to the kernel ?
Am 06.10.2012 13:56, schrieb Mauro Carvalho Chehab:
Em Sun, 23 Sep 2012 16:03:25 +0200
Frank Schäfer fschaefer@googlemail.com escreveu:
Ping !
Sorry, too busy those days.
No problem.
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.
Ok.
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...
AFAIKT, newer em28xx chips are using this concept. The em28xx-i2c code require
changes to support two I2C buses, and to handle 16 bit eeproms. We never cared
of doing that because we never needed, so far, to read anything from those
devices' eeproms.
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
Re: How to add support for the em2765 webcam Speedlink VAD Laplace to the kernel ?
Am 07.10.2012 04:56, schrieb Devin Heitmueller: On Sat, Oct 6, 2012 at 7:56 AM, Mauro Carvalho Chehab mche...@redhat.com wrote: AFAIKT, newer em28xx chips are using this concept. The em28xx-i2c code require changes to support two I2C buses, and to handle 16 bit eeproms. We never cared of doing that because we never needed, so far, to read anything from those devices' eeproms. I actually wrote the code to read the 16-bit eeprom from the em2874, but removed it before submitting it upstream because I was afraid well-intentioned em28xx users trying to add support for their boards would trash their eeprom. This is because performing a read against a 16-bit eeprom is equivalent to a write on an 8-bit eeprom. Hence if the user didn't know what he/she was doing, and used the 16-bit eeprom code against an older eeprom, the eeprom would get trashed (this actually happened to me once when I was doing the em2874 device support originally). Yes, I've read the comment in the code... According to the (possibly outdated) em2580/em2585 datasheet I've found, these chips support 16bit eeproms ONLY. What do we know about the others ? Are there any chips which support both 8bit and 16bit eeproms ? Maybe we can make it depending on the chip_id. With regards to eeprom type probing: I've made some experiments to find out what happens when trying to access the 16bit eeprom in my device as 8bit eeprom. My hope was to get a clear result like an i2c error, no data or all bytes beeing 0x00 or 0xff. Unfortunately, there is no error and I'm getting random data (would have to cerify if it's really random). So probing will be difficult. If we really want that code back in the tree, I can dig it up -- but I won't be responsible for users killing their devices. Indeed, we should be very careful. Regards, Frank Devin -- 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] libv4lconvert: clarify the behavior and resulting restrictions of v4lconvert_convert()
Hi Hans, Am 07.10.2012 15:05, schrieb Hans de Goede: Hi Frank, Thanks for all your work on this. I'm afraid that atm I'm very busy with work, so I don't have time to review your patches. I hope to find some time for this next weekend... No problem, I will send you a reminder in 2 weeks. ;) I didn't have much time yet to work on further libv4lconvert patches, too. Regards, Frank Regards, Hans On 10/03/2012 06:48 PM, Frank Schäfer wrote: Signed-off-by: Frank Schäfer fschaefer@googlemail.com --- lib/include/libv4lconvert.h | 20 ++-- 1 Datei geändert, 18 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-) diff --git a/lib/include/libv4lconvert.h b/lib/include/libv4lconvert.h index 167b57d..509655e 100644 --- a/lib/include/libv4lconvert.h +++ b/lib/include/libv4lconvert.h @@ -89,8 +89,24 @@ LIBV4L_PUBLIC int v4lconvert_needs_conversion(struct v4lconvert_data *data, const struct v4l2_format *src_fmt, /* in */ const struct v4l2_format *dest_fmt); /* in */ -/* return value of -1 on error, otherwise the amount of bytes written to - dest */ +/* This function does the following conversions: +- format conversion +- cropping + if enabled: +- processing (auto whitebalance, auto gain, gamma correction) +- horizontal/vertical flipping +- 90 degree (clockwise) rotation + + NOTE: the last 3 steps are enabled/disabled depending on +- the internal device list +- the state of the (software emulated) image controls + + Therefore this function should +- not be used when getting the frames from libv4l +- be called only once per frame + Otherwise this may result in unintended double conversions ! + + Returns the amount of bytes written to dest an -1 on error */ LIBV4L_PUBLIC int v4lconvert_convert(struct v4lconvert_data *data, const struct v4l2_format *src_fmt, /* in */ const struct v4l2_format *dest_fmt, /* in */ -- 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: [PATCHv9 18/25] v4l: add buffer exporting via dmabuf
On Sun October 7 2012 15:38:30 Laurent Pinchart wrote:
Hi Hans,
On Friday 05 October 2012 10:55:40 Hans Verkuil wrote:
On Tue October 2 2012 16:27:29 Tomasz Stanislawski wrote:
This patch adds extension to V4L2 api. It allow to export a mmap buffer as
file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer
offset used by mmap and return a file descriptor on success.
Signed-off-by: Tomasz Stanislawski t.stanisl...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
[snip]
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index e04a73e..f429b6a 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -688,6 +688,33 @@ struct v4l2_buffer {
#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE0x0800
#define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x1000
+/**
+ * struct v4l2_exportbuffer - export of video buffer as DMABUF file
descriptor + *
+ * @fd: file descriptor associated with DMABUF (set by driver)
+ * @flags: flags for newly created file, currently only O_CLOEXEC
is
+ * supported, refer to manual of open syscall for more
details
+ * @index: id number of the buffer
+ * @type:enum v4l2_buf_type; buffer type (type == *_MPLANE for
+ * multiplanar buffers);
+ * @plane: index of the plane to be exported, 0 for single plane
queues
+ *
+ * Contains data used for exporting a video buffer as DMABUF file
descriptor. + * The buffer is identified by a 'cookie' returned by
VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
userspace). All + * reserved fields must be set to zero. The field
reserved0 is expected to + * become a structure 'type' allowing an
alternative layout of the structure + * content. Therefore this field
should not be used for any other extensions. + */
+struct v4l2_exportbuffer {
+ __s32 fd;
+ __u32 flags;
+ __u32 type; /* enum v4l2_buf_type */
+ __u32 index;
+ __u32 plane;
As suggested in my comments in the previous patch, I think it is a more
natural order to have the type/index/plane fields first in this struct.
Actually, I think that flags should also come before fd:
struct v4l2_exportbuffer {
__u32 type; /* enum v4l2_buf_type */
__u32 index;
__u32 plane;
__u32 flags;
__s32 fd;
__u32 reserved[11];
};
It would indeed feel more natural, but putting them right before the reserved
fields allows creating an anonymous union around type, index and plane and
extending it with reserved fields if needed. That's (at least to my
understanding) the rationale behind the current structure layout.
The anonymous union argument makes no sense to me, to be honest. It's standard
practice within V4L2 to have the IN fields first, then the OUT fields, followed
by reserved fields for future expansion. Should we ever need a, say, sub-plane
index (whatever that might be), then we can use one of the reserved fields.
Regards,
Hans
+ __u32 reserved[11];
+};
+
/*
* O V E R L A Y P R E V I E W
*/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/11] introduce macros for i2c_msg initialization
This patch set introduces some macros for describing how an i2c_msg is
being initialized. There are three macros: I2C_MSG_READ, for a read
message, I2C_MSG_WRITE, for a write message, and I2C_MSG_OP, for some other
kind of message, which is expected to be very rarely used.
Some i2c_msg initializations have been updated accordingly using the
following semantic patch:
// smpl
@r@
field list[n] ds;
type T;
identifier i;
@@
struct i2c_msg {
ds
T i;
...
};
@@
initializer list[r.n] is;
expression a;
identifier nm;
identifier r.i;
@@
struct i2c_msg nm = {
is,
- a
+ .i = a
,...
};
@@
initializer list[r.n] is;
expression a;
identifier nm;
identifier r.i;
@@
struct i2c_msg nm[...] = { ..., {
is,
- a
+ .i = a
,...}, ...};
@@
initializer list[r.n] is;
expression a;
identifier nm;
identifier r.i;
@@
struct i2c_msg nm[] = { ..., {
is,
- a
+ .i = a
,...}, ...};
//
// ensure everyone has all fields, pointer case first
@rt@
type T;
identifier i;
@@
struct i2c_msg {
...
T *i;
...
};
@t1@
expression e;
identifier nm,rt.i;
position p;
@@
struct i2c_msg nm = {@p
.i = e,
};
@@
identifier nm,rt.i;
position p!= t1.p;
@@
struct i2c_msg nm = {@p
+ .i = NULL,
...
};
@t2@
expression e;
identifier nm,rt.i;
position p;
@@
struct i2c_msg nm[] = { ..., {@p
.i = e,
}, ...};
@@
identifier nm,rt.i;
position p!= t2.p;
@@
struct i2c_msg nm[] = { ..., {@p
+ .i = NULL,
...
}, ...};
@t3@
expression e;
identifier nm,rt.i;
position p;
@@
struct i2c_msg nm[...] = { ..., {@p
.i = e,
}, ...};
@@
identifier nm,rt.i;
position p!= t3.p;
@@
struct i2c_msg nm[...] = { ..., {@p
+ .i = NULL,
...
}, ...};
// -
@f1@
expression e;
identifier nm,r.i;
position p;
@@
struct i2c_msg nm = {@p
.i = e,
};
@@
identifier nm,r.i;
position p!= f1.p;
@@
struct i2c_msg nm = {@p
+ .i = 0,
...
};
@f2@
expression e;
identifier nm,r.i;
position p;
@@
struct i2c_msg nm[] = { ..., {@p
.i = e,
}, ...};
@@
identifier nm,r.i;
position p!= f2.p;
@@
struct i2c_msg nm[] = { ..., {@p
+ .i = 0,
...
}, ...};
@f3@
expression e;
identifier nm,r.i;
position p;
@@
struct i2c_msg nm[...] = { ..., {@p
.i = e,
}, ...};
@@
identifier nm,r.i;
position p!= f3.p;
@@
struct i2c_msg nm[...] = { ..., {@p
+ .i = 0,
...
}, ...};
//
@@
constant c;
identifier x;
type T,T1;
T[] b;
@@
struct i2c_msg x =
{ .buf = \((T1)b\|(T1)(b)\|(T1)(b[0])\), .len =
(
0
|
sizeof (...)
|
- c
+ sizeof(b)
)
};
@@
constant c;
identifier x;
type T,T1;
T[] b;
@@
struct i2c_msg x[...] = {...,
{ .buf = \((T1)b\|(T1)(b)\|(T1)(b[0])\), .len =
(
0
|
sizeof (...)
|
- c
+ sizeof(b)
)
} ,...};
@@
constant c;
identifier x;
type T,T1;
T[] b;
@@
struct i2c_msg x[] = {...,
{ .buf = \((T1)b\|(T1)(b)\|(T1)(b[0])\), .len =
(
0
|
sizeof (...)
|
- c
+ sizeof(b)
)
} ,...};
@@
constant c;
identifier x;
type T1;
expression b;
@@
struct i2c_msg x =
{ .buf = (T1)(b), .len =
(
0
|
sizeof (...)
|
- c
+ sizeof(b)
)
};
@@
constant c;
identifier x;
type T1;
expression b;
@@
struct i2c_msg x[...] = {...,
{ .buf = (T1)(b), .len =
(
0
|
sizeof (...)
|
- c
+ sizeof(b)
)
} ,...};
@@
constant c;
identifier x;
type T1;
expression b;
@@
struct i2c_msg x[] = {...,
{ .buf = (T1)(b), .len =
(
0
|
sizeof (...)
|
- c
+ sizeof(b)
)
} ,...};
//
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
// has to come before the next rule, which matcher fewer fields
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
//
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x[] = {...,
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
,...};
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x[] = {...,
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
,...};
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x[] = {...,
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
,...};
//
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x[...] = {...,
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
,...};
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x[...] = {...,
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
,...};
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x[...] = {...,
- {.addr = a, .buf = b, .len = c, .flags = d}
+
[PATCH 6/13] drivers/media/tuners/tda18271-common.c: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/tda18271-common.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/media/tuners/tda18271-common.c
b/drivers/media/tuners/tda18271-common.c
index 221171e..d05ed53 100644
--- a/drivers/media/tuners/tda18271-common.c
+++ b/drivers/media/tuners/tda18271-common.c
@@ -125,10 +125,8 @@ int tda18271_read_regs(struct dvb_frontend *fe)
unsigned char buf = 0x00;
int ret;
struct i2c_msg msg[] = {
- { .addr = priv-i2c_props.addr, .flags = 0,
- .buf = buf, .len = 1 },
- { .addr = priv-i2c_props.addr, .flags = I2C_M_RD,
- .buf = regs, .len = 16 }
+ I2C_MSG_WRITE(priv-i2c_props.addr, buf, sizeof(buf)),
+ I2C_MSG_READ(priv-i2c_props.addr, regs, 16)
};
tda18271_i2c_gate_ctrl(fe, 1);
@@ -155,10 +153,8 @@ int tda18271_read_extended(struct dvb_frontend *fe)
unsigned char buf = 0x00;
int ret, i;
struct i2c_msg msg[] = {
- { .addr = priv-i2c_props.addr, .flags = 0,
- .buf = buf, .len = 1 },
- { .addr = priv-i2c_props.addr, .flags = I2C_M_RD,
- .buf = regdump, .len = TDA18271_NUM_REGS }
+ I2C_MSG_WRITE(priv-i2c_props.addr, buf, sizeof(buf)),
+ I2C_MSG_READ(priv-i2c_props.addr, regdump, sizeof(regdump))
};
tda18271_i2c_gate_ctrl(fe, 1);
@@ -192,8 +188,7 @@ int tda18271_write_regs(struct dvb_frontend *fe, int idx,
int len)
struct tda18271_priv *priv = fe-tuner_priv;
unsigned char *regs = priv-tda18271_regs;
unsigned char buf[TDA18271_NUM_REGS + 1];
- struct i2c_msg msg = { .addr = priv-i2c_props.addr, .flags = 0,
- .buf = buf };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-i2c_props.addr, buf, 0);
int i, ret = 1, max;
BUG_ON((len == 0) || (idx + len sizeof(buf)));
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/13] drivers/media/tuners/tua9001.c: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/tua9001.c |7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/media/tuners/tua9001.c b/drivers/media/tuners/tua9001.c
index 3896684..a9e7e91 100644
--- a/drivers/media/tuners/tua9001.c
+++ b/drivers/media/tuners/tua9001.c
@@ -27,12 +27,7 @@ static int tua9001_wr_reg(struct tua9001_priv *priv, u8 reg,
u16 val)
int ret;
u8 buf[3] = { reg, (val 8) 0xff, (val 0) 0xff };
struct i2c_msg msg[1] = {
- {
- .addr = priv-cfg-i2c_addr,
- .flags = 0,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_addr, buf, sizeof(buf))
};
ret = i2c_transfer(priv-i2c, msg, 1);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/13] drivers/media/tuners/tda8290.c: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the bufferin each case.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/tda8290.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/media/tuners/tda8290.c b/drivers/media/tuners/tda8290.c
index 8c48521..83ecee2 100644
--- a/drivers/media/tuners/tda8290.c
+++ b/drivers/media/tuners/tda8290.c
@@ -463,7 +463,8 @@ static void tda8290_standby(struct dvb_frontend *fe)
unsigned char cb1[] = { 0x30, 0xD0 };
unsigned char tda8290_standby[] = { 0x00, 0x02 };
unsigned char tda8290_agc_tri[] = { 0x02, 0x20 };
- struct i2c_msg msg = {.addr = priv-tda827x_addr, .flags=0, .buf=cb1,
.len = 2};
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-tda827x_addr, cb1,
+ sizeof(cb1));
tda8290_i2c_bridge(fe, 1);
if (priv-ver TDA8275A)
@@ -532,8 +533,8 @@ static void tda8290_init_tuner(struct dvb_frontend *fe)
0x3F, 0x2A, 0x04, 0xFF, 0x00, 0x00,
0x40 };
unsigned char tda8275a_init[] = { 0x00, 0x00, 0x00, 0x00, 0xdC, 0x05,
0x8b,
0x0c, 0x04, 0x20, 0xFF, 0x00, 0x00,
0x4b };
- struct i2c_msg msg = {.addr = priv-tda827x_addr, .flags=0,
- .buf=tda8275_init, .len = 14};
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-tda827x_addr, tda8275_init,
+ sizeof(tda8275_init));
if (priv-ver TDA8275A)
msg.buf = tda8275a_init;
@@ -569,7 +570,7 @@ static int tda829x_find_tuner(struct dvb_frontend *fe)
int i, ret, tuners_found;
u32 tuner_addrs;
u8 data;
- struct i2c_msg msg = { .flags = I2C_M_RD, .buf = data, .len = 1 };
+ struct i2c_msg msg = I2C_MSG_READ(0, data, sizeof(data));
if (!analog_ops-i2c_gate_ctrl) {
printk(KERN_ERR tda8290: no gate control were provided!\n);
@@ -658,8 +659,8 @@ static int tda8290_probe(struct tuner_i2c_props *i2c_props)
#define TDA8290_ID 0x89
u8 reg = 0x1f, id;
struct i2c_msg msg_read[] = {
- { .addr = i2c_props-addr, .flags = 0, .len = 1, .buf = reg },
- { .addr = i2c_props-addr, .flags = I2C_M_RD, .len = 1, .buf =
id },
+ I2C_MSG_WRITE(i2c_props-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(i2c_props-addr, id, sizeof(id)),
};
/* detect tda8290 */
@@ -685,8 +686,8 @@ static int tda8295_probe(struct tuner_i2c_props *i2c_props)
#define TDA8295C2_ID 0x8b
u8 reg = 0x2f, id;
struct i2c_msg msg_read[] = {
- { .addr = i2c_props-addr, .flags = 0, .len = 1, .buf = reg },
- { .addr = i2c_props-addr, .flags = I2C_M_RD, .len = 1, .buf =
id },
+ I2C_MSG_WRITE(i2c_props-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(i2c_props-addr, id, sizeof(id)),
};
/* detect tda8295 */
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/13] drivers/media/tuners/fc0011.c: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer in each case.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/fc0011.c |9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/media/tuners/fc0011.c b/drivers/media/tuners/fc0011.c
index e488254..5dbba98 100644
--- a/drivers/media/tuners/fc0011.c
+++ b/drivers/media/tuners/fc0011.c
@@ -80,8 +80,7 @@ struct fc0011_priv {
static int fc0011_writereg(struct fc0011_priv *priv, u8 reg, u8 val)
{
u8 buf[2] = { reg, val };
- struct i2c_msg msg = { .addr = priv-addr,
- .flags = 0, .buf = buf, .len = 2 };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-addr, buf, sizeof(buf));
if (i2c_transfer(priv-i2c, msg, 1) != 1) {
dev_err(priv-i2c-dev,
@@ -97,10 +96,8 @@ static int fc0011_readreg(struct fc0011_priv *priv, u8 reg,
u8 *val)
{
u8 dummy;
struct i2c_msg msg[2] = {
- { .addr = priv-addr,
- .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-addr,
- .flags = I2C_M_RD, .buf = val ? : dummy, .len = 1 },
+ I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-addr, val ? : dummy, sizeof(dummy)),
};
if (i2c_transfer(priv-i2c, msg, 2) != 2) {
--
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 11/13] drivers/media/tuners/tda18218.c: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/tda18218.c | 19 +++
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/drivers/media/tuners/tda18218.c b/drivers/media/tuners/tda18218.c
index 1819853..c0c2fef 100644
--- a/drivers/media/tuners/tda18218.c
+++ b/drivers/media/tuners/tda18218.c
@@ -26,11 +26,7 @@ static int tda18218_wr_regs(struct tda18218_priv *priv, u8
reg, u8 *val, u8 len)
int ret = 0, len2, remaining;
u8 buf[1 + len];
struct i2c_msg msg[1] = {
- {
- .addr = priv-cfg-i2c_address,
- .flags = 0,
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_address, buf, 0)
};
for (remaining = len; remaining 0;
@@ -65,17 +61,8 @@ static int tda18218_rd_regs(struct tda18218_priv *priv, u8
reg, u8 *val, u8 len)
int ret;
u8 buf[reg+len]; /* we must start read always from reg 0x00 */
struct i2c_msg msg[2] = {
- {
- .addr = priv-cfg-i2c_address,
- .flags = 0,
- .len = 1,
- .buf = \x00,
- }, {
- .addr = priv-cfg-i2c_address,
- .flags = I2C_M_RD,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_address, \x00, 1),
+ I2C_MSG_READ(priv-cfg-i2c_address, buf, sizeof(buf))
};
ret = i2c_transfer(priv-i2c, msg, 2);
--
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 8/13] drivers/media/tuners/fc2580.c: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer in each case.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/fc2580.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/media/tuners/fc2580.c b/drivers/media/tuners/fc2580.c
index aff39ae..d0c0ff1 100644
--- a/drivers/media/tuners/fc2580.c
+++ b/drivers/media/tuners/fc2580.c
@@ -45,12 +45,7 @@ static int fc2580_wr_regs(struct fc2580_priv *priv, u8 reg,
u8 *val, int len)
int ret;
u8 buf[1 + len];
struct i2c_msg msg[1] = {
- {
- .addr = priv-cfg-i2c_addr,
- .flags = 0,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_addr, buf, sizeof(buf))
};
buf[0] = reg;
@@ -73,17 +68,8 @@ static int fc2580_rd_regs(struct fc2580_priv *priv, u8 reg,
u8 *val, int len)
int ret;
u8 buf[len];
struct i2c_msg msg[2] = {
- {
- .addr = priv-cfg-i2c_addr,
- .flags = 0,
- .len = 1,
- .buf = reg,
- }, {
- .addr = priv-cfg-i2c_addr,
- .flags = I2C_M_RD,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-cfg-i2c_addr, buf, sizeof(buf))
};
ret = i2c_transfer(priv-i2c, msg, 2);
--
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 12/13] drivers/media/tuners/max2165.c: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer, when this is possible.
The second case is simplified to use simple variables rather than arrays.
The variable b0 is dropped completely, and the variable reg that it
contains is used instead. The variable b1 is replaced by a u8-typed
variable named buf (the name used earlier in the file). The uses of b1 are
then adjusted accordingly.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/max2165.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/media/tuners/max2165.c b/drivers/media/tuners/max2165.c
index ba84936..6638617 100644
--- a/drivers/media/tuners/max2165.c
+++ b/drivers/media/tuners/max2165.c
@@ -47,7 +47,7 @@ static int max2165_write_reg(struct max2165_priv *priv, u8
reg, u8 data)
{
int ret;
u8 buf[] = { reg, data };
- struct i2c_msg msg = { .flags = 0, .buf = buf, .len = 2 };
+ struct i2c_msg msg = I2C_MSG_WRITE(0, buf, sizeof(buf));
msg.addr = priv-config-i2c_address;
@@ -68,11 +68,10 @@ static int max2165_read_reg(struct max2165_priv *priv, u8
reg, u8 *p_data)
int ret;
u8 dev_addr = priv-config-i2c_address;
- u8 b0[] = { reg };
- u8 b1[] = { 0 };
+ u8 buf;
struct i2c_msg msg[] = {
- { .addr = dev_addr, .flags = 0, .buf = b0, .len = 1 },
- { .addr = dev_addr, .flags = I2C_M_RD, .buf = b1, .len = 1 },
+ I2C_MSG_WRITE(dev_addr, reg, sizeof(reg)),
+ I2C_MSG_READ(dev_addr, buf, sizeof(buf)),
};
ret = i2c_transfer(priv-i2c, msg, 2);
@@ -81,10 +80,10 @@ static int max2165_read_reg(struct max2165_priv *priv, u8
reg, u8 *p_data)
return -EIO;
}
- *p_data = b1[0];
+ *p_data = buf;
if (debug = 2)
dprintk(%s: reg=0x%02X, data=0x%02X\n,
- __func__, reg, b1[0]);
+ __func__, reg, buf);
return 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
[PATCH 5/13] drivers/media/tuners: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer, when this is possible.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/fc0012.c |8 +++-
drivers/media/tuners/fc0013.c |8 +++-
drivers/media/tuners/mc44s803.c |8 +++-
drivers/media/tuners/mt2060.c | 13 +
drivers/media/tuners/mt2063.c | 23 ++-
drivers/media/tuners/mt2131.c | 13 +
drivers/media/tuners/mt2266.c | 13 +
drivers/media/tuners/mxl5005s.c |8
drivers/media/tuners/tda827x.c | 29 +++--
drivers/media/tuners/tuner-i2c.h| 12
drivers/media/tuners/tuner-simple.c |5 +
drivers/media/tuners/xc4000.c |9 +++--
drivers/media/tuners/xc5000.c |9 +++--
13 files changed, 56 insertions(+), 102 deletions(-)
diff --git a/drivers/media/tuners/fc0012.c b/drivers/media/tuners/fc0012.c
index 308135a..01dac7e 100644
--- a/drivers/media/tuners/fc0012.c
+++ b/drivers/media/tuners/fc0012.c
@@ -24,9 +24,7 @@
static int fc0012_writereg(struct fc0012_priv *priv, u8 reg, u8 val)
{
u8 buf[2] = {reg, val};
- struct i2c_msg msg = {
- .addr = priv-addr, .flags = 0, .buf = buf, .len = 2
- };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-addr, buf, sizeof(buf));
if (i2c_transfer(priv-i2c, msg, 1) != 1) {
err(I2C write reg failed, reg: %02x, val: %02x, reg, val);
@@ -38,8 +36,8 @@ static int fc0012_writereg(struct fc0012_priv *priv, u8 reg,
u8 val)
static int fc0012_readreg(struct fc0012_priv *priv, u8 reg, u8 *val)
{
struct i2c_msg msg[2] = {
- { .addr = priv-addr, .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-addr, .flags = I2C_M_RD, .buf = val, .len = 1 },
+ I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-addr, val, 1),
};
if (i2c_transfer(priv-i2c, msg, 2) != 2) {
diff --git a/drivers/media/tuners/fc0013.c b/drivers/media/tuners/fc0013.c
index bd8f0f1..174f0b0 100644
--- a/drivers/media/tuners/fc0013.c
+++ b/drivers/media/tuners/fc0013.c
@@ -27,9 +27,7 @@
static int fc0013_writereg(struct fc0013_priv *priv, u8 reg, u8 val)
{
u8 buf[2] = {reg, val};
- struct i2c_msg msg = {
- .addr = priv-addr, .flags = 0, .buf = buf, .len = 2
- };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-addr, buf, sizeof(buf));
if (i2c_transfer(priv-i2c, msg, 1) != 1) {
err(I2C write reg failed, reg: %02x, val: %02x, reg, val);
@@ -41,8 +39,8 @@ static int fc0013_writereg(struct fc0013_priv *priv, u8 reg,
u8 val)
static int fc0013_readreg(struct fc0013_priv *priv, u8 reg, u8 *val)
{
struct i2c_msg msg[2] = {
- { .addr = priv-addr, .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-addr, .flags = I2C_M_RD, .buf = val, .len = 1 },
+ I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-addr, val, 1),
};
if (i2c_transfer(priv-i2c, msg, 2) != 2) {
diff --git a/drivers/media/tuners/mc44s803.c b/drivers/media/tuners/mc44s803.c
index f1b7640..df47e03 100644
--- a/drivers/media/tuners/mc44s803.c
+++ b/drivers/media/tuners/mc44s803.c
@@ -37,9 +37,8 @@
static int mc44s803_writereg(struct mc44s803_priv *priv, u32 val)
{
u8 buf[3];
- struct i2c_msg msg = {
- .addr = priv-cfg-i2c_address, .flags = 0, .buf = buf, .len = 3
- };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-cfg-i2c_address, buf,
+ sizeof(buf));
buf[0] = (val 0xff) 16;
buf[1] = (val 0xff00) 8;
@@ -59,8 +58,7 @@ static int mc44s803_readreg(struct mc44s803_priv *priv, u8
reg, u32 *val)
u8 buf[3];
int ret;
struct i2c_msg msg[] = {
- { .addr = priv-cfg-i2c_address, .flags = I2C_M_RD,
- .buf = buf, .len = 3 },
+ I2C_MSG_READ(priv-cfg-i2c_address, buf, sizeof(buf)),
};
wval = MC44S803_REG_SM(MC44S803_REG_DATAREG, MC44S803_ADDR) |
diff --git a/drivers/media/tuners/mt2060.c b/drivers/media/tuners/mt2060.c
index 13381de..5fb2e77
[PATCH 3/13] drivers/media/tuners/qt1010.c: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer, when this is possible.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/qt1010.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/media/tuners/qt1010.c b/drivers/media/tuners/qt1010.c
index bc419f8..37ff254 100644
--- a/drivers/media/tuners/qt1010.c
+++ b/drivers/media/tuners/qt1010.c
@@ -25,10 +25,8 @@
static int qt1010_readreg(struct qt1010_priv *priv, u8 reg, u8 *val)
{
struct i2c_msg msg[2] = {
- { .addr = priv-cfg-i2c_address,
- .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-cfg-i2c_address,
- .flags = I2C_M_RD, .buf = val, .len = 1 },
+ I2C_MSG_WRITE(priv-cfg-i2c_address, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-cfg-i2c_address, val, 1),
};
if (i2c_transfer(priv-i2c, msg, 2) != 2) {
@@ -43,8 +41,8 @@ static int qt1010_readreg(struct qt1010_priv *priv, u8 reg,
u8 *val)
static int qt1010_writereg(struct qt1010_priv *priv, u8 reg, u8 val)
{
u8 buf[2] = { reg, val };
- struct i2c_msg msg = { .addr = priv-cfg-i2c_address,
- .flags = 0, .buf = buf, .len = 2 };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-cfg-i2c_address, buf,
+ sizeof(buf));
if (i2c_transfer(priv-i2c, msg, 1) != 1) {
dev_warn(priv-i2c-dev, %s: i2c wr failed reg=%02x\n,
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/13] drivers/media/tuners/tda18212.c: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
In the second initialization, a length expressed as an explicit constant is
also re-expressed as the size of the buffer (reg).
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/tda18212.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c
index 5d9f028..fb810ce 100644
--- a/drivers/media/tuners/tda18212.c
+++ b/drivers/media/tuners/tda18212.c
@@ -34,12 +34,7 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8
reg, u8 *val,
int ret;
u8 buf[len+1];
struct i2c_msg msg[1] = {
- {
- .addr = priv-cfg-i2c_address,
- .flags = 0,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_address, buf, sizeof(buf))
};
buf[0] = reg;
@@ -63,17 +58,8 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8
reg, u8 *val,
int ret;
u8 buf[len];
struct i2c_msg msg[2] = {
- {
- .addr = priv-cfg-i2c_address,
- .flags = 0,
- .len = 1,
- .buf = reg,
- }, {
- .addr = priv-cfg-i2c_address,
- .flags = I2C_M_RD,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_address, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-cfg-i2c_address, buf, sizeof(buf))
};
ret = i2c_transfer(priv-i2c, msg, 2);
--
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/13] drivers/media/tuners/mxl5007t.c: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
In each case, a length expressed as an explicit constant is also
re-expressed as the size of the buffer, when this is possible.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/mxl5007t.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/media/tuners/mxl5007t.c b/drivers/media/tuners/mxl5007t.c
index 69e453e..c0c28be 100644
--- a/drivers/media/tuners/mxl5007t.c
+++ b/drivers/media/tuners/mxl5007t.c
@@ -464,8 +464,8 @@ reg_pair_t *mxl5007t_calc_rf_tune_regs(struct
mxl5007t_state *state,
static int mxl5007t_write_reg(struct mxl5007t_state *state, u8 reg, u8 val)
{
u8 buf[] = { reg, val };
- struct i2c_msg msg = { .addr = state-i2c_props.addr, .flags = 0,
- .buf = buf, .len = 2 };
+ struct i2c_msg msg = I2C_MSG_WRITE(state-i2c_props.addr, buf,
+ sizeof(buf));
int ret;
ret = i2c_transfer(state-i2c_props.adap, msg, 1);
@@ -494,10 +494,8 @@ static int mxl5007t_read_reg(struct mxl5007t_state *state,
u8 reg, u8 *val)
{
u8 buf[2] = { 0xfb, reg };
struct i2c_msg msg[] = {
- { .addr = state-i2c_props.addr, .flags = 0,
- .buf = buf, .len = 2 },
- { .addr = state-i2c_props.addr, .flags = I2C_M_RD,
- .buf = val, .len = 1 },
+ I2C_MSG_WRITE(state-i2c_props.addr, buf, sizeof(buf)),
+ I2C_MSG_READ(state-i2c_props.addr, val, 1),
};
int ret;
@@ -512,10 +510,8 @@ static int mxl5007t_read_reg(struct mxl5007t_state *state,
u8 reg, u8 *val)
static int mxl5007t_soft_reset(struct mxl5007t_state *state)
{
u8 d = 0xff;
- struct i2c_msg msg = {
- .addr = state-i2c_props.addr, .flags = 0,
- .buf = d, .len = 1
- };
+ struct i2c_msg msg = I2C_MSG_WRITE(state-i2c_props.addr, d,
+ sizeof(d));
int ret = i2c_transfer(state-i2c_props.adap, msg, 1);
if (ret != 1) {
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
In the second i2c_msg structure, a length expressed as an explicit constant
is also re-expressed as the size of the buffer, reg.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/e4000.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
index 1b33ed3..8f182fc 100644
--- a/drivers/media/tuners/e4000.c
+++ b/drivers/media/tuners/e4000.c
@@ -26,12 +26,7 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8
*val, int len)
int ret;
u8 buf[1 + len];
struct i2c_msg msg[1] = {
- {
- .addr = priv-cfg-i2c_addr,
- .flags = 0,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_addr, buf, sizeof(buf))
};
buf[0] = reg;
@@ -54,17 +49,8 @@ static int e4000_rd_regs(struct e4000_priv *priv, u8 reg, u8
*val, int len)
int ret;
u8 buf[len];
struct i2c_msg msg[2] = {
- {
- .addr = priv-cfg-i2c_addr,
- .flags = 0,
- .len = 1,
- .buf = reg,
- }, {
- .addr = priv-cfg-i2c_addr,
- .flags = I2C_M_RD,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-cfg-i2c_addr, buf, sizeof(buf))
};
ret = i2c_transfer(priv-i2c, msg, 2);
--
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
Zolid USB DVB-T Tuner Pictures
Hi... I saw on this page... http://linuxtv.org/wiki/index.php/DVB-T_USB_Devices ...That I can contribute to the project by writing to this list. Now, I don't have much knowledge about USB; I don't even have Linux (but I probably will within a few months). I saw that some of the mentioned devices on the above page, are missing a picture. So what I can do, is that I have a Zolid USB DVB-T Tuner bought from Aldi - like they all are. I've taken some pictures, cut them in Photoshop, scaled, saved as png and finally optimized them using pngout. Sizes are: Approx. 2100x500 for the originals, 1024x500..600 for the large ones, 512x190..300 for medium-size, 128x51..80 for the smaller ones. (Whoa, 5 hours work for 5 pictures!) Note: This is only one device, it seems a little difficult to figure out which version it is, but as I have the original box and a USB-Probe dump, it might be possible to identify it fully. What I can say, is that it uses the IT9135 chip. VID/PID 0x048D/00x9135. Descriptor Version Number is 0x0200. Device MaxPacketSize is 64 (see below) Device Version Number is 0x0200 It has two configurations, each configuration has 4 interfaces. The first configuration's interfaces have a max packet size of 512 The second configuration's interfaces have a max packet size of 64. Apart from that, the configurations match eachother. -So my guess is that this is a v2 device. When looking at the above mentioned page, and I search the table for 'Zolid', I find an entry saying ITE Inc. Zolid Mini DVB-T Stick Version 2. My box says Mini USB DVB-T Tuner and the markings on the device just says SMART GROUP Made in Taiwan, www.unisupport.net, PS0712 and 05/2011. (In fact, I bought exactly this device, because I believe this is the one that's listed here!) ...Now...Who wants those pictures ? :) Love Jens -- 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: How to add support for the em2765 webcam Speedlink VAD Laplace to the kernel ?
Em Sun, 07 Oct 2012 15:41:50 +0200
Frank Schäfer fschaefer@googlemail.com escreveu:
Am 06.10.2012 13:56, schrieb Mauro Carvalho Chehab:
Em Sun, 23 Sep 2012 16:03:25 +0200
Frank Schäfer fschaefer@googlemail.com escreveu:
Ping !
Sorry, too busy those days.
No problem.
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.
Ok.
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...
AFAIKT, newer em28xx chips are using this concept. The em28xx-i2c code
require
changes to support two I2C buses, and to handle 16 bit eeproms. We never
cared
of doing that because we never needed, so far, to read anything from those
devices' eeproms.
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
Re: [PATCH 2/5] fence: dma-buf cross-device synchronization (v9)
Op 28-09-12 14:42, Maarten Lankhorst schreef: A fence can be attached to a buffer which is being filled or consumed by hw, to allow userspace to pass the buffer without waiting to another device. For example, userspace can call page_flip ioctl to display the next frame of graphics after kicking the GPU but while the GPU is still rendering. The display device sharing the buffer with the GPU would attach a callback to get notified when the GPU's rendering-complete IRQ fires, to update the scan-out address of the display, without having to wake up userspace. A fence is transient, one-shot deal. It is allocated and attached to one or more dma-buf's. When the one that attached it is done, with the pending operation, it can signal the fence: + fence_signal() To have a rough approximation whether a fence is fired, call: + fence_is_signaled() The dma-buf-mgr handles tracking, and waiting on, the fences associated with a dma-buf. The one pending on the fence can add an async callback: + fence_add_callback() The callback can optionally be cancelled with: + fence_remove_callback() To wait synchronously, optionally with a timeout: + fence_wait() + fence_wait_timeout() ... Implementing this makes the locking feel weird, instead of abusing fence-event_queue.lock I think it would make sense to remove this part entirely, and have a simply linked list plus a pointer to a spinlock. enable_signaling should be called with this spinlock held. This way, the enable_signaling callback would be easier to implement and doesn't have to double check for races as much in there. Also __fence_signal should be exported which would be the same as fence_signal, but without taking the spinlock as separate step, so it can be called with the spinlock held. How do you feel about these proposed changes? ~Maarten -- 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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
Am 07.10.2012 17:38, schrieb Julia Lawall:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
In the second i2c_msg structure, a length expressed as an explicit constant
is also re-expressed as the size of the buffer, reg.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/e4000.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
index 1b33ed3..8f182fc 100644
--- a/drivers/media/tuners/e4000.c
+++ b/drivers/media/tuners/e4000.c
@@ -26,12 +26,7 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg,
u8 *val, int len)
int ret;
u8 buf[1 + len];
struct i2c_msg msg[1] = {
- {
- .addr = priv-cfg-i2c_addr,
- .flags = 0,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_addr, buf, sizeof(buf))
};
Any reason why struct i2c_msg is an array ?
re,
wh
buf[0] = reg;
@@ -54,17 +49,8 @@ static int e4000_rd_regs(struct e4000_priv *priv, u8 reg,
u8 *val, int len)
int ret;
u8 buf[len];
struct i2c_msg msg[2] = {
- {
- .addr = priv-cfg-i2c_addr,
- .flags = 0,
- .len = 1,
- .buf = reg,
- }, {
- .addr = priv-cfg-i2c_addr,
- .flags = I2C_M_RD,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-cfg-i2c_addr, buf, sizeof(buf))
};
ret = i2c_transfer(priv-i2c, msg, 2);
--
To unsubscribe from this list: send the line unsubscribe kernel-janitors 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 9/13] drivers/media/tuners/fc0011.c: use macros for i2c_msg initialization
Am 07.10.2012 17:38, schrieb Julia Lawall:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer in each case.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/fc0011.c |9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/media/tuners/fc0011.c b/drivers/media/tuners/fc0011.c
index e488254..5dbba98 100644
--- a/drivers/media/tuners/fc0011.c
+++ b/drivers/media/tuners/fc0011.c
@@ -80,8 +80,7 @@ struct fc0011_priv {
static int fc0011_writereg(struct fc0011_priv *priv, u8 reg, u8 val)
{
u8 buf[2] = { reg, val };
- struct i2c_msg msg = { .addr = priv-addr,
- .flags = 0, .buf = buf, .len = 2 };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-addr, buf, sizeof(buf));
if (i2c_transfer(priv-i2c, msg, 1) != 1) {
dev_err(priv-i2c-dev,
@@ -97,10 +96,8 @@ static int fc0011_readreg(struct fc0011_priv *priv, u8
reg, u8 *val)
{
u8 dummy;
struct i2c_msg msg[2] = {
- { .addr = priv-addr,
- .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-addr,
- .flags = I2C_M_RD, .buf = val ? : dummy, .len = 1 },
+ I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-addr, val ? : dummy, sizeof(dummy)),
};
This dummy looks strange, can it be that this is used uninitialised ?
re,
wh
if (i2c_transfer(priv-i2c, msg, 2) != 2) {
--
To unsubscribe from this list: send the line unsubscribe kernel-janitors 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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
On Sun, 7 Oct 2012, walter harms wrote:
Am 07.10.2012 17:38, schrieb Julia Lawall:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
In the second i2c_msg structure, a length expressed as an explicit constant
is also re-expressed as the size of the buffer, reg.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/e4000.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
index 1b33ed3..8f182fc 100644
--- a/drivers/media/tuners/e4000.c
+++ b/drivers/media/tuners/e4000.c
@@ -26,12 +26,7 @@ static int e4000_wr_regs(struct e4000_priv *priv, u8 reg, u8
*val, int len)
int ret;
u8 buf[1 + len];
struct i2c_msg msg[1] = {
- {
- .addr = priv-cfg-i2c_addr,
- .flags = 0,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_addr, buf, sizeof(buf))
};
Any reason why struct i2c_msg is an array ?
I assumed that it looked more harmonious with the other uses of
i2c_transfer, which takes as arguments an array and the number of
elements.
But there are some files that instead use i2c_transfer(priv-i2c, msg, 1).
I can change them all to do that if that is preferred. But maybe I
will wait a little bit to see if there are other issues to address at
the same time.
thanks,
julia
re,
wh
buf[0] = reg;
@@ -54,17 +49,8 @@ static int e4000_rd_regs(struct e4000_priv *priv, u8 reg, u8
*val, int len)
int ret;
u8 buf[len];
struct i2c_msg msg[2] = {
- {
- .addr = priv-cfg-i2c_addr,
- .flags = 0,
- .len = 1,
- .buf = reg,
- }, {
- .addr = priv-cfg-i2c_addr,
- .flags = I2C_M_RD,
- .len = sizeof(buf),
- .buf = buf,
- }
+ I2C_MSG_WRITE(priv-cfg-i2c_addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-cfg-i2c_addr, buf, sizeof(buf))
};
ret = i2c_transfer(priv-i2c, msg, 2);
--
To unsubscribe from this list: send the line unsubscribe kernel-janitors 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 kernel-janitors 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 9/13] drivers/media/tuners/fc0011.c: use macros for i2c_msg initialization
On Sun, 7 Oct 2012, walter harms wrote:
Am 07.10.2012 17:38, schrieb Julia Lawall:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer in each case.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/fc0011.c |9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/media/tuners/fc0011.c b/drivers/media/tuners/fc0011.c
index e488254..5dbba98 100644
--- a/drivers/media/tuners/fc0011.c
+++ b/drivers/media/tuners/fc0011.c
@@ -80,8 +80,7 @@ struct fc0011_priv {
static int fc0011_writereg(struct fc0011_priv *priv, u8 reg, u8 val)
{
u8 buf[2] = { reg, val };
- struct i2c_msg msg = { .addr = priv-addr,
- .flags = 0, .buf = buf, .len = 2 };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-addr, buf, sizeof(buf));
if (i2c_transfer(priv-i2c, msg, 1) != 1) {
dev_err(priv-i2c-dev,
@@ -97,10 +96,8 @@ static int fc0011_readreg(struct fc0011_priv *priv, u8 reg,
u8 *val)
{
u8 dummy;
struct i2c_msg msg[2] = {
- { .addr = priv-addr,
- .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-addr,
- .flags = I2C_M_RD, .buf = val ? : dummy, .len = 1 },
+ I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-addr, val ? : dummy, sizeof(dummy)),
};
This dummy looks strange, can it be that this is used uninitialised ?
I'm not sure to understand the question. The read, when it happens in
i2c_transfer will initialize dummy. On the other hand, I don't know what
i2c_transfer does when the buffer is NULL and the size is 1. It does not
look very elegant at least.
julia
--
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 9/13] drivers/media/tuners/fc0011.c: use macros for i2c_msg initialization
Am 07.10.2012 18:50, schrieb Julia Lawall:
On Sun, 7 Oct 2012, walter harms wrote:
Am 07.10.2012 17:38, schrieb Julia Lawall:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as
the size
of the buffer in each case.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/fc0011.c |9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/media/tuners/fc0011.c
b/drivers/media/tuners/fc0011.c
index e488254..5dbba98 100644
--- a/drivers/media/tuners/fc0011.c
+++ b/drivers/media/tuners/fc0011.c
@@ -80,8 +80,7 @@ struct fc0011_priv {
static int fc0011_writereg(struct fc0011_priv *priv, u8 reg, u8 val)
{
u8 buf[2] = { reg, val };
-struct i2c_msg msg = { .addr = priv-addr,
-.flags = 0, .buf = buf, .len = 2 };
+struct i2c_msg msg = I2C_MSG_WRITE(priv-addr, buf, sizeof(buf));
if (i2c_transfer(priv-i2c, msg, 1) != 1) {
dev_err(priv-i2c-dev,
@@ -97,10 +96,8 @@ static int fc0011_readreg(struct fc0011_priv
*priv, u8 reg, u8 *val)
{
u8 dummy;
struct i2c_msg msg[2] = {
-{ .addr = priv-addr,
- .flags = 0, .buf = reg, .len = 1 },
-{ .addr = priv-addr,
- .flags = I2C_M_RD, .buf = val ? : dummy, .len = 1 },
+I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
+I2C_MSG_READ(priv-addr, val ? : dummy, sizeof(dummy)),
};
This dummy looks strange, can it be that this is used uninitialised ?
I'm not sure to understand the question. The read, when it happens in
i2c_transfer will initialize dummy. On the other hand, I don't know
what i2c_transfer does when the buffer is NULL and the size is 1. It
does not look very elegant at least.
mea culpa, i mixed read and write
re,
wh
julia
--
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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
Am 07.10.2012 18:44, schrieb Julia Lawall:
On Sun, 7 Oct 2012, walter harms wrote:
Am 07.10.2012 17:38, schrieb Julia Lawall:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
In the second i2c_msg structure, a length expressed as an explicit
constant
is also re-expressed as the size of the buffer, reg.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/e4000.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
index 1b33ed3..8f182fc 100644
--- a/drivers/media/tuners/e4000.c
+++ b/drivers/media/tuners/e4000.c
@@ -26,12 +26,7 @@ static int e4000_wr_regs(struct e4000_priv *priv,
u8 reg, u8 *val, int len)
int ret;
u8 buf[1 + len];
struct i2c_msg msg[1] = {
-{
-.addr = priv-cfg-i2c_addr,
-.flags = 0,
-.len = sizeof(buf),
-.buf = buf,
-}
+I2C_MSG_WRITE(priv-cfg-i2c_addr, buf, sizeof(buf))
};
Any reason why struct i2c_msg is an array ?
I assumed that it looked more harmonious with the other uses of
i2c_transfer, which takes as arguments an array and the number of elements.
But there are some files that instead use i2c_transfer(priv-i2c, msg, 1).
I can change them all to do that if that is preferred. But maybe I will
wait a little bit to see if there are other issues to address at the
same time.
thanks,
julia
Hi Julia,
please be aware i am not the maintainer only a distant watcher :)
do you really thing that a macro is appropriate here ? I feel uneasy about it
but i can not offer an other solution.
nothing to worry about,
just my 2 cents.
re,
wh
re,
wh
buf[0] = reg;
@@ -54,17 +49,8 @@ static int e4000_rd_regs(struct e4000_priv *priv,
u8 reg, u8 *val, int len)
int ret;
u8 buf[len];
struct i2c_msg msg[2] = {
-{
-.addr = priv-cfg-i2c_addr,
-.flags = 0,
-.len = 1,
-.buf = reg,
-}, {
-.addr = priv-cfg-i2c_addr,
-.flags = I2C_M_RD,
-.len = sizeof(buf),
-.buf = buf,
-}
+I2C_MSG_WRITE(priv-cfg-i2c_addr, reg, sizeof(reg)),
+I2C_MSG_READ(priv-cfg-i2c_addr, buf, sizeof(buf))
};
ret = i2c_transfer(priv-i2c, msg, 2);
--
To unsubscribe from this list: send the line unsubscribe
kernel-janitors 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
kernel-janitors 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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
On Sun, 7 Oct 2012, walter harms wrote:
Am 07.10.2012 18:44, schrieb Julia Lawall:
On Sun, 7 Oct 2012, walter harms wrote:
Am 07.10.2012 17:38, schrieb Julia Lawall:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
In the second i2c_msg structure, a length expressed as an explicit
constant
is also re-expressed as the size of the buffer, reg.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/e4000.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
index 1b33ed3..8f182fc 100644
--- a/drivers/media/tuners/e4000.c
+++ b/drivers/media/tuners/e4000.c
@@ -26,12 +26,7 @@ static int e4000_wr_regs(struct e4000_priv *priv,
u8 reg, u8 *val, int len)
int ret;
u8 buf[1 + len];
struct i2c_msg msg[1] = {
-{
-.addr = priv-cfg-i2c_addr,
-.flags = 0,
-.len = sizeof(buf),
-.buf = buf,
-}
+I2C_MSG_WRITE(priv-cfg-i2c_addr, buf, sizeof(buf))
};
Any reason why struct i2c_msg is an array ?
I assumed that it looked more harmonious with the other uses of
i2c_transfer, which takes as arguments an array and the number of elements.
But there are some files that instead use i2c_transfer(priv-i2c, msg, 1).
I can change them all to do that if that is preferred. But maybe I will
wait a little bit to see if there are other issues to address at the
same time.
thanks,
julia
Hi Julia,
please be aware i am not the maintainer only a distant watcher :)
do you really thing that a macro is appropriate here ? I feel uneasy about it
but i can not offer an other solution.
Some people thought that it would be nice to have the macros rather than
the inlined field initializations, especially since there is no flag for
write. A separate question is whether an array of one element is useful,
or whether one should systematically use on a simple variable of the
structure type. I'm open to suggestions about either point.
julia
--
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
ir-keytable: Bug in gen_keytables.pl script
Hello,
I recently received this launchpad bug:
https://bugs.launchpad.net/ubuntu/+source/v4l-utils/+bug/1054122
It seems that the mentioned key mappings are missing.
If you check the generated mapping file
http://git.linuxtv.org/v4l-utils.git/blob/HEAD:/utils/keytable/rc_keymaps/imon_pad
and compare it to the driver file
http://git.linuxtv.org/media_tree.git/blob/refs/heads/staging/for_v3.7:/drivers/media/rc/keymaps/rc-imon-pad.c#l111
you'll notice that the parsing stopped at the BTN_xyz table entries:
{ 0x299115b7, KEY_KEYBOARD },
{ 0x299135b7, KEY_KEYBOARD },
processing stopped here
{ 0x0101, BTN_LEFT },
{ 0x0102, BTN_RIGHT },
{ 0x01010080, BTN_LEFT },
{ 0x01020080, BTN_RIGHT },
{ 0x688301b7, BTN_LEFT },
{ 0x688481b7, BTN_RIGHT },
{ 0x2a9395b7, KEY_CYCLEWINDOWS }, /* TaskSwitcher */
{ 0x2b8395b7, KEY_TIME }, /* Timer */
Mauro, could you please take a look? I guess the BTN_xyz entries should
be also added to the keytable files. Unfortunately my Perl skills are
horrible.
Thanks,
Gregor
--
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 Oct 7 19:00:25 CEST 2012 git hash:d92462401dde1effa04a51d0a15000e6246d2a43 gcc version: i686-linux-gcc (GCC) 4.7.1 host hardware:x86_64 host os: 3.4.07-marune linux-git-arm-eabi-davinci: ERRORS linux-git-arm-eabi-exynos: WARNINGS linux-git-arm-eabi-omap: ERRORS linux-git-i686: WARNINGS linux-git-m32r: WARNINGS linux-git-mips: WARNINGS linux-git-powerpc64: OK linux-git-sh: WARNINGS 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: ERRORS linux-2.6.38.2-x86_64: ERRORS linux-2.6.39.1-x86_64: ERRORS linux-3.0-x86_64: ERRORS linux-3.1-x86_64: ERRORS linux-3.2.1-x86_64: ERRORS linux-3.3-x86_64: ERRORS linux-3.4-x86_64: ERRORS linux-3.5-x86_64: ERRORS linux-3.6-x86_64: ERRORS 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: ERRORS linux-2.6.38.2-i686: ERRORS linux-2.6.39.1-i686: ERRORS linux-3.0-i686: ERRORS linux-3.1-i686: ERRORS linux-3.2.1-i686: ERRORS linux-3.3-i686: ERRORS linux-3.4-i686: ERRORS linux-3.5-i686: ERRORS linux-3.6-i686: ERRORS 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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
On Sun, 2012-10-07 at 19:18 +0200, Julia Lawall wrote:
On Sun, 7 Oct 2012, walter harms wrote:
Am 07.10.2012 18:44, schrieb Julia Lawall:
On Sun, 7 Oct 2012, walter harms wrote:
Am 07.10.2012 17:38, schrieb Julia Lawall:
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
[]
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
[]
do you really thing that a macro is appropriate here ? I feel uneasy about
it
but i can not offer an other solution.
I think the macros are fine.
Some people thought that it would be nice to have the macros rather than
the inlined field initializations, especially since there is no flag for
write. A separate question is whether an array of one element is useful,
or whether one should systematically use on a simple variable of the
structure type. I'm open to suggestions about either point.
I think the macro naming is not great.
Maybe add DEFINE_/DECLARE_/_INIT or something other than an action
name type to the macro names.
I think the consistency is better if all the references are done
as arrays, even for single entry arrays.
It's all quibbling in any case.
--
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 9/13] drivers/media/tuners/fc0011.c: use macros for i2c_msg initialization
On Sun, 7 Oct 2012 18:50:31 +0200 (CEST)
Julia Lawall julia.law...@lip6.fr wrote:
@@ -97,10 +96,8 @@ static int fc0011_readreg(struct fc0011_priv *priv, u8
reg, u8 *val)
{
u8 dummy;
struct i2c_msg msg[2] = {
- { .addr = priv-addr,
-.flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-addr,
-.flags = I2C_M_RD, .buf = val ? : dummy, .len = 1 },
+ I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-addr, val ? : dummy, sizeof(dummy)),
};
This dummy looks strange, can it be that this is used uninitialised ?
I'm not sure to understand the question. The read, when it happens in
i2c_transfer will initialize dummy. On the other hand, I don't know what
i2c_transfer does when the buffer is NULL and the size is 1. It does not
look very elegant at least.
Well its use case is if you only care about the side effects and not the actual
data
returned by the read operation.
--
Greetings, Michael.
PGP encryption is encouraged / 908D8B0E
signature.asc
Description: PGP signature
[v4l-utils] Use RCC variable to call rcc compiler
In cross compile environment rcc native version may be staged in a different directory or even called rcc4 or somesuch. Lets provide a facility to specify it in environment Signed-off-by: Khem Raj raj.k...@gmail.com --- utils/qv4l2/Makefile.am |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/qv4l2/Makefile.am b/utils/qv4l2/Makefile.am index 02d0bcb..86d0285 100644 --- a/utils/qv4l2/Makefile.am +++ b/utils/qv4l2/Makefile.am @@ -29,7 +29,7 @@ moc_capture-win.cpp: $(srcdir)/capture-win.h # Call the Qt resource compiler qrc_qv4l2.cpp: $(srcdir)/qv4l2.qrc - rcc -name qv4l2 -o $@ $(srcdir)/qv4l2.qrc + $(RCC) -name qv4l2 -o $@ $(srcdir)/qv4l2.qrc install-data-local: $(INSTALL_DATA) -D -p $(srcdir)/qv4l2.desktop $(DESTDIR)$(datadir)/applications/qv4l2.desktop -- 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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
Some people thought that it would be nice to have the macros rather than the inlined field initializations, especially since there is no flag for write. A separate question is whether an array of one element is useful, or whether one should systematically use on a simple variable of the structure type. I'm open to suggestions about either point. I think the macro naming is not great. Maybe add DEFINE_/DECLARE_/_INIT or something other than an action name type to the macro names. DEFINE and DECLARE usually have a declared variable as an argument, which is not the case here. These macros are like the macros PCI_DEVICE and PCI_DEVICE_CLASS. Are READ and WRITE the action names? They are really the important information in this case. I think the consistency is better if all the references are done as arrays, even for single entry arrays. Is it worth creating arrays where msg is used? Or would it be better to leave that aspect as it is? julia -- 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 v3 0/3] OMAP 3 CSI-2 configuration
Hi all, This is an update to an old patchset for CSI-2 configuration for OMAP 3430 and 3630r. The patches have been tested on the 3630 only so far, and I don't plan to test them on 3430 in the near future. I changed quite a few things after a discussion with Tony a few days ago. The ISP driver now maps the relevant register from the control block and uses it directly. Which register is required is determined by the ISP revision: this is theoretically wrong, but since we only support OMAP 3430 and 3630 which have different ISPs it should be all right. If we need to support more OMAPs in the future we could revisit how that's being determined. Comments, questions and other kind of feedback is very welcome. Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- 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 v3 1/3] omap3isp: Add CSI configuration registers from control block to ISP resources
Add the registers used to configure the CSI-2 receiver PHY on OMAP3430 and
3630 and map them in the ISP driver. The register is part of the control
block but it only is needed by the ISP driver.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
arch/arm/mach-omap2/devices.c | 10 ++
drivers/media/platform/omap3isp/isp.c |6 --
drivers/media/platform/omap3isp/isp.h |2 ++
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index c00c689..9e4d5da 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -201,6 +201,16 @@ static struct resource omap3isp_resources[] = {
.flags = IORESOURCE_MEM,
},
{
+ .start = OMAP343X_CTRL_BASE + OMAP343X_CONTROL_CSIRXFE,
+ .end= OMAP343X_CTRL_BASE + OMAP343X_CONTROL_CSIRXFE
+ 3,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = OMAP343X_CTRL_BASE +
OMAP3630_CONTROL_CAMERA_PHY_CTRL,
+ .end= OMAP343X_CTRL_BASE +
OMAP3630_CONTROL_CAMERA_PHY_CTRL + 3,
+ .flags = IORESOURCE_MEM,
+ },
+ {
.start = INT_34XX_CAM_IRQ,
.flags = IORESOURCE_IRQ,
}
diff --git a/drivers/media/platform/omap3isp/isp.c
b/drivers/media/platform/omap3isp/isp.c
index d7aa513..88fba2c 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -100,7 +100,8 @@ static const struct isp_res_mapping isp_res_maps[] = {
1 OMAP3_ISP_IOMEM_RESZ |
1 OMAP3_ISP_IOMEM_SBL |
1 OMAP3_ISP_IOMEM_CSI2A_REGS1 |
- 1 OMAP3_ISP_IOMEM_CSIPHY2,
+ 1 OMAP3_ISP_IOMEM_CSIPHY2 |
+ 1 OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE,
},
{
.isp_rev = ISP_REVISION_15_0,
@@ -117,7 +118,8 @@ static const struct isp_res_mapping isp_res_maps[] = {
1 OMAP3_ISP_IOMEM_CSI2A_REGS2 |
1 OMAP3_ISP_IOMEM_CSI2C_REGS1 |
1 OMAP3_ISP_IOMEM_CSIPHY1 |
- 1 OMAP3_ISP_IOMEM_CSI2C_REGS2,
+ 1 OMAP3_ISP_IOMEM_CSI2C_REGS2 |
+ 1 OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL,
},
};
diff --git a/drivers/media/platform/omap3isp/isp.h
b/drivers/media/platform/omap3isp/isp.h
index 8be7487..6fed222 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -72,6 +72,8 @@ enum isp_mem_resources {
OMAP3_ISP_IOMEM_CSI2C_REGS1,
OMAP3_ISP_IOMEM_CSIPHY1,
OMAP3_ISP_IOMEM_CSI2C_REGS2,
+ OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE,
+ OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL,
OMAP3_ISP_IOMEM_LAST
};
--
1.7.2.5
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/3] omap3isp: Configure CSI-2 phy based on platform data
Configure CSI-2 phy based on platform data in the ISP driver. For that, the
new V4L2_CID_IMAGE_SOURCE_PIXEL_RATE control is used. Previously the same
was configured from the board code.
This patch is dependent on omap3: Provide means for changing CSI2 PHY
configuration.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
drivers/media/platform/omap3isp/isp.h |3 -
drivers/media/platform/omap3isp/ispcsiphy.c | 140 +-
drivers/media/platform/omap3isp/ispcsiphy.h | 10 --
3 files changed, 70 insertions(+), 83 deletions(-)
diff --git a/drivers/media/platform/omap3isp/isp.h
b/drivers/media/platform/omap3isp/isp.h
index 6fed222..accb3b0 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -129,9 +129,6 @@ struct isp_reg {
struct isp_platform_callback {
u32 (*set_xclk)(struct isp_device *isp, u32 xclk, u8 xclksel);
- int (*csiphy_config)(struct isp_csiphy *phy,
-struct isp_csiphy_dphy_cfg *dphy,
-struct isp_csiphy_lanes_cfg *lanes);
};
/*
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
b/drivers/media/platform/omap3isp/ispcsiphy.c
index f13bfbd..4ac1081 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -119,36 +119,6 @@ static void csiphy_routing_cfg(struct isp_csiphy *phy, u32
iface, bool on,
}
/*
- * csiphy_lanes_config - Configuration of CSIPHY lanes.
- *
- * Updates HW configuration.
- * Called with phy-mutex taken.
- */
-static void csiphy_lanes_config(struct isp_csiphy *phy)
-{
- unsigned int i;
- u32 reg;
-
- reg = isp_reg_readl(phy-isp, phy-cfg_regs, ISPCSI2_PHY_CFG);
-
- for (i = 0; i phy-num_data_lanes; i++) {
- reg = ~(ISPCSI2_PHY_CFG_DATA_POL_MASK(i + 1) |
-ISPCSI2_PHY_CFG_DATA_POSITION_MASK(i + 1));
- reg |= (phy-lanes.data[i].pol
- ISPCSI2_PHY_CFG_DATA_POL_SHIFT(i + 1));
- reg |= (phy-lanes.data[i].pos
- ISPCSI2_PHY_CFG_DATA_POSITION_SHIFT(i + 1));
- }
-
- reg = ~(ISPCSI2_PHY_CFG_CLOCK_POL_MASK |
-ISPCSI2_PHY_CFG_CLOCK_POSITION_MASK);
- reg |= phy-lanes.clk.pol ISPCSI2_PHY_CFG_CLOCK_POL_SHIFT;
- reg |= phy-lanes.clk.pos ISPCSI2_PHY_CFG_CLOCK_POSITION_SHIFT;
-
- isp_reg_writel(phy-isp, reg, phy-cfg_regs, ISPCSI2_PHY_CFG);
-}
-
-/*
* csiphy_power_autoswitch_enable
* @enable: Sets or clears the autoswitch function enable flag.
*/
@@ -193,43 +163,28 @@ static int csiphy_set_power(struct isp_csiphy *phy, u32
power)
}
/*
- * csiphy_dphy_config - Configure CSI2 D-PHY parameters.
- *
- * Called with phy-mutex taken.
+ * TCLK values are OK at their reset values
*/
-static void csiphy_dphy_config(struct isp_csiphy *phy)
-{
- u32 reg;
-
- /* Set up ISPCSIPHY_REG0 */
- reg = isp_reg_readl(phy-isp, phy-phy_regs, ISPCSIPHY_REG0);
-
- reg = ~(ISPCSIPHY_REG0_THS_TERM_MASK |
-ISPCSIPHY_REG0_THS_SETTLE_MASK);
- reg |= phy-dphy.ths_term ISPCSIPHY_REG0_THS_TERM_SHIFT;
- reg |= phy-dphy.ths_settle ISPCSIPHY_REG0_THS_SETTLE_SHIFT;
-
- isp_reg_writel(phy-isp, reg, phy-phy_regs, ISPCSIPHY_REG0);
-
- /* Set up ISPCSIPHY_REG1 */
- reg = isp_reg_readl(phy-isp, phy-phy_regs, ISPCSIPHY_REG1);
-
- reg = ~(ISPCSIPHY_REG1_TCLK_TERM_MASK |
-ISPCSIPHY_REG1_TCLK_MISS_MASK |
-ISPCSIPHY_REG1_TCLK_SETTLE_MASK);
- reg |= phy-dphy.tclk_term ISPCSIPHY_REG1_TCLK_TERM_SHIFT;
- reg |= phy-dphy.tclk_miss ISPCSIPHY_REG1_TCLK_MISS_SHIFT;
- reg |= phy-dphy.tclk_settle ISPCSIPHY_REG1_TCLK_SETTLE_SHIFT;
+#define TCLK_TERM 0
+#define TCLK_MISS 1
+#define TCLK_SETTLE14
- isp_reg_writel(phy-isp, reg, phy-phy_regs, ISPCSIPHY_REG1);
-}
-
-static int csiphy_config(struct isp_csiphy *phy,
-struct isp_csiphy_dphy_cfg *dphy,
-struct isp_csiphy_lanes_cfg *lanes)
+static int omap3isp_csiphy_config(struct isp_csiphy *phy)
{
+ struct isp_csi2_device *csi2 = phy-csi2;
+ struct isp_pipeline *pipe = to_isp_pipeline(csi2-subdev.entity);
+ struct isp_v4l2_subdevs_group *subdevs = pipe-external-host_priv;
+ struct isp_csiphy_lanes_cfg *lanes;
+ int csi2_ddrclk_khz;
unsigned int used_lanes = 0;
unsigned int i;
+ u32 reg;
+
+ if (subdevs-interface == ISP_INTERFACE_CCP2B_PHY1
+ || subdevs-interface == ISP_INTERFACE_CCP2B_PHY2)
+ lanes = subdevs-bus.ccp2.lanecfg;
+ else
+ lanes = subdevs-bus.csi2.lanecfg;
/* Clock and data lanes verification */
for (i = 0; i phy-num_data_lanes; i++) {
@@ -248,10 +203,56 @@ static int csiphy_config(struct isp_csiphy *phy,
if
[PATCH v3 2/3] omap3isp: Add PHY routing configuration
Add PHY routing configuration for both 3430 and 3630. Also add register bit
definitions of CSIRXFE and CAMERA_PHY_CTRL registers on OMAP 3430 and 3630,
respectively.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
---
drivers/media/platform/omap3isp/ispcsiphy.c | 86 +++
drivers/media/platform/omap3isp/ispreg.h| 22 +++
2 files changed, 108 insertions(+), 0 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
b/drivers/media/platform/omap3isp/ispcsiphy.c
index 348f67e..f13bfbd 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -32,6 +32,92 @@
#include ispreg.h
#include ispcsiphy.h
+static void csiphy_routing_cfg_3630(struct isp_csiphy *phy, u32 iface,
+ bool ccp2_strobe)
+{
+ u32 cam_phy_ctrl =
+ isp_reg_readl(phy-isp,
+ OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
+ u32 shift, mode;
+
+ switch (iface) {
+ case ISP_INTERFACE_CCP2B_PHY1:
+ cam_phy_ctrl =
+ ~OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2;
+ shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT;
+ break;
+ case ISP_INTERFACE_CSI2C_PHY1:
+ shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT;
+ mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY;
+ break;
+ case ISP_INTERFACE_CCP2B_PHY2:
+ cam_phy_ctrl |=
+ OMAP3630_CONTROL_CAMERA_PHY_CTRL_CSI1_RX_SEL_PHY2;
+ shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT;
+ break;
+ case ISP_INTERFACE_CSI2A_PHY2:
+ shift = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY2_SHIFT;
+ mode = OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_DPHY;
+ break;
+ default:
+ pr_warn(bad iface %d\n, iface);
+ return;
+ }
+
+ /* Select data/clock or data/strobe mode for CCP2 */
+ switch (iface) {
+ case ISP_INTERFACE_CCP2B_PHY1:
+ case ISP_INTERFACE_CCP2B_PHY2:
+ if (ccp2_strobe)
+ mode =
OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_STROBE;
+ else
+ mode =
OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_CCP2_DATA_CLOCK;
+ }
+
+ cam_phy_ctrl =
+ ~(OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK shift);
+ cam_phy_ctrl |= mode shift;
+
+ isp_reg_writel(phy-isp, cam_phy_ctrl,
+ OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
+}
+
+static void csiphy_routing_cfg_3430(struct isp_csiphy *phy, u32 iface, bool on,
+ bool ccp2_strobe)
+{
+ uint32_t csirxfe = OMAP343X_CONTROL_CSIRXFE_PWRDNZ
+ | OMAP343X_CONTROL_CSIRXFE_RESET;
+
+ /* Nothing to configure here. */
+ if (iface == ISP_INTERFACE_CSI2A_PHY2)
+ return;
+
+ if (iface != ISP_INTERFACE_CCP2B_PHY1)
+ return;
+
+ if (!on) {
+ isp_reg_writel(phy-isp, 0,
+ OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
+ return;
+ }
+
+ if (ccp2_strobe)
+ csirxfe |= OMAP343X_CONTROL_CSIRXFE_SELFORM;
+
+ isp_reg_writel(phy-isp, csirxfe,
+ OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
+}
+
+static void csiphy_routing_cfg(struct isp_csiphy *phy, u32 iface, bool on,
+ bool ccp2_strobe)
+{
+ if (phy-isp-mmio_base[OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL]
+on)
+ return csiphy_routing_cfg_3630(phy, iface, ccp2_strobe);
+ if (phy-isp-mmio_base[OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE])
+ return csiphy_routing_cfg_3430(phy, iface, on, ccp2_strobe);
+}
+
/*
* csiphy_lanes_config - Configuration of CSIPHY lanes.
*
diff --git a/drivers/media/platform/omap3isp/ispreg.h
b/drivers/media/platform/omap3isp/ispreg.h
index 084ea77..66d2b42 100644
--- a/drivers/media/platform/omap3isp/ispreg.h
+++ b/drivers/media/platform/omap3isp/ispreg.h
@@ -1583,4 +1583,26 @@
#define ISPCSIPHY_REG2_CCP2_SYNC_PATTERN_MASK \
(0x7f ISPCSIPHY_REG2_CCP2_SYNC_PATTERN_SHIFT)
+/*
-
+ * CONTROL registers for CSI-2 phy routing
+ */
+
+/* OMAP343X_CONTROL_CSIRXFE */
+#define OMAP343X_CONTROL_CSIRXFE_CSIB_INV (1 7)
+#define OMAP343X_CONTROL_CSIRXFE_RESENABLE (1 8)
+#define OMAP343X_CONTROL_CSIRXFE_SELFORM (1 10)
+#define OMAP343X_CONTROL_CSIRXFE_PWRDNZ(1 12)
+#define OMAP343X_CONTROL_CSIRXFE_RESET (1 13)
+
+/* OMAP3630_CONTROL_CAMERA_PHY_CTRL */
+#define OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_PHY1_SHIFT2
+#define
Re: [PATCH 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
On Sun, 2012-10-07 at 20:56 +0200, Julia Lawall wrote: Some people thought that it would be nice to have the macros rather than the inlined field initializations, especially since there is no flag for write. A separate question is whether an array of one element is useful, or whether one should systematically use on a simple variable of the structure type. I'm open to suggestions about either point. I think the macro naming is not great. Maybe add DEFINE_/DECLARE_/_INIT or something other than an action name type to the macro names. DEFINE and DECLARE usually have a declared variable as an argument, which is not the case here. These macros are like the macros PCI_DEVICE and PCI_DEVICE_CLASS. I understand that. Are READ and WRITE the action names? They are really the important information in this case. Yes, most (all?) uses of _READ and _WRITE macros actually perform some I/O. I think the consistency is better if all the references are done as arrays, even for single entry arrays. Is it worth creating arrays where msg is used? Or would it be better to leave that aspect as it is? Reasonable arguments can be made either way. -- 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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
On Sun, 7 Oct 2012, Joe Perches wrote: On Sun, 2012-10-07 at 20:56 +0200, Julia Lawall wrote: Some people thought that it would be nice to have the macros rather than the inlined field initializations, especially since there is no flag for write. A separate question is whether an array of one element is useful, or whether one should systematically use on a simple variable of the structure type. I'm open to suggestions about either point. I think the macro naming is not great. Maybe add DEFINE_/DECLARE_/_INIT or something other than an action name type to the macro names. DEFINE and DECLARE usually have a declared variable as an argument, which is not the case here. These macros are like the macros PCI_DEVICE and PCI_DEVICE_CLASS. I understand that. Are READ and WRITE the action names? They are really the important information in this case. Yes, most (all?) uses of _READ and _WRITE macros actually perform some I/O. I2C_MSG_READ_DATA? I2C_MSG_READ_INFO? I2C_MSG_READ_INIT? I2C_MSG_PREPARE_READ? julia -- 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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
On 08/10/12 08:39, Joe Perches wrote: On Sun, 2012-10-07 at 20:56 +0200, Julia Lawall wrote: Some people thought that it would be nice to have the macros rather than the inlined field initializations, especially since there is no flag for write. A separate question is whether an array of one element is useful, or whether one should systematically use on a simple variable of the structure type. I'm open to suggestions about either point. I think the macro naming is not great. Maybe add DEFINE_/DECLARE_/_INIT or something other than an action name type to the macro names. DEFINE and DECLARE usually have a declared variable as an argument, which is not the case here. These macros are like the macros PCI_DEVICE and PCI_DEVICE_CLASS. I understand that. Are READ and WRITE the action names? They are really the important information in this case. Yes, most (all?) uses of _READ and _WRITE macros actually perform some I/O. Well, they are describing an IO operation even if they don't perform it directly. What else would you call them? I think the macro names are fine as is. ~Ryan -- 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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
On Sun, 2012-10-07 at 23:43 +0200, Julia Lawall wrote: On Sun, 7 Oct 2012, Joe Perches wrote: Are READ and WRITE the action names? They are really the important information in this case. Yes, most (all?) uses of _READ and _WRITE macros actually perform some I/O. I2C_MSG_READ_DATA? I2C_MSG_READ_INFO? I2C_MSG_READ_INIT? I2C_MSG_PREPARE_READ? Dunno, naming is hard. Maybe: I2C_INPUT_MSG I2C_OUTPUT_MSG I2C_OP_MSG cheers, Joe -- 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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
On 08/10/12 03:44, Julia Lawall wrote:
On Sun, 7 Oct 2012, walter harms wrote:
Am 07.10.2012 17:38, schrieb Julia Lawall:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
In the second i2c_msg structure, a length expressed as an explicit
constant
is also re-expressed as the size of the buffer, reg.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/e4000.c | 20 +++-
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/media/tuners/e4000.c b/drivers/media/tuners/e4000.c
index 1b33ed3..8f182fc 100644
--- a/drivers/media/tuners/e4000.c
+++ b/drivers/media/tuners/e4000.c
@@ -26,12 +26,7 @@ static int e4000_wr_regs(struct e4000_priv *priv,
u8 reg, u8 *val, int len)
int ret;
u8 buf[1 + len];
struct i2c_msg msg[1] = {
-{
-.addr = priv-cfg-i2c_addr,
-.flags = 0,
-.len = sizeof(buf),
-.buf = buf,
-}
+I2C_MSG_WRITE(priv-cfg-i2c_addr, buf, sizeof(buf))
};
Any reason why struct i2c_msg is an array ?
I assumed that it looked more harmonious with the other uses of
i2c_transfer, which takes as arguments an array and the number of elements.
But there are some files that instead use i2c_transfer(priv-i2c, msg, 1).
I can change them all to do that if that is preferred. But maybe I will
wait a little bit to see if there are other issues to address at the
same time.
This is probably a good thing to do, but the initial patch series should
just do the conversion to the macros. Too many additional changes runs
the risk of introducing bugs and making bisection difficult.
~Ryan
--
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/13] drivers/media/tuners/qt1010.c: use macros for i2c_msg initialization
On 08/10/12 02:38, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer, when this is possible.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/qt1010.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/media/tuners/qt1010.c b/drivers/media/tuners/qt1010.c
index bc419f8..37ff254 100644
--- a/drivers/media/tuners/qt1010.c
+++ b/drivers/media/tuners/qt1010.c
@@ -25,10 +25,8 @@
static int qt1010_readreg(struct qt1010_priv *priv, u8 reg, u8 *val)
{
struct i2c_msg msg[2] = {
- { .addr = priv-cfg-i2c_address,
- .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-cfg-i2c_address,
- .flags = I2C_M_RD, .buf = val, .len = 1 },
+ I2C_MSG_WRITE(priv-cfg-i2c_address, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-cfg-i2c_address, val, 1),
This is a bit inconsistent. For single length values we should either
consistently use sizeof(val) or 1. This has both.
~Ryan
--
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/13] drivers/media/tuners: use macros for i2c_msg initialization
On 08/10/12 02:38, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer, when this is possible.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/fc0012.c |8 +++-
drivers/media/tuners/fc0013.c |8 +++-
drivers/media/tuners/mc44s803.c |8 +++-
drivers/media/tuners/mt2060.c | 13 +
drivers/media/tuners/mt2063.c | 23 ++-
drivers/media/tuners/mt2131.c | 13 +
drivers/media/tuners/mt2266.c | 13 +
drivers/media/tuners/mxl5005s.c |8
drivers/media/tuners/tda827x.c | 29 +++--
drivers/media/tuners/tuner-i2c.h| 12
drivers/media/tuners/tuner-simple.c |5 +
drivers/media/tuners/xc4000.c |9 +++--
drivers/media/tuners/xc5000.c |9 +++--
13 files changed, 56 insertions(+), 102 deletions(-)
diff --git a/drivers/media/tuners/fc0012.c b/drivers/media/tuners/fc0012.c
index 308135a..01dac7e 100644
--- a/drivers/media/tuners/fc0012.c
+++ b/drivers/media/tuners/fc0012.c
@@ -24,9 +24,7 @@
static int fc0012_writereg(struct fc0012_priv *priv, u8 reg, u8 val)
{
u8 buf[2] = {reg, val};
- struct i2c_msg msg = {
- .addr = priv-addr, .flags = 0, .buf = buf, .len = 2
- };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-addr, buf, sizeof(buf));
if (i2c_transfer(priv-i2c, msg, 1) != 1) {
err(I2C write reg failed, reg: %02x, val: %02x, reg, val);
@@ -38,8 +36,8 @@ static int fc0012_writereg(struct fc0012_priv *priv, u8
reg, u8 val)
static int fc0012_readreg(struct fc0012_priv *priv, u8 reg, u8 *val)
{
struct i2c_msg msg[2] = {
- { .addr = priv-addr, .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-addr, .flags = I2C_M_RD, .buf = val, .len = 1 },
+ I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-addr, val, 1),
};
if (i2c_transfer(priv-i2c, msg, 2) != 2) {
diff --git a/drivers/media/tuners/fc0013.c b/drivers/media/tuners/fc0013.c
index bd8f0f1..174f0b0 100644
--- a/drivers/media/tuners/fc0013.c
+++ b/drivers/media/tuners/fc0013.c
@@ -27,9 +27,7 @@
static int fc0013_writereg(struct fc0013_priv *priv, u8 reg, u8 val)
{
u8 buf[2] = {reg, val};
- struct i2c_msg msg = {
- .addr = priv-addr, .flags = 0, .buf = buf, .len = 2
- };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-addr, buf, sizeof(buf));
if (i2c_transfer(priv-i2c, msg, 1) != 1) {
err(I2C write reg failed, reg: %02x, val: %02x, reg, val);
@@ -41,8 +39,8 @@ static int fc0013_writereg(struct fc0013_priv *priv, u8
reg, u8 val)
static int fc0013_readreg(struct fc0013_priv *priv, u8 reg, u8 *val)
{
struct i2c_msg msg[2] = {
- { .addr = priv-addr, .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-addr, .flags = I2C_M_RD, .buf = val, .len = 1 },
+ I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-addr, val, 1),
};
if (i2c_transfer(priv-i2c, msg, 2) != 2) {
diff --git a/drivers/media/tuners/mc44s803.c b/drivers/media/tuners/mc44s803.c
index f1b7640..df47e03 100644
--- a/drivers/media/tuners/mc44s803.c
+++ b/drivers/media/tuners/mc44s803.c
@@ -37,9 +37,8 @@
static int mc44s803_writereg(struct mc44s803_priv *priv, u32 val)
{
u8 buf[3];
- struct i2c_msg msg = {
- .addr = priv-cfg-i2c_address, .flags = 0, .buf = buf, .len = 3
- };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-cfg-i2c_address, buf,
+sizeof(buf));
buf[0] = (val 0xff) 16;
buf[1] = (val 0xff00) 8;
@@ -59,8 +58,7 @@ static int mc44s803_readreg(struct mc44s803_priv *priv, u8
reg, u32 *val)
u8 buf[3];
int ret;
struct i2c_msg msg[] = {
- { .addr = priv-cfg-i2c_address, .flags = I2C_M_RD,
- .buf = buf, .len = 3 },
+ I2C_MSG_READ(priv-cfg-i2c_address, buf, sizeof(buf)),
};
wval = MC44S803_REG_SM(MC44S803_REG_DATAREG, MC44S803_ADDR) |
diff
Re: [PATCH 12/13] drivers/media/tuners/max2165.c: use macros for i2c_msg initialization
On 08/10/12 02:38, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer, when this is possible.
The second case is simplified to use simple variables rather than arrays.
The variable b0 is dropped completely, and the variable reg that it
contains is used instead. The variable b1 is replaced by a u8-typed
variable named buf (the name used earlier in the file). The uses of b1 are
then adjusted accordingly.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/max2165.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/media/tuners/max2165.c b/drivers/media/tuners/max2165.c
index ba84936..6638617 100644
--- a/drivers/media/tuners/max2165.c
+++ b/drivers/media/tuners/max2165.c
@@ -47,7 +47,7 @@ static int max2165_write_reg(struct max2165_priv *priv, u8
reg, u8 data)
{
int ret;
u8 buf[] = { reg, data };
- struct i2c_msg msg = { .flags = 0, .buf = buf, .len = 2 };
+ struct i2c_msg msg = I2C_MSG_WRITE(0, buf, sizeof(buf));
msg.addr = priv-config-i2c_address;
@@ -68,11 +68,10 @@ static int max2165_read_reg(struct max2165_priv *priv, u8
reg, u8 *p_data)
int ret;
u8 dev_addr = priv-config-i2c_address;
- u8 b0[] = { reg };
- u8 b1[] = { 0 };
+ u8 buf;
struct i2c_msg msg[] = {
- { .addr = dev_addr, .flags = 0, .buf = b0, .len = 1 },
- { .addr = dev_addr, .flags = I2C_M_RD, .buf = b1, .len = 1 },
+ I2C_MSG_WRITE(dev_addr, reg, sizeof(reg)),
+ I2C_MSG_READ(dev_addr, buf, sizeof(buf)),
};
Not sure if the array changes should be done here or as a separate
patch. Some of the other patches also have cases where single index
arrays (both buffers and messages) could be converted. Should either
convert all or none of them. I think its probably best to do as a
separate series on top of this though.
~Ryan
ret = i2c_transfer(priv-i2c, msg, 2);
@@ -81,10 +80,10 @@ static int max2165_read_reg(struct max2165_priv *priv, u8
reg, u8 *p_data)
return -EIO;
}
- *p_data = b1[0];
+ *p_data = buf;
if (debug = 2)
dprintk(%s: reg=0x%02X, data=0x%02X\n,
- __func__, reg, b1[0]);
+ __func__, reg, buf);
return 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: ir-keytable: Bug in gen_keytables.pl script
Em Sun, 07 Oct 2012 19:24:26 +0200
Gregor Jasny gja...@googlemail.com escreveu:
Hello,
I recently received this launchpad bug:
https://bugs.launchpad.net/ubuntu/+source/v4l-utils/+bug/1054122
It seems that the mentioned key mappings are missing.
If you check the generated mapping file
http://git.linuxtv.org/v4l-utils.git/blob/HEAD:/utils/keytable/rc_keymaps/imon_pad
and compare it to the driver file
http://git.linuxtv.org/media_tree.git/blob/refs/heads/staging/for_v3.7:/drivers/media/rc/keymaps/rc-imon-pad.c#l111
you'll notice that the parsing stopped at the BTN_xyz table entries:
{ 0x299115b7, KEY_KEYBOARD },
{ 0x299135b7, KEY_KEYBOARD },
processing stopped here
{ 0x0101, BTN_LEFT },
{ 0x0102, BTN_RIGHT },
{ 0x01010080, BTN_LEFT },
{ 0x01020080, BTN_RIGHT },
{ 0x688301b7, BTN_LEFT },
{ 0x688481b7, BTN_RIGHT },
{ 0x2a9395b7, KEY_CYCLEWINDOWS }, /* TaskSwitcher */
{ 0x2b8395b7, KEY_TIME }, /* Timer */
Mauro, could you please take a look? I guess the BTN_xyz entries should
be also added to the keytable files. Unfortunately my Perl skills are
horrible.
Thanks for noticing!
Fixed. The fix for it was quick and simple:
- if (m/(0x[\dA-Fa-f]+)[\s\,]+(KEY_[^\s\,\}]+)/) {
- $out .= $1 $2\n;
+ if (m/(0x[\dA-Fa-f]+)[\s\,]+(KEY|BTN)(\_[^\s\,\}]+)/) {
+ $out .= $1 $2$3\n;
Basically, the regex there weren't expecting mouse buttons. Some MCE remotes
have it.
I also updated the keytables, syncing it with the very latest git tree.
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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
Em Sun, 07 Oct 2012 14:51:58 -0700
Joe Perches j...@perches.com escreveu:
On Sun, 2012-10-07 at 23:43 +0200, Julia Lawall wrote:
On Sun, 7 Oct 2012, Joe Perches wrote:
Are READ and WRITE the action names? They are really the important
information in this case.
Yes, most (all?) uses of _READ and _WRITE macros actually
perform some I/O.
I2C_MSG_READ_DATA?
I2C_MSG_READ_INFO?
I2C_MSG_READ_INIT?
I2C_MSG_PREPARE_READ?
Dunno, naming is hard. Maybe:
I2C_INPUT_MSG
I2C_OUTPUT_MSG
I2C_OP_MSG
READ/WRITE sounds better, IMHO, as it will generally match with the
function names (they're generally named like foo_i2c_read or foo_reg_read).
I think none of them uses input or output. Btw, with I2C buses, a
register read is coded as a write ops, that sets the register's sub-address,
followed by a read ops from that (and subsequent) registers.
I'm afraid that using INPUT/OUTPUT will sound confusing.
So, IMHO, I2C_READ_MSG and I2C_WRITE_MSG sounds better.
Btw, as the WRITE + READ operation is very common (I think it is
much more common than a simple READ msg), it could make sense to have
just one macro for it, like:
INIT_I2C_READ_SUBADDR() that would take both write and read values.
I also don't like the I2C_MSG_OP. The operations there are always
read or write.
So, IMHO, the better would be to code the read and write message init message
as something similar to:
#define DECLARE_I2C_MSG_READ(_msg, _addr, _buf, _len, _flags)
\
struct i2c_msg _msg[1] = {
\
{.addr = _addr, .buf = _buf, .len = _len, .flags = (_flags) |
I2C_M_RD }\
}
#define DECLARE_I2C_MSG_WRITE(_msg, _addr, _buf, _len, _flags)
\
struct i2c_msg _msg[1] = {
\
{.addr = _addr, .buf = _buf, .len = _len, .flags = (_flags)
~I2C_M_RD } \
}
#define DECLARE_I2C_MSG_READ_SUBADDR(_msg, _addr, _subaddr, _sublen,_subflags,
_buf,_len, _flags) \
struct i2c_msg _msg[2] = {
\
{.addr = _addr, .buf = _subbuf, .len = _sublen, .flags =
(_subflags) ~I2C_M_RD } \
{.addr = _addr, .buf = _buf, .len = _len, .flags = (_flags) |
I2C_M_RD }\
}
Notes:
1) in the case of DECLARE_I2C_MSG_READ_SUBADDR(), I'm almost sure that, in all
cases, the
first message will always have buffer size equal to 1. If so, you can simplify
the number
of arguments there.
2) It could make sense to have, in fact, two versions for each, one with _FLAGS
and another one
without. On most cases, the one without flags are used.
3) I bet that most of the cases where 2 messages are used, the first message
has length equal
to one, and it uses a fixed u8 constant with the I2C sub-address. So, maybe it
would be nice
to have a variant for this case.
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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
On 08/10/12 12:56, Mauro Carvalho Chehab wrote:
Em Sun, 07 Oct 2012 14:51:58 -0700
Joe Perches j...@perches.com escreveu:
On Sun, 2012-10-07 at 23:43 +0200, Julia Lawall wrote:
On Sun, 7 Oct 2012, Joe Perches wrote:
Are READ and WRITE the action names? They are really the important
information in this case.
Yes, most (all?) uses of _READ and _WRITE macros actually
perform some I/O.
I2C_MSG_READ_DATA?
I2C_MSG_READ_INFO?
I2C_MSG_READ_INIT?
I2C_MSG_PREPARE_READ?
Dunno, naming is hard. Maybe:
I2C_INPUT_MSG
I2C_OUTPUT_MSG
I2C_OP_MSG
READ/WRITE sounds better, IMHO, as it will generally match with the
function names (they're generally named like foo_i2c_read or foo_reg_read).
I think none of them uses input or output. Btw, with I2C buses, a
register read is coded as a write ops, that sets the register's sub-address,
followed by a read ops from that (and subsequent) registers.
I'm afraid that using INPUT/OUTPUT will sound confusing.
So, IMHO, I2C_READ_MSG and I2C_WRITE_MSG sounds better.
Btw, as the WRITE + READ operation is very common (I think it is
much more common than a simple READ msg), it could make sense to have
just one macro for it, like:
INIT_I2C_READ_SUBADDR() that would take both write and read values.
I also don't like the I2C_MSG_OP. The operations there are always
read or write.
So, IMHO, the better would be to code the read and write message init message
as something similar to:
#define DECLARE_I2C_MSG_READ(_msg, _addr, _buf, _len, _flags)
\
struct i2c_msg _msg[1] = {
\
{.addr = _addr, .buf = _buf, .len = _len, .flags = (_flags) |
I2C_M_RD }\
}
#define DECLARE_I2C_MSG_WRITE(_msg, _addr, _buf, _len, _flags)
\
struct i2c_msg _msg[1] = {
\
{.addr = _addr, .buf = _buf, .len = _len, .flags = (_flags)
~I2C_M_RD } \
}
#define DECLARE_I2C_MSG_READ_SUBADDR(_msg, _addr, _subaddr,
_sublen,_subflags, _buf,_len, _flags) \
struct i2c_msg _msg[2] = {
\
{.addr = _addr, .buf = _subbuf, .len = _sublen, .flags =
(_subflags) ~I2C_M_RD } \
{.addr = _addr, .buf = _buf, .len = _len, .flags = (_flags) |
I2C_M_RD }\
}
I think this is probably more confusing, not less. The macro takes 8
arguments, and in many cases will wrap onto two or more lines. The large
number of arguments also makes it difficult for a casual reader to
determine exactly what it does. In comparison:
I2C_MSG_WRITE(info-i2c_addr, reg, 1);
I2C_MSG_READ(info-i2c_addr, buf, sizeof(buf));
is fairly self-explanatory, especially for someone familiar with i2c,
without having to look up the macro definitions.
Notes:
1) in the case of DECLARE_I2C_MSG_READ_SUBADDR(), I'm almost sure that, in
all cases, the
first message will always have buffer size equal to 1. If so, you can
simplify the number
of arguments there.
2) It could make sense to have, in fact, two versions for each, one with
_FLAGS and another one
without. On most cases, the one without flags are used.
3) I bet that most of the cases where 2 messages are used, the first message
has length equal
to one, and it uses a fixed u8 constant with the I2C sub-address. So, maybe
it would be nice
to have a variant for this case.
That ends up with a whole bunch of variants of the macro for something
which should be very simple. The proposal has three macros, and the
I2C_MSG_OP macro is only required for a one or two corner cases where
non-standard flags are used.
~Ryan
--
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 13/13] drivers/media/tuners/e4000.c: use macros for i2c_msg initialization
On Sun, 7 Oct 2012, Joe Perches wrote: On Sun, 2012-10-07 at 23:43 +0200, Julia Lawall wrote: On Sun, 7 Oct 2012, Joe Perches wrote: Are READ and WRITE the action names? They are really the important information in this case. Yes, most (all?) uses of _READ and _WRITE macros actually perform some I/O. I2C_MSG_READ_DATA? I2C_MSG_READ_INFO? I2C_MSG_READ_INIT? I2C_MSG_PREPARE_READ? Dunno, naming is hard. Maybe: I2C_INPUT_MSG I2C_OUTPUT_MSG I2C_OP_MSG The current terminology, however, is READ, not INPUT (.flags = I2C_M_RD). julia -- 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/13] drivers/media/tuners/qt1010.c: use macros for i2c_msg initialization
On Mon, 8 Oct 2012, Ryan Mallon wrote:
On 08/10/12 02:38, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer, when this is possible.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/qt1010.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/media/tuners/qt1010.c b/drivers/media/tuners/qt1010.c
index bc419f8..37ff254 100644
--- a/drivers/media/tuners/qt1010.c
+++ b/drivers/media/tuners/qt1010.c
@@ -25,10 +25,8 @@
static int qt1010_readreg(struct qt1010_priv *priv, u8 reg, u8 *val)
{
struct i2c_msg msg[2] = {
- { .addr = priv-cfg-i2c_address,
- .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-cfg-i2c_address,
- .flags = I2C_M_RD, .buf = val, .len = 1 },
+ I2C_MSG_WRITE(priv-cfg-i2c_address, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-cfg-i2c_address, val, 1),
This is a bit inconsistent. For single length values we should either
consistently use sizeof(val) or 1. This has both.
val is a pointer. It does not have size 1.
julia
--
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/13] drivers/media/tuners: use macros for i2c_msg initialization
As far as I can see, the comments on this are:
b[0] should be just b
There should be a newline before the elements of a multi-element array
Some one-element array buffers should just be variables
sizeof(val) should be used
I will redo the patches to include the first two, but not the others.
I will send another set of patches for the third one. It seems like the
conclusion is that the buffer should always be a variable if it can, but
the message should always be an array for uniformity in the call to
i2c_transfer.
I believe that the fourth one is not correct. The variable in question is
a pointer, so sizeof would give the wrong result. If it is preferred, I
could not use sizeof for the other structure in the same code, but it
seems just a little safer to use it.
thanks,
julia
On Mon, 8 Oct 2012, Ryan Mallon wrote:
On 08/10/12 02:38, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer, when this is possible.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/fc0012.c |8 +++-
drivers/media/tuners/fc0013.c |8 +++-
drivers/media/tuners/mc44s803.c |8 +++-
drivers/media/tuners/mt2060.c | 13 +
drivers/media/tuners/mt2063.c | 23 ++-
drivers/media/tuners/mt2131.c | 13 +
drivers/media/tuners/mt2266.c | 13 +
drivers/media/tuners/mxl5005s.c |8
drivers/media/tuners/tda827x.c | 29 +++--
drivers/media/tuners/tuner-i2c.h| 12
drivers/media/tuners/tuner-simple.c |5 +
drivers/media/tuners/xc4000.c |9 +++--
drivers/media/tuners/xc5000.c |9 +++--
13 files changed, 56 insertions(+), 102 deletions(-)
diff --git a/drivers/media/tuners/fc0012.c b/drivers/media/tuners/fc0012.c
index 308135a..01dac7e 100644
--- a/drivers/media/tuners/fc0012.c
+++ b/drivers/media/tuners/fc0012.c
@@ -24,9 +24,7 @@
static int fc0012_writereg(struct fc0012_priv *priv, u8 reg, u8 val)
{
u8 buf[2] = {reg, val};
- struct i2c_msg msg = {
- .addr = priv-addr, .flags = 0, .buf = buf, .len = 2
- };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-addr, buf, sizeof(buf));
if (i2c_transfer(priv-i2c, msg, 1) != 1) {
err(I2C write reg failed, reg: %02x, val: %02x, reg, val);
@@ -38,8 +36,8 @@ static int fc0012_writereg(struct fc0012_priv *priv, u8 reg,
u8 val)
static int fc0012_readreg(struct fc0012_priv *priv, u8 reg, u8 *val)
{
struct i2c_msg msg[2] = {
- { .addr = priv-addr, .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-addr, .flags = I2C_M_RD, .buf = val, .len = 1 },
+ I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-addr, val, 1),
};
if (i2c_transfer(priv-i2c, msg, 2) != 2) {
diff --git a/drivers/media/tuners/fc0013.c b/drivers/media/tuners/fc0013.c
index bd8f0f1..174f0b0 100644
--- a/drivers/media/tuners/fc0013.c
+++ b/drivers/media/tuners/fc0013.c
@@ -27,9 +27,7 @@
static int fc0013_writereg(struct fc0013_priv *priv, u8 reg, u8 val)
{
u8 buf[2] = {reg, val};
- struct i2c_msg msg = {
- .addr = priv-addr, .flags = 0, .buf = buf, .len = 2
- };
+ struct i2c_msg msg = I2C_MSG_WRITE(priv-addr, buf, sizeof(buf));
if (i2c_transfer(priv-i2c, msg, 1) != 1) {
err(I2C write reg failed, reg: %02x, val: %02x, reg, val);
@@ -41,8 +39,8 @@ static int fc0013_writereg(struct fc0013_priv *priv, u8 reg,
u8 val)
static int fc0013_readreg(struct fc0013_priv *priv, u8 reg, u8 *val)
{
struct i2c_msg msg[2] = {
- { .addr = priv-addr, .flags = 0, .buf = reg, .len = 1 },
- { .addr = priv-addr, .flags = I2C_M_RD, .buf = val, .len = 1 },
+ I2C_MSG_WRITE(priv-addr, reg, sizeof(reg)),
+ I2C_MSG_READ(priv-addr, val, 1),
};
if (i2c_transfer(priv-i2c, msg, 2) != 2) {
diff --git a/drivers/media/tuners/mc44s803.c b/drivers/media/tuners/mc44s803.c
index f1b7640..df47e03 100644
--- a/drivers/media/tuners/mc44s803.c
+++ b/drivers/media/tuners/mc44s803.c
@@ -37,9 +37,8 @@
static int mc44s803_writereg(struct mc44s803_priv
Re: [PATCH 12/13] drivers/media/tuners/max2165.c: use macros for i2c_msg initialization
On Mon, 8 Oct 2012, Ryan Mallon wrote:
On 08/10/12 02:38, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as the size
of the buffer, when this is possible.
The second case is simplified to use simple variables rather than arrays.
The variable b0 is dropped completely, and the variable reg that it
contains is used instead. The variable b1 is replaced by a u8-typed
variable named buf (the name used earlier in the file). The uses of b1 are
then adjusted accordingly.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/max2165.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/media/tuners/max2165.c b/drivers/media/tuners/max2165.c
index ba84936..6638617 100644
--- a/drivers/media/tuners/max2165.c
+++ b/drivers/media/tuners/max2165.c
@@ -47,7 +47,7 @@ static int max2165_write_reg(struct max2165_priv *priv, u8
reg, u8 data)
{
int ret;
u8 buf[] = { reg, data };
- struct i2c_msg msg = { .flags = 0, .buf = buf, .len = 2 };
+ struct i2c_msg msg = I2C_MSG_WRITE(0, buf, sizeof(buf));
msg.addr = priv-config-i2c_address;
@@ -68,11 +68,10 @@ static int max2165_read_reg(struct max2165_priv *priv, u8
reg, u8 *p_data)
int ret;
u8 dev_addr = priv-config-i2c_address;
- u8 b0[] = { reg };
- u8 b1[] = { 0 };
+ u8 buf;
struct i2c_msg msg[] = {
- { .addr = dev_addr, .flags = 0, .buf = b0, .len = 1 },
- { .addr = dev_addr, .flags = I2C_M_RD, .buf = b1, .len = 1 },
+ I2C_MSG_WRITE(dev_addr, reg, sizeof(reg)),
+ I2C_MSG_READ(dev_addr, buf, sizeof(buf)),
};
Not sure if the array changes should be done here or as a separate
patch. Some of the other patches also have cases where single index
arrays (both buffers and messages) could be converted. Should either
convert all or none of them. I think its probably best to do as a
separate series on top of this though.
OK, I will do it that way.
thanks,
julia
--
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/13] drivers/media/tuners/qt1010.c: use macros for i2c_msg initialization
On 08/10/12 16:05, Julia Lawall wrote:
On Mon, 8 Oct 2012, Ryan Mallon wrote:
On 08/10/12 02:38, Julia Lawall wrote:
From: Julia Lawall julia.law...@lip6.fr
Introduce use of I2c_MSG_READ/WRITE/OP, for readability.
A length expressed as an explicit constant is also re-expressed as
the size
of the buffer, when this is possible.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// smpl
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = I2C_M_RD}
+ I2C_MSG_READ(a,b,c)
;
@@
expression a,b,c;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = 0}
+ I2C_MSG_WRITE(a,b,c)
;
@@
expression a,b,c,d;
identifier x;
@@
struct i2c_msg x =
- {.addr = a, .buf = b, .len = c, .flags = d}
+ I2C_MSG_OP(a,b,c,d)
;
// /smpl
Signed-off-by: Julia Lawall julia.law...@lip6.fr
---
drivers/media/tuners/qt1010.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/media/tuners/qt1010.c
b/drivers/media/tuners/qt1010.c
index bc419f8..37ff254 100644
--- a/drivers/media/tuners/qt1010.c
+++ b/drivers/media/tuners/qt1010.c
@@ -25,10 +25,8 @@
static int qt1010_readreg(struct qt1010_priv *priv, u8 reg, u8 *val)
{
struct i2c_msg msg[2] = {
-{ .addr = priv-cfg-i2c_address,
- .flags = 0, .buf = reg, .len = 1 },
-{ .addr = priv-cfg-i2c_address,
- .flags = I2C_M_RD, .buf = val, .len = 1 },
+I2C_MSG_WRITE(priv-cfg-i2c_address, reg, sizeof(reg)),
+I2C_MSG_READ(priv-cfg-i2c_address, val, 1),
This is a bit inconsistent. For single length values we should either
consistently use sizeof(val) or 1. This has both.
val is a pointer. It does not have size 1.
Sorry, I mean either:
I2C_MSG_WRITE(priv-cfg-i2c_address, reg, sizeof(reg)),
I2C_MSG_READ(priv-cfg-i2c_address, val, sizeof(*val)),
or:
I2C_MSG_WRITE(priv-cfg-i2c_address, reg, 1),
I2C_MSG_READ(priv-cfg-i2c_address, val, 1),
~Ryan
--
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/13] drivers/media/tuners/qt1010.c: use macros for i2c_msg initialization
Sorry, I mean either: I2C_MSG_WRITE(priv-cfg-i2c_address, reg, sizeof(reg)), I2C_MSG_READ(priv-cfg-i2c_address, val, sizeof(*val)), Of course. Sorry for not having seen that. I can do that. julia -- 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
