Re: [PATCH v4 17/22] [media] em28xx-i2c: Fix error code for I2C error transfers

[Mauro Carvalho Chehab]

Em Sun, 05 Jan 2014 21:40:38 +0100
Frank Schäfer  escreveu:

> Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab:
> > The proper error code for I2C errors are EREMOTEIO. The em28xx driver
> > is using EIO instead.
> >
> > Replace all occurrences of EIO at em28xx-i2c, in order to fix it.
> >
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  drivers/media/usb/em28xx/em28xx-i2c.c | 20 ++--
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
> > b/drivers/media/usb/em28xx/em28xx-i2c.c
> > index 9fa7ed51e5b1..8b35aa51b9bb 100644
> > --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> > +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
> > @@ -72,7 +72,7 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 
> > addr, u8 *buf, u16 len)
> > if (ret != 2 + len) {
> > em28xx_warn("failed to trigger write to i2c address 0x%x 
> > (error=%i)\n",
> > addr, ret);
> > -   return (ret < 0) ? ret : -EIO;
> > +   return (ret < 0) ? ret : -EREMOTEIO;
> > }
> > /* wait for completion */
> > while (time_is_after_jiffies(timeout)) {
> > @@ -91,7 +91,7 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 
> > addr, u8 *buf, u16 len)
> > msleep(5);
> > }
> > em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
> > -   return -EIO;
> > +   return -EREMOTEIO;
> >  }
> >  
> >  /*
> > @@ -115,7 +115,7 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 
> > addr, u8 *buf, u16 len)
> > if (ret != 2) {
> > em28xx_warn("failed to trigger read from i2c address 0x%x 
> > (error=%i)\n",
> > addr, ret);
> > -   return (ret < 0) ? ret : -EIO;
> > +   return (ret < 0) ? ret : -EREMOTEIO;
> > }
> >  
> > /* wait for completion */
> > @@ -142,7 +142,7 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 
> > addr, u8 *buf, u16 len)
> > if (ret != len) {
> > em28xx_warn("reading from i2c device at 0x%x failed: couldn't 
> > get the received message from the bridge (error=%i)\n",
> > addr, ret);
> > -   return (ret < 0) ? ret : -EIO;
> > +   return (ret < 0) ? ret : -EREMOTEIO;
> > }
> > for (i = 0; i < len; i++)
> > buf[i] = buf2[len - 1 - i];
> > @@ -162,7 +162,7 @@ static int em2800_i2c_check_for_device(struct em28xx 
> > *dev, u8 addr)
> > ret = em2800_i2c_recv_bytes(dev, addr, &buf, 1);
> > if (ret == 1)
> > return 0;
> > -   return (ret < 0) ? ret : -EIO;
> > +   return (ret < 0) ? ret : -EREMOTEIO;
> >  }
> >  
> >  /*
> > @@ -191,7 +191,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, 
> > u16 addr, u8 *buf,
> > } else {
> > em28xx_warn("%i bytes write to i2c device at 0x%x 
> > requested, but %i bytes written\n",
> > len, addr, ret);
> > -   return -EIO;
> > +   return -EREMOTEIO;
> > }
> > }
> >  
> > @@ -219,7 +219,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, 
> > u16 addr, u8 *buf,
> > }
> >  
> > em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
> > -   return -EIO;
> > +   return -EREMOTEIO;
> >  }
> >  
> >  /*
> > @@ -268,7 +268,7 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, 
> > u16 addr, u8 *buf, u16 len)
> > }
> >  
> > em28xx_warn("unknown i2c error (status=%i)\n", ret);
> > -   return -EIO;
> > +   return -EREMOTEIO;
> >  }
> >  
> >  /*
> > @@ -283,7 +283,7 @@ static int em28xx_i2c_check_for_device(struct em28xx 
> > *dev, u16 addr)
> > ret = em28xx_i2c_recv_bytes(dev, addr, &buf, 1);
> > if (ret == 1)
> > return 0;
> > -   return (ret < 0) ? ret : -EIO;
> > +   return (ret < 0) ? ret : -EREMOTEIO;
> >  }
> >  
> >  /*
> > @@ -312,7 +312,7 @@ static int em25xx_bus_B_send_bytes(struct em28xx *dev, 
> > u16 addr, u8 *buf,
> > } else {
> > em28xx_warn("%i bytes write to i2c device at 0x%x 
> > requested, but %i bytes written\n",
> > len, addr, ret);
> > -   return -EIO;
> > +   return -EREMOTEIO;
> > }
> > }
> > /* Check success */
> Why the hell -EREMOTEIO ???
> See Documentation/i2c/fault-codes.
> It's not even listed there !

> 
> What are you trying to fix here ?

This is not a fixup patch.

The idea of this path is to make em28xx more compliant with the Kernel
error codes. 

As em28xx was the first USB media driver, it doesn't follow the best 
standards, despite all efforts we've made back in 2005, when the driver
was merged, in order to follow the Kernel rules. We eventually fixed
several things along the time, but we didn't took much care to fix the
I2C error codes so far.

Now that we're taking care of it, we should do it right.

However, you're actually right here.

Re: omap3isp device tree support

[Julien BERAUD]

Le 03/01/2014 12:30, Enrico a écrit :

On Wed, Dec 18, 2013 at 11:09 AM, Enrico  wrote:

On Tue, Dec 17, 2013 at 2:11 PM, Florian Vaussard
 wrote:

So I converted the iommu to DT (patches just sent), used pdata quirks
for the isp / mtv9032 data, added a few patches from other people
(mainly clk to fix a crash when deferring the omap3isp probe), and a few
small hacks. I get a 3.13-rc3 (+ board-removal part from Tony Lindgren)
to boot on DT with a working MT9V032 camera. The missing part is the DT
binding for the omap3isp, but I guess that we will have to wait a bit
more for this.

If you want to test, I have a development tree here [1]. Any feedback is
welcome.

Cheers,

Florian

[1] https://github.com/vaussard/linux/commits/overo-for-3.14/iommu/dt

Thanks Florian,

i will report what i get with my setup.

And here i am.

I can confirm it works, video source is tvp5150 (with platform data in
pdata-quirks.c) in bt656 mode.

Laurent, i used the two bt656 patches from your omap3isp/bt656 tree so
if you want to push it you can add a Tested-by me.

There is only one problem, but it's unrelated to your DT work.

It's an old problem (see for example [1] and [2]), seen by other
people too and it seems it's still there.
Basically if i capture with yavta while the system is idle then it
just waits without getting any frame.
If i add some cpu load (usually i do a "cat /dev/zero" in a ssh
terminal) it starts capturing correctly.

The strange thing is that i do get isp interrupts in the idle case, so
i don't know why they don't "propagate" to yavta.

Any hints on how to debug this?

Enrico

[1]: https://linuxtv.org/patch/7836/
[2]: https://www.mail-archive.com/linux-media@vger.kernel.org/msg44923.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
I have had what looked a lot like these problems before and it was due 
to a wrong configuration of the ccdc cropping regarding to the blanking. 
Could you send me the configuration of the pipeline that you apply with 
media-ctl, just in case this is the same problem.


Regards,
Julien BERAUD
--
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 v4 18/22] [media] em28xx: don't return -ENODEV for I2C xfer errors

[Mauro Carvalho Chehab]

Em Sun, 05 Jan 2014 21:49:57 +0100
Frank Schäfer  escreveu:

> Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab:
> > -ENODEV reports a permanent condition where a device is not found,
> > and used only during device probing or device removal, as stated
> > at  the V4L2 spec:
> > http://linuxtv.org/downloads/v4l-dvb-apis/gen_errors.html
> >
> > Except during device detection, this is not the case of I2C
> > transfer timeout errors.
> >
> > So, change them to return -EREMOTEIO instead.
> Mauro,
> 
> make a step back and think about this again.
> What are you doing here ?
> 
> You noticed that your HVR-950 device is working unstable for an unknown
> reason.
> Your specific device seems to be the only one, nobody could reproduce
> your issues so far and with the HVR-900 for example (which is nearly
> identical) we don't see any errors.
> 
> Then you started playing with the em28xx i2c transfer functions and
> noticed, that if you repeat the same i2c operations again and again, you
> finally have luck and it succeeds.
> From that you draw the conclusion, that i2c status 0x10 can't be a
> permanent error and hence 0x10 can't be -ENODEV.
> But that's wrong. Your problems could also be caused by
> sporadic/temporary circuit failures (such as loose contacts or whatever).
> Hardware errors can always happen and they are no reason for not using
> -ENODEV.

There is: it violates both V4L2 API and I2C error fault code. -ENODEV should
only be used on device probing time, according with
Documentation/i2c/fault-codes [1]:
ENODEV
Returned by driver probe() methods. 

[1] V4L2 spec also allows using it at device removal (e. g. when the
USB device is physically disconnected).

According with I2C fault codes, the proper error for a timeout is ETIMEDOUT:

ETIMEDOUT
This is returned by drivers when an operation took too much
time, and was aborted before it completed.

SMBus adapters may return it when an operation took more
time than allowed by the SMBus specification; for example,
when a slave stretches clocks too far.  I2C has no such
timeouts, but it's normal for I2C adapters to impose some
arbitrary limits (much longer than SMBus!) too.

> Whether -ENODEV is the appropriate error code or not depends on the
> devices error detection capabilities.

No, it depends if the code is called during probing time or not. Outside
device probing, an I2C driver should _never_ return ENODEV.

> The second assumption you make is that 0x10 means that a timeout occured.
> Which timeout are you talking about ?
> The only timeout I can think of can happen with devices which use clock
> stretching and hold down SCL too long.
> In any case you have no evidence for this theory, too.
> What we can say for sure ist that 0x10 includes the NACK error case
> because this happens when the slave device isn't present / doesn't respond.

You can't affirm that the device isn't present.

Let remind a conversation we had with Devin at #v4l back on Seg 27 Mai 2013,
where he checked those error codes on his datasheets:

(16:23:05) mchehab: btw, talking about missing data, register 0x05 is also not 
documented, although it is one of the most used register
(16:24:11) frankschaefer: This is definitely no kernel bloating.
(16:24:29) devinheitmueller: 5[5] is busy and 5[4] is aborted. Last I checked 
the driver implemented them properly so I didn't see a need for anything 
additional.
(16:25:17) frankschaefer: "busy" and "aborted" what ?
(16:25:22) devinheitmueller: i2c.
(16:25:40) devinheitmueller: Oh wait, busy was only on em2874. But aborted was 
on both versions of the chip.

Again, according with Documentation/i2c/fault-codes, for an I2C
aborted I/O, the error code is ETIMEDOUT.

> But that's all. We have no idea what 0x10 really means in detail.
> Maybe it also covers things like clock stretching timeouts or general
> SCL and SDA line errors, but we'll never know that without the datasheet.
> 
> Interpreting 0x10 as -ENODEV and all other errors as -EIO has been
> working fine for ages now.

When originally written, the only case where -ENODEV was returned was 
during I2C device probing time (I2C reads or writes with size equal to 
zero).

Latter patches corrupted it, and added -ENODEV to be returned outsize
the probing time.

> Changing it based on such fragile theories is no good idea.

The API specs is not a fragile theory. Is a document to be followed,
to warrant compliance.

> NACK from my side.
> 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  drivers/media/usb/em28xx/em28xx-i2c.c | 20 +---
> >  1 file changed, 9 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
> > b/drivers/media/usb/em28xx/em28xx-i2c.c
> > index 8b35aa51b9bb..c3ba8ace5c94 100644
> > --- a/drivers/media/usb/em28xx/em28xx-i2c.c
> > +++ b/drivers/media/usb/em28xx/em28xx-i2c.c

Re: [PATCH 1/3] [media] s5k5baf: Fix build warning

[Sachin Kamat]

On 24 December 2013 17:12, Sachin Kamat  wrote:
> Fixes the following warnings:
> drivers/media/i2c/s5k5baf.c: In function 's5k5baf_fw_parse':
> drivers/media/i2c/s5k5baf.c:362:3: warning:
> format '%d' expects argument of type 'int', but argument 3 has type 'size_t' 
> [-Wformat=]
> drivers/media/i2c/s5k5baf.c:383:4: warning:
> format '%d' expects argument of type 'int', but argument 4 has type 'size_t' 
> [-Wformat=]
>
> Signed-off-by: Sachin Kamat 
> Reported-by: kbuild test robot 
> ---
>  drivers/media/i2c/s5k5baf.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
> index e3b44a87460b..139bdd4f5dde 100644
> --- a/drivers/media/i2c/s5k5baf.c
> +++ b/drivers/media/i2c/s5k5baf.c
> @@ -359,7 +359,7 @@ static int s5k5baf_fw_parse(struct device *dev, struct 
> s5k5baf_fw **fw,
> int ret;
>
> if (count < S5K5BAG_FW_TAG_LEN + 1) {
> -   dev_err(dev, "firmware file too short (%d)\n", count);
> +   dev_err(dev, "firmware file too short (%zu)\n", count);
> return -EINVAL;
> }
>
> @@ -379,7 +379,7 @@ static int s5k5baf_fw_parse(struct device *dev, struct 
> s5k5baf_fw **fw,
>
> f = (struct s5k5baf_fw *)d;
> if (count < 1 + 2 * f->count) {
> -   dev_err(dev, "invalid firmware header (count=%d size=%d)\n",
> +   dev_err(dev, "invalid firmware header (count=%d size=%zu)\n",
> f->count, 2 * (count + S5K5BAG_FW_TAG_LEN));
> return -EINVAL;
> }
> --
> 1.7.9.5
>

Gentle ping on this series :)

-- 
With warm regards,
Sachin
--
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/2] em28xx: Fix support for audio-only interface on em2860

[Mauro Carvalho Chehab]

Driver probing for em2860 is broken, causing an oops and making the audio
module to be probed forever.

Fixes it.

Tested with a KWorld 305U (USB ID: eb1a:e305). Note:
the board silkscreen of this device says that it is a PVR-310U-Dongle ver. A.

Mauro Carvalho Chehab (2):
  em28xx: prevent registering wrong interfaces for audio-only
  em28xx: only initialize extensions on the main interface

 drivers/media/usb/em28xx/em28xx-cards.c | 12 
 drivers/media/usb/em28xx/em28xx-dvb.c   | 10 ++
 drivers/media/usb/em28xx/em28xx-input.c | 10 ++
 drivers/media/usb/em28xx/em28xx-video.c | 10 ++
 4 files changed, 42 insertions(+)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] em28xx: only initialize extensions on the main interface

[Mauro Carvalho Chehab]

For devices with a separated audio-only interface (em2860), call
em28xx_init_extension() only once.

That fixes a bug with Kworld 305U:

[  658.730715] em2860 #0: V4L2 video device registered as video1
[  658.730728] em2860 #0: V4L2 VBI device registered as vbi0
[  658.736907] em2860 #0: Remote control support is not available for this 
card.
[  658.736965] em2860 #1: Remote control support is not available for this 
card.
[  658.737230] [ cut here ]
[  658.737246] WARNING: CPU: 2 PID: 60 at lib/list_debug.c:36 
__list_add+0x8a/0xc0()
[  658.737256] list_add double add: new=8800a9a40410, 
prev=8800a9a40410, next=a08720d0.
[  658.737266] Modules linked in: tuner_xc2028 netconsole rc_hauppauge 
em28xx_rc rc_core tuner_simple tuner_types tda9887 tda8290 tuner tvp5150 
msp3400 em28xx_v4l em28xx tveeprom
 v4l2_common fuse ccm nf_conntrack_netbios_ns nf_conntrack_broadcast 
ipt_MASQUERADE ip6t_REJECT xt_conntrack ebtable_nat ebtable_broute bridge stp 
llc ebtable_filter ebtables ip6tabl
e_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle 
ip6table_security ip6table_raw ip6table_filter ip6_tables iptable_nat 
nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4
nf_nat nf_conntrack iptable_mangle iptable_security bnep iptable_raw vfat fat 
arc4 iwldvm mac80211 x86_pkg_temp_thermal coretemp kvm_intel nfsd iwlwifi 
snd_hda_codec_hdmi kvm snd_hda
_codec_realtek snd_hda_intel snd_hda_codec auth_rpcgss nfs_acl cfg80211 lockd 
snd_hwdep snd_seq btusb sunrpc crc32_pclmul bluetooth crc32c_intel 
snd_seq_device snd_pcm uvcvideo r8169
 ghash_clmulni_intel videobuf2_vmalloc videobuf2_memops videobuf2_core 
snd_page_alloc snd_timer snd videodev mei_me iTCO_wdt mii shpchp joydev mei 
media iTCO_vendor_support lpc_ich m
icrocode soundcore rfkill serio_raw i2c_i801 mfd_core nouveau i915 ttm 
i2c_algo_bit drm_kms_helper drm i2c_core mxm_wmi wmi video
[  658.738601] CPU: 2 PID: 60 Comm: kworker/2:1 Not tainted 3.13.0-rc1+ #18
[  658.738611] Hardware name: SAMSUNG ELECTRONICS CO., LTD. 
550P5C/550P7C/SAMSUNG_NP1234567890, BIOS P04ABI.013.130220.dg 02/20/2013
[  658.738624] Workqueue: events request_module_async [em28xx]
[  658.738646]  0009 8802209dfc68 816a3c96 
8802209dfcb0
[  658.738700]  8802209dfca0 8106aaad 8800a9a40410 
a08720d0
[  658.738754]  8800a9a40410  0080 
8802209dfd00
[  658.738814] Call Trace:
[  658.738836]  [] dump_stack+0x45/0x56
[  658.738851]  [] warn_slowpath_common+0x7d/0xa0
[  658.738864]  [] warn_slowpath_fmt+0x4c/0x50
[  658.738880]  [] ? em28xx_init_extension+0x1d/0x80 
[em28xx]
[  658.738898]  [] __list_add+0x8a/0xc0
[  658.738911]  [] em28xx_init_extension+0x38/0x80 
[em28xx]
[  658.738927]  [] request_module_async+0x19/0x110 
[em28xx]
[  658.738942]  [] process_one_work+0x1f5/0x510
[  658.738954]  [] ? process_one_work+0x193/0x510
[  658.738967]  [] worker_thread+0x11b/0x3a0
[  658.738979]  [] ? manage_workers.isra.24+0x2b0/0x2b0
[  658.738992]  [] kthread+0xff/0x120
[  658.739005]  [] ? kthread_create_on_node+0x250/0x250
[  658.739017]  [] ret_from_fork+0x7c/0xb0
[  658.739029]  [] ? kthread_create_on_node+0x250/0x250
[  658.739040] ---[ end trace c1acd24b354108de ]---
[  658.739051] em2860 #1: Remote control support is not available for this 
card.
[  658.742407] em28xx-audio.c: probing for em28xx Audio Vendor Class
[  658.742429] em28xx-audio.c: Copyright (C) 2006 Markus Rechberger
[  658.742440] em28xx-audio.c: Copyright (C) 2007-2011 Mauro Carvalho Chehab
[  658.744798] em28xx-audio.c: probing for em28xx Audio Vendor Class
[  658.744823] em28xx-audio.c: Copyright (C) 2006 Markus Rechberger
[  658.744836] em28xx-audio.c: Copyright (C) 2007-2011 Mauro Carvalho Chehab
[  658.746849] em28xx-audio.c: probing for em28xx Audio Vendor Class
[  658.746863] em28xx-audio.c: Copyright (C) 2006 Markus Rechberger
[  658.746874] em28xx-audio.c: Copyright (C) 2007-2011 Mauro Carvalho Chehab
...

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx-cards.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
b/drivers/media/usb/em28xx/em28xx-cards.c
index 95ba1cefc350..849725e5acb1 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -2779,6 +2779,18 @@ static void request_module_async(struct work_struct 
*work)
 * can be initialised right now. Otherwise, the module init
 * code will do it.
 */
+
+   /*
+* Devicdes with an audio-only interface also have a V4L/DVB/RC
+* interface. Don't register extensions twice on those devices.
+*/
+   if (dev->is_audio_only) {
+#if defined(CONFIG_MODULES) && defined(MODULE)
+   request_modul

[PATCH 1/2] em28xx: prevent registering wrong interfaces for audio-only

[Mauro Carvalho Chehab]

A few devices (em2860) use a separate interface for audio only
Audio Vendor Class USB. That interface should not be used by
Remote Controller, Analog TV or Digital TV.

Prevents initializing all non-audio extensions for the audio
only interface.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx-dvb.c   | 10 ++
 drivers/media/usb/em28xx/em28xx-input.c | 10 ++
 drivers/media/usb/em28xx/em28xx-video.c | 10 ++
 3 files changed, 30 insertions(+)

diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c 
b/drivers/media/usb/em28xx/em28xx-dvb.c
index 7fa1c804c34c..5c6be66ac858 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -992,6 +992,11 @@ static int em28xx_dvb_init(struct em28xx *dev)
int result = 0, mfe_shared = 0;
struct em28xx_dvb *dvb;
 
+   if (dev->is_audio_only) {
+   /* Shouldn't initialize IR for this interface */
+   return 0;
+   }
+
if (!dev->board.has_dvb) {
/* This device does not support the extension */
return 0;
@@ -1431,6 +1436,11 @@ static inline void prevent_sleep(struct dvb_frontend_ops 
*ops)
 
 static int em28xx_dvb_fini(struct em28xx *dev)
 {
+   if (dev->is_audio_only) {
+   /* Shouldn't initialize IR for this interface */
+   return 0;
+   }
+
if (!dev->board.has_dvb) {
/* This device does not support the extension */
return 0;
diff --git a/drivers/media/usb/em28xx/em28xx-input.c 
b/drivers/media/usb/em28xx/em28xx-input.c
index f3b629dd57ae..9a5dad96ff08 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -673,6 +673,11 @@ static int em28xx_ir_init(struct em28xx *dev)
u64 rc_type;
u16 i2c_rc_dev_addr = 0;
 
+   if (dev->is_audio_only) {
+   /* Shouldn't initialize IR for this interface */
+   return 0;
+   }
+
if (dev->board.buttons)
em28xx_init_buttons(dev);
 
@@ -802,6 +807,11 @@ static int em28xx_ir_fini(struct em28xx *dev)
 {
struct em28xx_IR *ir = dev->ir;
 
+   if (dev->is_audio_only) {
+   /* Shouldn't initialize IR for this interface */
+   return 0;
+   }
+
em28xx_shutdown_buttons(dev);
 
/* skip detach on non attached boards */
diff --git a/drivers/media/usb/em28xx/em28xx-video.c 
b/drivers/media/usb/em28xx/em28xx-video.c
index 856f18f9daea..ac3986b67201 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -1877,6 +1877,11 @@ static int em28xx_v4l2_open(struct file *filp)
 */
 static int em28xx_v4l2_fini(struct em28xx *dev)
 {
+   if (dev->is_audio_only) {
+   /* Shouldn't initialize IR for this interface */
+   return 0;
+   }
+
if (!dev->has_video) {
/* This device does not support the v4l2 extension */
return 0;
@@ -2208,6 +2213,11 @@ static int em28xx_v4l2_init(struct em28xx *dev)
unsigned int maxw;
struct v4l2_ctrl_handler *hdl = &dev->ctrl_handler;
 
+   if (dev->is_audio_only) {
+   /* Shouldn't initialize IR for this interface */
+   return 0;
+   }
+
if (!dev->has_video) {
/* This device does not support the v4l2 extension */
return 0;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] xc2028: disable device power-down because power state handling is broken

[Mauro Carvalho Chehab]

Em Mon, 30 Dec 2013 14:37:58 +0100
Frank Schäfer  escreveu:

> xc2028 power state handling is broken.
> I2C read/write operations fail when the device is powered down at that moment,
> which causes the get_rf_strength and get_rf_strength callbacks (and probably
> others, too) to fail.
> I don't know how to fix this properly, so disable the device power-down until
> anyone comes up with a better solution.
> 
> Signed-off-by: Frank Schäfer 
> ---
>  drivers/media/tuners/tuner-xc2028.c |4 +++-
>  1 Datei geändert, 3 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)
> 
> diff --git a/drivers/media/tuners/tuner-xc2028.c 
> b/drivers/media/tuners/tuner-xc2028.c
> index 4be5cf8..cb3dc5e 100644
> --- a/drivers/media/tuners/tuner-xc2028.c
> +++ b/drivers/media/tuners/tuner-xc2028.c
> @@ -1291,16 +1291,18 @@ static int xc2028_sleep(struct dvb_frontend *fe)
>   dump_stack();
>   }
>  
> + /* FIXME: device power-up/-down handling is broken */
> +/*
>   mutex_lock(&priv->lock);
>  
>   if (priv->firm_version < 0x0202)
>   rc = send_seq(priv, {0x00, XREG_POWER_DOWN, 0x00, 0x00});
>   else
>   rc = send_seq(priv, {0x80, XREG_POWER_DOWN, 0x00, 0x00});
> -
>   priv->state = XC2028_SLEEP;
>  
>   mutex_unlock(&priv->lock);
> +*/

This patch is completely broken.

First of all, there are both modprobe and config parameters that disables
the poweroff mode.

Second, it doesn't fix the bug, just hides it.

Third, it keeps the xc3028 energized, with spends power and heats the
device, with reduces its lifetime.

I'm working on a proper fix for it.

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 2/2] tuner-xc2028: Don't read status if device is powered down

[Mauro Carvalho Chehab]

That removes those timeout errors:

[ 3675.930940] xc2028 19-0061: Device is Xceive 3028 version 1.0, firmware 
version 2.7
[ 3676.060487] xc2028 19-0061: divisor= 00 00 8d d0 (freq=567.250)
[ 3676.349449] xc2028 19-0061: Putting xc2028/3028 into poweroff mode.
[ 3698.247645] xc2028 19-0061: xc2028_get_reg 0002 called
[ 3698.253276] em2860 #0: I2C transfer timeout on writing to addr 0xc2
[ 3698.253301] xc2028 19-0061: i2c input error: rc = -121 (should be 2)
[ 3698.253327] xc2028 19-0061: xc2028_signal called
[ 3698.253339] xc2028 19-0061: xc2028_get_reg 0002 called
[ 3698.259283] em2860 #0: I2C transfer timeout on writing to addr 0xc2
[ 3698.259312] xc2028 19-0061: i2c input error: rc = -121 (should be 2)

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/tuners/tuner-xc2028.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/media/tuners/tuner-xc2028.c 
b/drivers/media/tuners/tuner-xc2028.c
index 75afab718ba6..cca508d4aafb 100644
--- a/drivers/media/tuners/tuner-xc2028.c
+++ b/drivers/media/tuners/tuner-xc2028.c
@@ -267,6 +267,7 @@ static int check_device_status(struct xc2028_data *priv)
case XC2028_WAITING_FIRMWARE:
return -EAGAIN;
case XC2028_ACTIVE:
+   return 1;
case XC2028_SLEEP:
return 0;
case XC2028_NODEV:
@@ -913,6 +914,12 @@ static int xc2028_signal(struct dvb_frontend *fe, u16 
*strength)
if (rc < 0)
return rc;
 
+   /* If the device is sleeping, no channel is tuned */
+   if (!rc) {
+   *strength = 0;
+   return 0;
+   }
+
mutex_lock(&priv->lock);
 
/* Sync Lock Indicator */
@@ -960,6 +967,12 @@ static int xc2028_get_afc(struct dvb_frontend *fe, s32 
*afc)
if (rc < 0)
return rc;
 
+   /* If the device is sleeping, no channel is tuned */
+   if (!rc) {
+   *afc = 0;
+   return 0;
+   }
+
mutex_lock(&priv->lock);
 
/* Sync Lock Indicator */
@@ -1277,12 +1290,12 @@ static int xc2028_sleep(struct dvb_frontend *fe)
if (rc < 0)
return rc;
 
-   /* Avoid firmware reload on slow devices or if PM disabled */
-   if (no_poweroff || priv->ctrl.disable_power_mgmt)
+   /* Device is already in sleep mode */
+   if (!rc)
return 0;
 
-   /* Device is already in sleep mode */
-   if (priv->state == XC2028_SLEEP)
+   /* Avoid firmware reload on slow devices or if PM disabled */
+   if (no_poweroff || priv->ctrl.disable_power_mgmt)
return 0;
 
tuner_dbg("Putting xc2028/3028 into poweroff mode.\n");
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] tuner-xc2028: Fix xc3028 timeouts

[Mauro Carvalho Chehab]

When xc2028/3028 is powered down, it won't response to any command, until
a firmware is loaded. That means that reading frontend status will fail
with a timeout.

Also, any trial to put the device to sleep twice will fail.

This small series fix those two bugs.

Mauro Carvalho Chehab (2):
  tuner-xc2028: Don't try to sleep twice
  tuner-xc2028: Don't read status if device is powered down

 drivers/media/tuners/tuner-xc2028.c | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] tuner-xc2028: Don't try to sleep twice

[Mauro Carvalho Chehab]

Only send a power down command for the device if it is not already
in power down state. That prevents a timeout when trying to talk
with the device.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/tuners/tuner-xc2028.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/media/tuners/tuner-xc2028.c 
b/drivers/media/tuners/tuner-xc2028.c
index 1057da54c6e0..75afab718ba6 100644
--- a/drivers/media/tuners/tuner-xc2028.c
+++ b/drivers/media/tuners/tuner-xc2028.c
@@ -709,6 +709,8 @@ static int load_scode(struct dvb_frontend *fe, unsigned int 
type,
return 0;
 }
 
+static int xc2028_sleep(struct dvb_frontend *fe);
+
 static int check_firmware(struct dvb_frontend *fe, unsigned int type,
  v4l2_std_id std, __u16 int_freq)
 {
@@ -881,7 +883,7 @@ read_not_reliable:
return 0;
 
 fail:
-   priv->state = XC2028_SLEEP;
+   priv->state = XC2028_NO_FIRMWARE;
 
memset(&priv->cur_fw, 0, sizeof(priv->cur_fw));
if (retry_count < 8) {
@@ -891,6 +893,9 @@ fail:
goto retry;
}
 
+   /* Firmware didn't load. Put the device to sleep */
+   xc2028_sleep(fe);
+
if (rc == -ENOENT)
rc = -EINVAL;
return rc;
@@ -1276,6 +1281,10 @@ static int xc2028_sleep(struct dvb_frontend *fe)
if (no_poweroff || priv->ctrl.disable_power_mgmt)
return 0;
 
+   /* Device is already in sleep mode */
+   if (priv->state == XC2028_SLEEP)
+   return 0;
+
tuner_dbg("Putting xc2028/3028 into poweroff mode.\n");
if (debug > 1) {
tuner_dbg("Printing sleep stack trace:\n");
@@ -1289,7 +1298,8 @@ static int xc2028_sleep(struct dvb_frontend *fe)
else
rc = send_seq(priv, {0x80, XREG_POWER_DOWN, 0x00, 0x00});
 
-   priv->state = XC2028_SLEEP;
+   if (rc >= 0)
+   priv->state = XC2028_SLEEP;
 
mutex_unlock(&priv->lock);
 
@@ -1357,7 +1367,7 @@ static void load_firmware_cb(const struct firmware *fw,
 
if (rc < 0)
return;
-   priv->state = XC2028_SLEEP;
+   priv->state = XC2028_ACTIVE;
 }
 
 static int xc2028_set_config(struct dvb_frontend *fe, void *priv_cfg)
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFCv1 PATCH 22/27] v4l2-ctrls: add ctrl64 event.

[Hans Verkuil]

From: Hans Verkuil 

The current control event is not able to handle the 64-bit ranges or the
config_store. Add a new extended event that is able to handle this.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 61 ++--
 include/uapi/linux/videodev2.h   | 19 ++-
 2 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 8d6711e..0014324 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1091,8 +1091,10 @@ static void fill_event(struct v4l2_event *ev, struct 
v4l2_ctrl *ctrl, u32 change
ev->u.ctrl.flags = ctrl->flags;
if (ctrl->is_ptr)
ev->u.ctrl.value64 = 0;
-   else
+   else if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64)
ev->u.ctrl.value64 = *ctrl->cur.p_s64;
+   else
+   ev->u.ctrl.value = *ctrl->cur.p_s32;
ev->u.ctrl.minimum = ctrl->minimum;
ev->u.ctrl.maximum = ctrl->maximum;
if (ctrl->type == V4L2_CTRL_TYPE_MENU
@@ -1103,19 +1105,48 @@ static void fill_event(struct v4l2_event *ev, struct 
v4l2_ctrl *ctrl, u32 change
ev->u.ctrl.default_value = ctrl->default_value;
 }
 
+static void fill_event64(struct v4l2_event *ev, struct v4l2_ctrl *ctrl, u32 
changes)
+{
+   memset(ev->reserved, 0, sizeof(ev->reserved));
+   ev->type = V4L2_EVENT_CTRL64;
+   ev->id = ctrl->id;
+   ev->u.ctrl64.changes = changes;
+   ev->u.ctrl64.type = ctrl->type;
+   ev->u.ctrl64.config_store = ctrl->cur_store;
+   ev->u.ctrl64.flags = ctrl->flags;
+   if (ctrl->is_ptr)
+   ev->u.ctrl64.value64 = 0;
+   else if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64)
+   ev->u.ctrl64.value64 = *ctrl->cur.p_s64;
+   else
+   ev->u.ctrl64.value = *ctrl->cur.p_s32;
+   ev->u.ctrl64.minimum = ctrl->minimum;
+   ev->u.ctrl64.maximum = ctrl->maximum;
+   if (ctrl->type == V4L2_CTRL_TYPE_MENU
+   || ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU)
+   ev->u.ctrl64.step = 1;
+   else
+   ev->u.ctrl64.step = ctrl->step;
+   ev->u.ctrl64.default_value = ctrl->default_value;
+}
+
 static void send_event(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 changes)
 {
struct v4l2_event ev;
+   struct v4l2_event ev64;
struct v4l2_subscribed_event *sev;
 
if (list_empty(&ctrl->ev_subs))
return;
fill_event(&ev, ctrl, changes);
+   fill_event64(&ev64, ctrl, changes);
 
list_for_each_entry(sev, &ctrl->ev_subs, node)
if (sev->fh != fh ||
-   (sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK))
+   (sev->flags & V4L2_EVENT_SUB_FL_ALLOW_FEEDBACK)) {
v4l2_event_queue_fh(sev->fh, &ev);
+   v4l2_event_queue_fh(sev->fh, &ev64);
+   }
 }
 
 static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
@@ -3239,13 +3270,23 @@ int v4l2_ctrl_modify_range(struct v4l2_ctrl *ctrl,
ctrl->maximum = max;
ctrl->step = step;
ctrl->default_value = def;
-   c.value = *ctrl->cur.p_s32;
-   if (validate_new(ctrl, &c))
-   c.value = def;
-   if (c.value != *ctrl->cur.p_s32)
-   ret = set_ctrl(NULL, ctrl, &c, V4L2_EVENT_CTRL_CH_RANGE);
-   else
-   send_event(NULL, ctrl, V4L2_EVENT_CTRL_CH_RANGE);
+   if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64) {
+   c.value64 = *ctrl->cur.p_s64;
+   if (validate_new(ctrl, &c))
+   c.value64 = def;
+   if (c.value64 != *ctrl->cur.p_s64)
+   ret = set_ctrl(NULL, ctrl, &c, 
V4L2_EVENT_CTRL_CH_RANGE);
+   else
+   send_event(NULL, ctrl, V4L2_EVENT_CTRL_CH_RANGE);
+   } else {
+   c.value = *ctrl->cur.p_s32;
+   if (validate_new(ctrl, &c))
+   c.value = def;
+   if (c.value != *ctrl->cur.p_s32)
+   ret = set_ctrl(NULL, ctrl, &c, 
V4L2_EVENT_CTRL_CH_RANGE);
+   else
+   send_event(NULL, ctrl, V4L2_EVENT_CTRL_CH_RANGE);
+   }
v4l2_ctrl_unlock(ctrl);
return ret;
 }
@@ -3324,7 +3365,7 @@ EXPORT_SYMBOL(v4l2_ctrl_log_status);
 int v4l2_ctrl_subscribe_event(struct v4l2_fh *fh,
const struct v4l2_event_subscription *sub)
 {
-   if (sub->type == V4L2_EVENT_CTRL)
+   if (sub->type == V4L2_EVENT_CTRL || sub->type == V4L2_EVENT_CTRL64)
return v4l2_event_subscribe(fh, sub, 0, &v4l2_ctrl_sub_ev_ops);
return -EINVAL;
 }
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 78aba44..afa335d 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1773,6 +1773,7 @@ struc

[RFCv1 PATCH 19/27] v4l2-ctrls: type_ops can handle matrix elements.

[Hans Verkuil]

From: Hans Verkuil 

Extend the control type operations to handle matrix elements.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 40 
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index b6edd81..3655b51 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1126,14 +1126,16 @@ static bool std_equal(const struct v4l2_ctrl *ctrl, u32 
idx,
case V4L2_CTRL_TYPE_BUTTON:
return false;
case V4L2_CTRL_TYPE_STRING:
+   idx *= ctrl->elem_size;
/* strings are always 0-terminated */
-   return !strcmp(ptr1.p_char, ptr2.p_char);
+   return !strcmp(ptr1.p_char + idx, ptr2.p_char + idx);
case V4L2_CTRL_TYPE_INTEGER64:
-   return *ptr1.p_s64 == *ptr2.p_s64;
+   return ptr1.p_s64[idx] == ptr2.p_s64[idx];
default:
-   if (ctrl->is_ptr)
-   return !memcmp(ptr1.p, ptr2.p, ctrl->elem_size);
-   return *ptr1.p_s32 == *ptr2.p_s32;
+   if (ctrl->is_int)
+   return ptr1.p_s32[idx] == ptr2.p_s32[idx];
+   idx *= ctrl->elem_size;
+   return !memcmp(ptr1.p + idx, ptr2.p + idx, ctrl->elem_size);
}
 }
 
@@ -1142,18 +1144,19 @@ static void std_init(const struct v4l2_ctrl *ctrl, u32 
idx,
 {
switch (ctrl->type) {
case V4L2_CTRL_TYPE_STRING:
-   memset(ptr.p_char, ' ', ctrl->minimum);
-   ptr.p_char[ctrl->minimum] = '\0';
+   idx *= ctrl->elem_size;
+   memset(ptr.p_char + idx, ' ', ctrl->minimum);
+   ptr.p_char[idx + ctrl->minimum] = '\0';
break;
case V4L2_CTRL_TYPE_INTEGER64:
-   *ptr.p_s64 = ctrl->default_value;
+   ptr.p_s64[idx] = ctrl->default_value;
break;
case V4L2_CTRL_TYPE_INTEGER:
case V4L2_CTRL_TYPE_INTEGER_MENU:
case V4L2_CTRL_TYPE_MENU:
case V4L2_CTRL_TYPE_BITMASK:
case V4L2_CTRL_TYPE_BOOLEAN:
-   *ptr.p_s32 = ctrl->default_value;
+   ptr.p_s32[idx] = ctrl->default_value;
break;
default:
break;
@@ -1216,36 +1219,37 @@ static int std_validate(const struct v4l2_ctrl *ctrl, 
u32 idx,
 
switch (ctrl->type) {
case V4L2_CTRL_TYPE_INTEGER:
-   return ROUND_TO_RANGE(*ptr.p_s32, u32, ctrl);
+   return ROUND_TO_RANGE(ptr.p_s32[idx], u32, ctrl);
case V4L2_CTRL_TYPE_INTEGER64:
-   return ROUND_TO_RANGE(*ptr.p_s64, u64, ctrl);
+   return ROUND_TO_RANGE(ptr.p_s64[idx], u64, ctrl);
 
case V4L2_CTRL_TYPE_BOOLEAN:
-   *ptr.p_s32 = !!*ptr.p_s32;
+   ptr.p_s32[idx] = !!ptr.p_s32[idx];
return 0;
 
case V4L2_CTRL_TYPE_MENU:
case V4L2_CTRL_TYPE_INTEGER_MENU:
-   if (*ptr.p_s32 < ctrl->minimum || *ptr.p_s32 > ctrl->maximum)
+   if (ptr.p_s32[idx] < ctrl->minimum || ptr.p_s32[idx] > 
ctrl->maximum)
return -ERANGE;
-   if (ctrl->menu_skip_mask & (1 << *ptr.p_s32))
+   if (ctrl->menu_skip_mask & (1 << ptr.p_s32[idx]))
return -EINVAL;
if (ctrl->type == V4L2_CTRL_TYPE_MENU &&
-   ctrl->qmenu[*ptr.p_s32][0] == '\0')
+   ctrl->qmenu[ptr.p_s32[idx]][0] == '\0')
return -EINVAL;
return 0;
 
case V4L2_CTRL_TYPE_BITMASK:
-   *ptr.p_s32 &= ctrl->maximum;
+   ptr.p_s32[idx] &= ctrl->maximum;
return 0;
 
case V4L2_CTRL_TYPE_BUTTON:
case V4L2_CTRL_TYPE_CTRL_CLASS:
-   *ptr.p_s32 = 0;
+   ptr.p_s32[idx] = 0;
return 0;
 
case V4L2_CTRL_TYPE_STRING:
-   len = strlen(ptr.p_char);
+   idx *= ctrl->elem_size;
+   len = strlen(ptr.p_char + idx);
if (len < ctrl->minimum)
return -ERANGE;
if ((len - ctrl->minimum) % ctrl->step)
-- 
1.8.5.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

[RFCv1 PATCH 10/27] v4l2-ctrls: create type_ops.

[Hans Verkuil]

From: Hans Verkuil 

Since properties can have non-standard types we need to be able to do 
type-specific
checks etc. In order to make that easy type operations are added. There are four
operations:

- equal: check if two values are equal
- init: initialize a value
- log: log the value
- validate: validate a new value

This patch uses the v4l2_ctrl_ptr union for the first time.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 267 ++-
 include/media/v4l2-ctrls.h   |  21 +++
 2 files changed, 190 insertions(+), 98 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 3927755..688ca31 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1118,6 +1118,149 @@ static void send_event(struct v4l2_fh *fh, struct 
v4l2_ctrl *ctrl, u32 changes)
v4l2_event_queue_fh(sev->fh, &ev);
 }
 
+static bool std_equal(const struct v4l2_ctrl *ctrl,
+ union v4l2_ctrl_ptr ptr1,
+ union v4l2_ctrl_ptr ptr2)
+{
+   switch (ctrl->type) {
+   case V4L2_CTRL_TYPE_BUTTON:
+   return false;
+   case V4L2_CTRL_TYPE_STRING:
+   /* strings are always 0-terminated */
+   return !strcmp(ptr1.p_char, ptr2.p_char);
+   case V4L2_CTRL_TYPE_INTEGER64:
+   return *ptr1.p_s64 == *ptr2.p_s64;
+   default:
+   if (ctrl->is_ptr)
+   return !memcmp(ptr1.p, ptr2.p, ctrl->elem_size);
+   return *ptr1.p_s32 == *ptr2.p_s32;
+   }
+}
+
+static void std_init(const struct v4l2_ctrl *ctrl,
+union v4l2_ctrl_ptr ptr)
+{
+   switch (ctrl->type) {
+   case V4L2_CTRL_TYPE_STRING:
+   memset(ptr.p_char, ' ', ctrl->minimum);
+   ptr.p_char[ctrl->minimum] = '\0';
+   break;
+   case V4L2_CTRL_TYPE_INTEGER64:
+   *ptr.p_s64 = ctrl->default_value;
+   break;
+   case V4L2_CTRL_TYPE_INTEGER:
+   case V4L2_CTRL_TYPE_INTEGER_MENU:
+   case V4L2_CTRL_TYPE_MENU:
+   case V4L2_CTRL_TYPE_BITMASK:
+   case V4L2_CTRL_TYPE_BOOLEAN:
+   *ptr.p_s32 = ctrl->default_value;
+   break;
+   default:
+   break;
+   }
+}
+
+static void std_log(const struct v4l2_ctrl *ctrl)
+{
+   union v4l2_ctrl_ptr ptr = ctrl->stores[0];
+
+   switch (ctrl->type) {
+   case V4L2_CTRL_TYPE_INTEGER:
+   pr_cont("%d", *ptr.p_s32);
+   break;
+   case V4L2_CTRL_TYPE_BOOLEAN:
+   pr_cont("%s", *ptr.p_s32 ? "true" : "false");
+   break;
+   case V4L2_CTRL_TYPE_MENU:
+   pr_cont("%s", ctrl->qmenu[*ptr.p_s32]);
+   break;
+   case V4L2_CTRL_TYPE_INTEGER_MENU:
+   pr_cont("%lld", ctrl->qmenu_int[*ptr.p_s32]);
+   break;
+   case V4L2_CTRL_TYPE_BITMASK:
+   pr_cont("0x%08x", *ptr.p_s32);
+   break;
+   case V4L2_CTRL_TYPE_INTEGER64:
+   pr_cont("%lld", *ptr.p_s64);
+   break;
+   case V4L2_CTRL_TYPE_STRING:
+   pr_cont("%s", ptr.p_char);
+   break;
+   default:
+   pr_cont("unknown type %d", ctrl->type);
+   break;
+   }
+}
+
+/* Round towards the closest legal value */
+#define ROUND_TO_RANGE(val, offset_type, ctrl) \
+({ \
+   offset_type offset; \
+   val += (ctrl)->step / 2;\
+   val = clamp_t(typeof(val), val, \
+ (ctrl)->minimum, (ctrl)->maximum);\
+   offset = (val) - (ctrl)->minimum;   \
+   offset = (ctrl)->step * (offset / (ctrl)->step);\
+   val = (ctrl)->minimum + offset; \
+   0;  \
+})
+
+/* Validate a new control */
+static int std_validate(const struct v4l2_ctrl *ctrl,
+   union v4l2_ctrl_ptr ptr)
+{
+   size_t len;
+
+   switch (ctrl->type) {
+   case V4L2_CTRL_TYPE_INTEGER:
+   return ROUND_TO_RANGE(*ptr.p_s32, u32, ctrl);
+   case V4L2_CTRL_TYPE_INTEGER64:
+   return ROUND_TO_RANGE(*ptr.p_s64, u64, ctrl);
+
+   case V4L2_CTRL_TYPE_BOOLEAN:
+   *ptr.p_s32 = !!*ptr.p_s32;
+   return 0;
+
+   case V4L2_CTRL_TYPE_MENU:
+   case V4L2_CTRL_TYPE_INTEGER_MENU:
+   if (*ptr.p_s32 < ctrl->minimum || *ptr.p_s32 > ctrl->maximum)
+   return -ERANGE;
+   if (ctrl->menu_skip_mask & (1 << *ptr.p_s32))
+   return -EINVAL;
+   if (ctrl->type == V4L2_CTRL_TYPE_MENU &&
+   ctrl->qmenu[*ptr

[RFCv1 PATCH 14/27] v4l2-ctrls: compare values only once.

[Hans Verkuil]

From: Hans Verkuil 

When setting a control the control's new value is compared to the current
value twice: once by new_to_store(), once by cluster_changed(). Not a big
deal when dealing with simple values, but it can be a problem when dealing
with compound types or matrices. So fix this: cluster_changed() sets the
has_changed flag, which is used by new_to_store() instead of having to do
another compare.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 16 ++--
 include/media/v4l2-ctrls.h   |  3 +++
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 66724b7..a1c5e3e 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1389,8 +1389,10 @@ static void new_to_store(struct v4l2_fh *fh, struct 
v4l2_ctrl *ctrl, u32 ch_flag
 
if (ctrl == NULL)
return;
-   changed = !ctrl->type_ops->equal(ctrl, store, ctrl->new);
-   ptr_to_ptr(ctrl, ctrl->new, store);
+
+   changed = ctrl->has_changed;
+   if (changed)
+   ptr_to_ptr(ctrl, ctrl->new, store);
 
if (ch_flags & V4L2_EVENT_CTRL_CH_FLAGS) {
/* Note: CH_FLAGS is only set for auto clusters. */
@@ -1436,17 +1438,19 @@ static void store_to_new(struct v4l2_ctrl *ctrl, 
unsigned store)
value that differs from the current value. */
 static int cluster_changed(struct v4l2_ctrl *master)
 {
-   int diff = 0;
+   bool changed = false;
int i;
 
-   for (i = 0; !diff && i < master->ncontrols; i++) {
+   for (i = 0; i < master->ncontrols; i++) {
struct v4l2_ctrl *ctrl = master->cluster[i];
 
if (ctrl == NULL)
continue;
-   diff = !ctrl->type_ops->equal(ctrl, ctrl->stores[0], ctrl->new);
+   ctrl->has_changed = !ctrl->type_ops->equal(ctrl,
+   ctrl->stores[0], ctrl->new);
+   changed |= ctrl->has_changed;
}
-   return diff;
+   return changed;
 }
 
 /* Control range checking */
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 2b9b2da..9ce740d 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -99,6 +99,8 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, 
void *priv);
   * @is_new:   Set when the user specified a new value for this control. It
   *is also set when called from v4l2_ctrl_handler_setup. Drivers
   *should never set this flag.
+  * @has_changed: Set when the current value differs from the new value. 
Drivers
+  *should never use this flag.
   * @is_private: If set, then this control is private to its handler and it
   *will not be added to any other handlers. Drivers can set
   *this flag.
@@ -165,6 +167,7 @@ struct v4l2_ctrl {
unsigned int done:1;
 
unsigned int is_new:1;
+   unsigned int has_changed:1;
unsigned int is_private:1;
unsigned int is_auto:1;
unsigned int is_int:1;
-- 
1.8.5.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

[RFCv1 PATCH 12/27] v4l2-ctrls: add initial support for configuration stores.

[Hans Verkuil]

From: Hans Verkuil 

Add support for multiple configuration stores. Three new fields are added to
v4l2_ctrl:

nstores: the number of stores for this control
store: the current store: for use by the control ops
cur_store: the store associated with the current value. Normally 0, but
it can be a specific store if stores map to shadow registers.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 157 +--
 include/media/v4l2-ctrls.h   |  11 ++-
 2 files changed, 122 insertions(+), 46 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 9b0362e..3e32e21 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1162,7 +1162,7 @@ static void std_init(const struct v4l2_ctrl *ctrl,
 
 static void std_log(const struct v4l2_ctrl *ctrl)
 {
-   union v4l2_ctrl_ptr ptr = ctrl->stores[0];
+   union v4l2_ctrl_ptr ptr = ctrl->stores[ctrl->cur_store];
 
switch (ctrl->type) {
case V4L2_CTRL_TYPE_INTEGER:
@@ -1304,6 +1304,13 @@ static int new_to_user(struct v4l2_ext_control *c,
return ptr_to_user(c, ctrl, ctrl->new);
 }
 
+static int store_to_user(struct v4l2_ext_control *c,
+struct v4l2_ctrl *ctrl,
+unsigned store)
+{
+   return ptr_to_user(c, ctrl, ctrl->stores[store ? store : 
ctrl->cur_store]);
+}
+
 /* Helper function: copy the caller-provider value to the given control value 
*/
 static int user_to_ptr(struct v4l2_ext_control *c,
   struct v4l2_ctrl *ctrl,
@@ -1375,14 +1382,15 @@ static void ptr_to_ptr(struct v4l2_ctrl *ctrl,
 }
 
 /* Copy the new value to the current value. */
-static void new_to_cur(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 
ch_flags)
+static void new_to_store(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 
ch_flags)
 {
+   union v4l2_ctrl_ptr store = ctrl->stores[ctrl->store];
bool changed;
 
if (ctrl == NULL)
return;
-   changed = !ctrl->type_ops->equal(ctrl, ctrl->stores[0], ctrl->new);
-   ptr_to_ptr(ctrl, ctrl->new, ctrl->stores[0]);
+   changed = !ctrl->type_ops->equal(ctrl, store, ctrl->new);
+   ptr_to_ptr(ctrl, ctrl->new, store);
 
if (ch_flags & V4L2_EVENT_CTRL_CH_FLAGS) {
/* Note: CH_FLAGS is only set for auto clusters. */
@@ -1709,6 +1717,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
enum v4l2_ctrl_type type,
s64 min, s64 max, u64 step, s64 def,
u32 elem_size,
+   u32 nstores, u32 initial_store,
u32 flags, const char * const *qmenu,
const s64 *qmenu_int, void *priv)
 {
@@ -1758,14 +1767,25 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
handler_set_err(hdl, -ERANGE);
return NULL;
}
+   if (nstores &&
+   (type == V4L2_CTRL_TYPE_BUTTON ||
+type == V4L2_CTRL_TYPE_CTRL_CLASS)) {
+   handler_set_err(hdl, -EINVAL);
+   return NULL;
+   }
+
+   if (nstores)
+   flags |= V4L2_CTRL_FLAG_CAN_STORE;
+   sz_extra = (1 + nstores) * sizeof(union v4l2_ctrl_ptr);
 
-   sz_extra = sizeof(union v4l2_ctrl_ptr);
if (type == V4L2_CTRL_TYPE_BUTTON)
flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
else if (type == V4L2_CTRL_TYPE_CTRL_CLASS)
flags |= V4L2_CTRL_FLAG_READ_ONLY;
else if (type == V4L2_CTRL_TYPE_STRING || type >= V4L2_PROP_TYPES)
-   sz_extra += 2 * elem_size;
+   sz_extra += (2 + nstores) * elem_size;
+   else
+   sz_extra += nstores * elem_size;
 
ctrl = kzalloc(sizeof(*ctrl) + sz_extra, GFP_KERNEL);
if (ctrl == NULL) {
@@ -1791,22 +1811,27 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
ctrl->is_ptr = type >= V4L2_PROP_TYPES || ctrl->is_string;
ctrl->is_int = !ctrl->is_ptr && type != V4L2_CTRL_TYPE_INTEGER64;
ctrl->elem_size = elem_size;
+   ctrl->nstores = nstores;
+   ctrl->cur_store = initial_store;
if (type == V4L2_CTRL_TYPE_MENU)
ctrl->qmenu = qmenu;
else if (type == V4L2_CTRL_TYPE_INTEGER_MENU)
ctrl->qmenu_int = qmenu_int;
ctrl->priv = priv;
ctrl->cur.val = ctrl->val = def;
-   props = &ctrl->stores[1];
+   props = &ctrl->stores[1 + nstores];
 
if (ctrl->is_ptr) {
ctrl->p = ctrl->new.p = props;
-   ctrl->stores[0].p = props + elem_size;
+   for (s = 0; s <= nstores; s++)
+   ctrl->stores[s].p = props + (s + 1) * elem_size;
} else {
ctrl->new.p = &ctrl->val;
ctrl->stores[0].p = &ctrl->cur.val;
+   f

[RFCv1 PATCH 06/27] v4l2-ctrls: add support for properties.

[Hans Verkuil]

From: Hans Verkuil 

This patch implements initial support for simple properties.

For the most part the changes are fairly obvious (basic support for is_ptr
types, the type_is_int function is replaced by a is_int bitfield, and
v4l2_query_ext_ctrl is added), but one change needs more explanation:

The v4l2_ctrl struct adds a 'new' field and a 'stores' array at the end
of the struct. This is in preparation for following patches where each control
can have multiple configuration stores. The idea is that stores[0] is the 
current
control value, stores[1] etc. are the control values for each configuration 
store
and the 'new' value can be accessed through 'stores[-1]', i.e. the 'new' field.

These new fields use the v4l2_ctrl_ptr union, which is a pointer to a control
value.

Note that these two new fields are not yet actually used.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 231 ++-
 include/media/v4l2-ctrls.h   |  38 +-
 2 files changed, 206 insertions(+), 63 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 10bfab6..3927755 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1081,20 +1081,6 @@ void v4l2_ctrl_fill(u32 id, const char **name, const 
char **unit,
 }
 EXPORT_SYMBOL(v4l2_ctrl_fill);
 
-/* Helper function to determine whether the control type is compatible with
-   VIDIOC_G/S_CTRL. */
-static bool type_is_int(const struct v4l2_ctrl *ctrl)
-{
-   switch (ctrl->type) {
-   case V4L2_CTRL_TYPE_INTEGER64:
-   case V4L2_CTRL_TYPE_STRING:
-   /* Nope, these need v4l2_ext_control */
-   return false;
-   default:
-   return true;
-   }
-}
-
 static void fill_event(struct v4l2_event *ev, struct v4l2_ctrl *ctrl, u32 
changes)
 {
memset(ev->reserved, 0, sizeof(ev->reserved));
@@ -1103,7 +1089,7 @@ static void fill_event(struct v4l2_event *ev, struct 
v4l2_ctrl *ctrl, u32 change
ev->u.ctrl.changes = changes;
ev->u.ctrl.type = ctrl->type;
ev->u.ctrl.flags = ctrl->flags;
-   if (ctrl->type == V4L2_CTRL_TYPE_STRING)
+   if (ctrl->is_ptr)
ev->u.ctrl.value64 = 0;
else
ev->u.ctrl.value64 = ctrl->cur.val64;
@@ -1138,6 +1124,9 @@ static int cur_to_user(struct v4l2_ext_control *c,
 {
u32 len;
 
+   if (ctrl->is_ptr && !ctrl->is_string)
+   return copy_to_user(c->p, ctrl->cur.p, ctrl->elem_size);
+
switch (ctrl->type) {
case V4L2_CTRL_TYPE_STRING:
len = strlen(ctrl->cur.string);
@@ -1165,6 +1154,9 @@ static int user_to_new(struct v4l2_ext_control *c,
u32 size;
 
ctrl->is_new = 1;
+   if (ctrl->is_ptr && !ctrl->is_string)
+   return copy_from_user(ctrl->p, c->p, ctrl->elem_size);
+
switch (ctrl->type) {
case V4L2_CTRL_TYPE_INTEGER64:
ctrl->val64 = c->value64;
@@ -1199,6 +1191,9 @@ static int new_to_user(struct v4l2_ext_control *c,
 {
u32 len;
 
+   if (ctrl->is_ptr && !ctrl->is_string)
+   return copy_to_user(c->p, ctrl->p, ctrl->elem_size);
+
switch (ctrl->type) {
case V4L2_CTRL_TYPE_STRING:
len = strlen(ctrl->string);
@@ -1225,6 +1220,7 @@ static void new_to_cur(struct v4l2_fh *fh, struct 
v4l2_ctrl *ctrl, u32 ch_flags)
 
if (ctrl == NULL)
return;
+
switch (ctrl->type) {
case V4L2_CTRL_TYPE_BUTTON:
changed = true;
@@ -1239,8 +1235,13 @@ static void new_to_cur(struct v4l2_fh *fh, struct 
v4l2_ctrl *ctrl, u32 ch_flags)
ctrl->cur.val64 = ctrl->val64;
break;
default:
-   changed = ctrl->val != ctrl->cur.val;
-   ctrl->cur.val = ctrl->val;
+   if (ctrl->is_ptr) {
+   changed = memcmp(ctrl->p, ctrl->cur.p, ctrl->elem_size);
+   memcpy(ctrl->cur.p, ctrl->p, ctrl->elem_size);
+   } else {
+   changed = ctrl->val != ctrl->cur.val;
+   ctrl->cur.val = ctrl->val;
+   }
break;
}
if (ch_flags & V4L2_EVENT_CTRL_CH_FLAGS) {
@@ -1280,7 +1281,10 @@ static void cur_to_new(struct v4l2_ctrl *ctrl)
ctrl->val64 = ctrl->cur.val64;
break;
default:
-   ctrl->val = ctrl->cur.val;
+   if (ctrl->is_ptr)
+   memcpy(ctrl->p, ctrl->cur.p, ctrl->elem_size);
+   else
+   ctrl->val = ctrl->cur.val;
break;
}
 }
@@ -1493,7 +1497,7 @@ static struct v4l2_ctrl_ref *find_private_ref(
   VIDIOC_G/S_CTRL. */
if (V4L2_CTRL_ID2CLASS(ref->ctrl->id) == V4L2_CTRL_CLASS_USER &&
V4L2_CTRL_DRIVER_PRIV(ref->ctrl->id)) {
- 

[RFCv1 PATCH 17/27] v4l2-ctrls: new strings and props must be accessed through the new field.

[Hans Verkuil]

From: Hans Verkuil 

Require that 'new' string and pointer values are accessed through the 'new'
field instead of through the union. This reduces the union to just val and
val64.

Signed-off-by: Hans Verkuil 
---
 drivers/media/radio/si4713/si4713.c| 4 ++--
 drivers/media/v4l2-core/v4l2-ctrls.c   | 3 +--
 drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c | 2 +-
 include/media/v4l2-ctrls.h | 2 --
 4 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/media/radio/si4713/si4713.c 
b/drivers/media/radio/si4713/si4713.c
index 07d5153..718e10d 100644
--- a/drivers/media/radio/si4713/si4713.c
+++ b/drivers/media/radio/si4713/si4713.c
@@ -1098,11 +1098,11 @@ static int si4713_s_ctrl(struct v4l2_ctrl *ctrl)
 
switch (ctrl->id) {
case V4L2_CID_RDS_TX_PS_NAME:
-   ret = si4713_set_rds_ps_name(sdev, ctrl->string);
+   ret = si4713_set_rds_ps_name(sdev, ctrl->new.p_char);
break;
 
case V4L2_CID_RDS_TX_RADIO_TEXT:
-   ret = si4713_set_rds_radio_text(sdev, ctrl->string);
+   ret = si4713_set_rds_radio_text(sdev, ctrl->new.p_char);
break;
 
case V4L2_CID_TUNE_ANTENNA_CAPACITOR:
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index e7effc8..bb7f4ccb 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1842,8 +1842,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
props = &ctrl->stores[1 + nstores];
 
if (ctrl->is_ptr) {
-   ctrl->p = ctrl->new.p = props;
-   for (s = 0; s <= nstores; s++)
+   for (s = -1; s <= (int)nstores; s++)
ctrl->stores[s].p = props + (s + 1) * elem_size;
} else {
ctrl->new.p = &ctrl->val;
diff --git a/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c 
b/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
index d582c5b..a4a6bee 100644
--- a/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/staging/media/solo6x10/solo6x10-v4l2-enc.c
@@ -1127,7 +1127,7 @@ static int solo_s_ctrl(struct v4l2_ctrl *ctrl)
solo_motion_toggle(solo_enc, ctrl->val);
return 0;
case V4L2_CID_OSD_TEXT:
-   strcpy(solo_enc->osd_text, ctrl->string);
+   strcpy(solo_enc->osd_text, ctrl->new.p_char);
err = solo_osd_print(solo_enc);
return err;
default:
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 3bfd9a6..b735b5c 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -201,8 +201,6 @@ struct v4l2_ctrl {
union {
s32 val;
s64 val64;
-   char *string;
-   void *p;
};
union v4l2_ctrl_ptr *stores;
union v4l2_ctrl_ptr new;
-- 
1.8.5.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

[RFCv1 PATCH 09/27] videodev2.h: add config_store to v4l2_ext_controls

[Hans Verkuil]

From: Hans Verkuil 

The ctrl_class is fairly pointless when used with drivers that use the control
framework: you can just fill in 0 and it will just work fine. There are still
some old unconverted drivers that do not support 0 and instead want the control
class there. The idea being that all controls in the list all belong to that
class. This was done to simplify drivers in the absence of the control 
framework.

When using the control framework the framework itself is smart enough to allow
controls of any class to be included in the control list.

Since configuration store IDs are in the range 1..255 (or so, in any case a 
relatively
small non-zero positive integer) it makes sense to effectively rename ctrl_class
to config_store. Set it to 0 and you get the normal behavior (you change the 
current
control value), set it to a configuration store ID and you get/set the control 
for
that store.

Signed-off-by: Hans Verkuil 
---
 include/uapi/linux/videodev2.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 0803da9..789f876 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1233,7 +1233,10 @@ struct v4l2_ext_control {
 } __attribute__ ((packed));
 
 struct v4l2_ext_controls {
-   __u32 ctrl_class;
+   union {
+   __u32 ctrl_class;
+   __u32 config_store;
+   };
__u32 count;
__u32 error_idx;
__u32 reserved[2];
-- 
1.8.5.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

[RFCv1 PATCH 15/27] v4l2-ctrls: prepare for matrix support: add cols & rows fields.

[Hans Verkuil]

From: Hans Verkuil 

Add cols and rows fields to the core control structures in preparation
for matrix support.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 24 
 include/media/v4l2-ctrls.h   |  6 ++
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index a1c5e3e..8ed8a70 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1729,7 +1729,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
u32 id, const char *name, const char *unit,
enum v4l2_ctrl_type type,
s64 min, s64 max, u64 step, s64 def,
-   u32 elem_size,
+   u32 cols, u32 rows, u32 elem_size,
u32 nstores, u32 initial_store,
u32 flags, const char * const *qmenu,
const s64 *qmenu_int, void *priv)
@@ -1743,6 +1743,11 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
if (hdl->error)
return NULL;
 
+   if (cols == 0)
+   cols = 1;
+   if (rows == 0)
+   rows = 1;
+
if (type == V4L2_CTRL_TYPE_INTEGER64)
elem_size = sizeof(s64);
else if (type == V4L2_CTRL_TYPE_STRING)
@@ -1823,6 +1828,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
ctrl->is_string = type == V4L2_CTRL_TYPE_STRING;
ctrl->is_ptr = type >= V4L2_PROP_TYPES || ctrl->is_string;
ctrl->is_int = !ctrl->is_ptr && type != V4L2_CTRL_TYPE_INTEGER64;
+   ctrl->cols = cols;
+   ctrl->rows = rows;
ctrl->elem_size = elem_size;
ctrl->nstores = nstores;
ctrl->cur_store = initial_store;
@@ -1893,8 +1900,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct 
v4l2_ctrl_handler *hdl,
 
ctrl = v4l2_ctrl_new(hdl, cfg->ops, cfg->type_ops, cfg->id, name, unit,
type, min, max,
-   is_menu ? cfg->menu_skip_mask : step,
-   def, cfg->elem_size,
+   is_menu ? cfg->menu_skip_mask : step, def,
+   cfg->cols, cfg->rows, cfg->elem_size,
cfg->nstores, cfg->initial_store,
flags, qmenu, qmenu_int, priv);
if (ctrl)
@@ -1921,7 +1928,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std(struct 
v4l2_ctrl_handler *hdl,
return NULL;
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, unit, type,
-min, max, step, def, 0, 0, 0,
+min, max, step, def, 0, 0, 0, 0, 0,
 flags, NULL, NULL, NULL);
 }
 EXPORT_SYMBOL(v4l2_ctrl_new_std);
@@ -1955,7 +1962,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct 
v4l2_ctrl_handler *hdl,
return NULL;
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, unit, type,
-0, max, mask, def, 0, 0, 0,
+0, max, mask, def, 0, 0, 0, 0, 0,
 flags, qmenu, qmenu_int, NULL);
 }
 EXPORT_SYMBOL(v4l2_ctrl_new_std_menu);
@@ -1988,7 +1995,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(struct 
v4l2_ctrl_handler *hdl,
return NULL;
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, unit, type,
-0, max, mask, def, 0, 0, 0,
+0, max, mask, def, 0, 0, 0, 0, 0,
 flags, qmenu, NULL, NULL);
 
 }
@@ -2014,7 +2021,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct 
v4l2_ctrl_handler *hdl,
return NULL;
}
return v4l2_ctrl_new(hdl, ops, NULL, id, name, unit, type,
-0, max, 0, def, 0, 0, 0,
+0, max, 0, def, 0, 0, 0, 0, 0,
 flags, NULL, qmenu_int, NULL);
 }
 EXPORT_SYMBOL(v4l2_ctrl_new_int_menu);
@@ -2358,7 +2365,8 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, 
struct v4l2_query_ext_ctr
qc->min.val = ctrl->minimum;
qc->max.val = ctrl->maximum;
qc->def.val = ctrl->default_value;
-   qc->cols = qc->rows = 1;
+   qc->cols = ctrl->cols;
+   qc->rows = ctrl->rows;
if (ctrl->type == V4L2_CTRL_TYPE_MENU
|| ctrl->type == V4L2_CTRL_TYPE_INTEGER_MENU)
qc->step.val = 1;
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 9ce740d..ad62b71 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -132,6 +132,8 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl 
*ctrl, void *priv);
   * @minimum:  The control's minimum value.
   * @maximum:  The control's maximum value.
   * @default_value: The control's default value.
+  * @rows

[RFCv1 PATCH 20/27] v4l2-ctrls: add matrix support.

[Hans Verkuil]

From: Hans Verkuil 

Finish the userspace-facing matrix support.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 106 ---
 1 file changed, 61 insertions(+), 45 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 3655b51..8d6711e 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1159,6 +1159,8 @@ static void std_init(const struct v4l2_ctrl *ctrl, u32 
idx,
ptr.p_s32[idx] = ctrl->default_value;
break;
default:
+   idx *= ctrl->elem_size;
+   memset(ptr.p + idx, 0, ctrl->elem_size);
break;
}
 }
@@ -1276,7 +1278,7 @@ static int ptr_to_user(struct v4l2_ext_control *c,
u32 len;
 
if (ctrl->is_ptr && !ctrl->is_string)
-   return copy_to_user(c->p, ptr.p, ctrl->elem_size);
+   return copy_to_user(c->p, ptr.p, c->size);
 
switch (ctrl->type) {
case V4L2_CTRL_TYPE_STRING:
@@ -1327,8 +1329,17 @@ static int user_to_ptr(struct v4l2_ext_control *c,
u32 size;
 
ctrl->is_new = 1;
-   if (ctrl->is_ptr && !ctrl->is_string)
-   return copy_from_user(ptr.p, c->p, ctrl->elem_size);
+   if (ctrl->is_ptr && !ctrl->is_string) {
+   unsigned idx;
+
+   ret = copy_from_user(ptr.p, c->p, c->size);
+   if (ret || !ctrl->is_matrix)
+   return ret;
+   for (idx = c->size / ctrl->elem_size;
+idx < ctrl->rows * ctrl->cols; idx++)
+   ctrl->type_ops->init(ctrl, idx, ptr);
+   return 0;
+   }
 
switch (ctrl->type) {
case V4L2_CTRL_TYPE_INTEGER64:
@@ -1371,21 +1382,7 @@ static void ptr_to_ptr(struct v4l2_ctrl *ctrl,
 {
if (ctrl == NULL)
return;
-   switch (ctrl->type) {
-   case V4L2_CTRL_TYPE_STRING:
-   /* strings are always 0-terminated */
-   strcpy(to.p_char, from.p_char);
-   break;
-   case V4L2_CTRL_TYPE_INTEGER64:
-   *to.p_s64 = *from.p_s64;
-   break;
-   default:
-   if (ctrl->is_ptr)
-   memcpy(to.p, from.p, ctrl->elem_size);
-   else
-   *to.p_s32 = *from.p_s32;
-   break;
-   }
+   memcpy(to.p, from.p, ctrl->rows * ctrl->cols * ctrl->elem_size);
 }
 
 /* Copy the new value to the current value. */
@@ -1446,15 +1443,19 @@ static void store_to_new(struct v4l2_ctrl *ctrl, 
unsigned store)
 static int cluster_changed(struct v4l2_ctrl *master)
 {
bool changed = false;
+   unsigned idx;
int i;
 
for (i = 0; i < master->ncontrols; i++) {
struct v4l2_ctrl *ctrl = master->cluster[i];
+   bool ctrl_changed = false;
 
if (ctrl == NULL)
continue;
-   ctrl->has_changed = !ctrl->type_ops->equal(ctrl, 0,
+   for (idx = 0; idx < ctrl->rows * ctrl->cols; idx++)
+   ctrl_changed |= !ctrl->type_ops->equal(ctrl, idx,
ctrl->stores[0], ctrl->new);
+   ctrl->has_changed = ctrl_changed;
changed |= ctrl->has_changed;
}
return changed;
@@ -1501,26 +1502,32 @@ static int validate_new(const struct v4l2_ctrl *ctrl,
struct v4l2_ext_control *c)
 {
union v4l2_ctrl_ptr ptr;
-
-   switch (ctrl->type) {
-   case V4L2_CTRL_TYPE_INTEGER:
-   case V4L2_CTRL_TYPE_INTEGER_MENU:
-   case V4L2_CTRL_TYPE_MENU:
-   case V4L2_CTRL_TYPE_BITMASK:
-   case V4L2_CTRL_TYPE_BOOLEAN:
-   case V4L2_CTRL_TYPE_BUTTON:
-   case V4L2_CTRL_TYPE_CTRL_CLASS:
-   ptr.p_s32 = &c->value;
-   return ctrl->type_ops->validate(ctrl, 0, ptr);
-
-   case V4L2_CTRL_TYPE_INTEGER64:
-   ptr.p_s64 = &c->value64;
-   return ctrl->type_ops->validate(ctrl, 0, ptr);
-
-   default:
-   ptr.p = c->p;
-   return ctrl->type_ops->validate(ctrl, 0, ptr);
+   unsigned idx;
+   int err = 0;
+
+   if (!ctrl->is_ptr) {
+   switch (ctrl->type) {
+   case V4L2_CTRL_TYPE_INTEGER:
+   case V4L2_CTRL_TYPE_INTEGER_MENU:
+   case V4L2_CTRL_TYPE_MENU:
+   case V4L2_CTRL_TYPE_BITMASK:
+   case V4L2_CTRL_TYPE_BOOLEAN:
+   case V4L2_CTRL_TYPE_BUTTON:
+   case V4L2_CTRL_TYPE_CTRL_CLASS:
+   ptr.p_s32 = &c->value;
+   return ctrl->type_ops->validate(ctrl, 0, ptr);
+
+   case V4L2_CTRL_TYPE_INTEGER64:
+   ptr.p_s64 = &c->value64;
+   return ctrl->type_ops->validate(ctrl, 0, ptr);
+   default:
+ 

[RFCv1 PATCH 01/27] v4l2-ctrls: increase internal min/max/step/def to 64 bit

[Hans Verkuil]

From: Hans Verkuil 

While VIDIOC_QUERYCTRL is limited to 32 bit min/max/step/def values
for controls, the upcoming VIDIOC_QUERY_EXT_CTRL isn't. So increase
the internal representation to 64 bits in preparation.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-common.c   |  6 ++-
 drivers/media/v4l2-core/v4l2-ctrls.c| 76 +
 drivers/staging/media/msi3101/sdr-msi3101.c |  2 +-
 include/media/v4l2-ctrls.h  | 38 +++
 4 files changed, 69 insertions(+), 53 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-common.c 
b/drivers/media/v4l2-core/v4l2-common.c
index 433d6d7..ccaa38f 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -111,9 +111,13 @@ int v4l2_ctrl_check(struct v4l2_ext_control *ctrl, struct 
v4l2_queryctrl *qctrl,
 EXPORT_SYMBOL(v4l2_ctrl_check);
 
 /* Fill in a struct v4l2_queryctrl */
-int v4l2_ctrl_query_fill(struct v4l2_queryctrl *qctrl, s32 min, s32 max, s32 
step, s32 def)
+int v4l2_ctrl_query_fill(struct v4l2_queryctrl *qctrl, s32 _min, s32 _max, s32 
_step, s32 _def)
 {
const char *name;
+   s64 min = _min;
+   s64 max = _max;
+   u64 step = _step;
+   s64 def = _def;
 
v4l2_ctrl_fill(qctrl->id, &name, &qctrl->type,
   &min, &max, &step, &def, &qctrl->flags);
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index fb46790..d36d7f5 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -859,7 +859,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 EXPORT_SYMBOL(v4l2_ctrl_get_name);
 
 void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
-   s32 *min, s32 *max, s32 *step, s32 *def, u32 *flags)
+   s64 *min, s64 *max, u64 *step, s64 *def, u32 *flags)
 {
*name = v4l2_ctrl_get_name(id);
*flags = 0;
@@ -1317,7 +1317,7 @@ static int cluster_changed(struct v4l2_ctrl *master)
 
 /* Control range checking */
 static int check_range(enum v4l2_ctrl_type type,
-   s32 min, s32 max, u32 step, s32 def)
+   s64 min, s64 max, u64 step, s64 def)
 {
switch (type) {
case V4L2_CTRL_TYPE_BOOLEAN:
@@ -1325,7 +1325,8 @@ static int check_range(enum v4l2_ctrl_type type,
return -ERANGE;
/* fall through */
case V4L2_CTRL_TYPE_INTEGER:
-   if (step <= 0 || min > max || def < min || def > max)
+   case V4L2_CTRL_TYPE_INTEGER64:
+   if (step == 0 || min > max || def < min || def > max)
return -ERANGE;
return 0;
case V4L2_CTRL_TYPE_BITMASK:
@@ -1350,23 +1351,30 @@ static int check_range(enum v4l2_ctrl_type type,
}
 }
 
+/* Round towards the closest legal value */
+#define ROUND_TO_RANGE(val, offset_type, ctrl) \
+({ \
+   offset_type offset; \
+   val += (ctrl)->step / 2;\
+   val = clamp_t(typeof(val), val, \
+ (ctrl)->minimum, (ctrl)->maximum);\
+   offset = (val) - (ctrl)->minimum;   \
+   offset = (ctrl)->step * (offset / (ctrl)->step);\
+   val = (ctrl)->minimum + offset; \
+   0;  \
+})
+
 /* Validate a new control */
 static int validate_new(const struct v4l2_ctrl *ctrl,
struct v4l2_ext_control *c)
 {
size_t len;
-   u32 offset;
-   s32 val;
 
switch (ctrl->type) {
case V4L2_CTRL_TYPE_INTEGER:
-   /* Round towards the closest legal value */
-   val = c->value + ctrl->step / 2;
-   val = clamp(val, ctrl->minimum, ctrl->maximum);
-   offset = val - ctrl->minimum;
-   offset = ctrl->step * (offset / ctrl->step);
-   c->value = ctrl->minimum + offset;
-   return 0;
+   return ROUND_TO_RANGE(*(s32 *)&c->value, u32, ctrl);
+   case V4L2_CTRL_TYPE_INTEGER64:
+   return ROUND_TO_RANGE(*(s64 *)&c->value64, u64, ctrl);
 
case V4L2_CTRL_TYPE_BOOLEAN:
c->value = !!c->value;
@@ -1392,9 +1400,6 @@ static int validate_new(const struct v4l2_ctrl *ctrl,
c->value = 0;
return 0;
 
-   case V4L2_CTRL_TYPE_INTEGER64:
-   return 0;
-
case V4L2_CTRL_TYPE_STRING:
len = strlen(c->string);
if (len < ctrl->minimum)
@@ -1618,7 +1623,7 @@ unlock:
 static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
const struct v4l2_ctrl_ops *ops,
u32 id, const char *name, enum v4l2_ctrl_type type,
-

[RFCv1 PATCH 00/27] Add property & configuration store support

[Hans Verkuil]

This patch series adds support for properties, matrices and configuration
stores to the control framework.

See this RFCv2 for a more detailed discussion:

http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/71822

Changes since that RFCv2 are:

- I dropped the 'property' bit in the control ID, instead a new flag is
  added: V4L2_CTRL_FLAG_PROPERTY.
- A V4L2_CTRL_FLAG_IS_PTR flag is added to simplify applications: if set, then
  applications need to use the 'p' field instead of 'val' or 'val64'. This can
  be deduced from various other fields as well, but that leads to ugly code.
  This flag is cheap to set and very helpful in applications.
- Matrix types have been dropped. If cols or rows are > 1, then you have a
  matrix, so there is no need for specific matrix types.
- As a result it is no longer possible to set just a sub-rectangle of a
  matrix. It is however possible to just set the first X elements of
  a matrix/array. It became too complex to deal with the sub-rectangle,
  both in the framework, for drivers and for applications, and there are
  not enough benefits to warrant that effort.

Other than those changes this patch series implements all the ideas described
in RFCv2.

The first 21 patches are pretty definitive and the only thing missing are
the DocBook patches and a v4l2-controls.txt patch.

Before I write those I would like to get feedback for this API enhancement.
The actual API changes are surprisingly small, and most of the work done in
the patches has more to do with data structure changes needed to simplify
handling the more complex control types than with actual new code.

Patch 22 adds a new event that can deal with the new 64-bit ranges and that
adds a config_store field. However, I am not yet convinced that this is
really needed. Feedback would be welcome.

Patches 23-27 add test code for vivi to test matrices and to test the new
selection properties. This code needs more work, particularly with regards
to naming.

A working v4l2-ctl that can handle the new stuff is available here:

http://git.linuxtv.org/hverkuil/v4l-utils.git/shortlog/refs/heads/propapi

Regards,

Hans

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFCv1 PATCH 03/27] v4l2-ctrls: use pr_info/cont instead of printk.

[Hans Verkuil]

From: Hans Verkuil 

Codingstyle fix.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index bb63d2a..10bfab6 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -2031,45 +2031,45 @@ static void log_ctrl(const struct v4l2_ctrl *ctrl,
if (ctrl->type == V4L2_CTRL_TYPE_CTRL_CLASS)
return;
 
-   printk(KERN_INFO "%s%s%s: ", prefix, colon, ctrl->name);
+   pr_info("%s%s%s: ", prefix, colon, ctrl->name);
 
switch (ctrl->type) {
case V4L2_CTRL_TYPE_INTEGER:
-   printk(KERN_CONT "%d", ctrl->cur.val);
+   pr_cont("%d", ctrl->cur.val);
break;
case V4L2_CTRL_TYPE_BOOLEAN:
-   printk(KERN_CONT "%s", ctrl->cur.val ? "true" : "false");
+   pr_cont("%s", ctrl->cur.val ? "true" : "false");
break;
case V4L2_CTRL_TYPE_MENU:
-   printk(KERN_CONT "%s", ctrl->qmenu[ctrl->cur.val]);
+   pr_cont("%s", ctrl->qmenu[ctrl->cur.val]);
break;
case V4L2_CTRL_TYPE_INTEGER_MENU:
-   printk(KERN_CONT "%lld", ctrl->qmenu_int[ctrl->cur.val]);
+   pr_cont("%lld", ctrl->qmenu_int[ctrl->cur.val]);
break;
case V4L2_CTRL_TYPE_BITMASK:
-   printk(KERN_CONT "0x%08x", ctrl->cur.val);
+   pr_cont("0x%08x", ctrl->cur.val);
break;
case V4L2_CTRL_TYPE_INTEGER64:
-   printk(KERN_CONT "%lld", ctrl->cur.val64);
+   pr_cont("%lld", ctrl->cur.val64);
break;
case V4L2_CTRL_TYPE_STRING:
-   printk(KERN_CONT "%s", ctrl->cur.string);
+   pr_cont("%s", ctrl->cur.string);
break;
default:
-   printk(KERN_CONT "unknown type %d", ctrl->type);
+   pr_cont("unknown type %d", ctrl->type);
break;
}
if (ctrl->flags & (V4L2_CTRL_FLAG_INACTIVE |
   V4L2_CTRL_FLAG_GRABBED |
   V4L2_CTRL_FLAG_VOLATILE)) {
if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE)
-   printk(KERN_CONT " inactive");
+   pr_cont(" inactive");
if (ctrl->flags & V4L2_CTRL_FLAG_GRABBED)
-   printk(KERN_CONT " grabbed");
+   pr_cont(" grabbed");
if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE)
-   printk(KERN_CONT " volatile");
+   pr_cont(" volatile");
}
-   printk(KERN_CONT "\n");
+   pr_cont("\n");
 }
 
 /* Log all controls owned by the handler */
-- 
1.8.5.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

[RFCv1 PATCH 13/27] v4l2-ctrls: add function to apply a configuration store.

[Hans Verkuil]

From: Hans Verkuil 

Drivers need to be able to select a specific store. Add a new function that can
be used to apply a given store.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 67 
 include/media/v4l2-ctrls.h   |  2 ++
 2 files changed, 69 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 3e32e21..66724b7 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1423,6 +1423,15 @@ static void cur_to_new(struct v4l2_ctrl *ctrl)
ptr_to_ptr(ctrl, ctrl->stores[0], ctrl->new);
 }
 
+static void store_to_new(struct v4l2_ctrl *ctrl, unsigned store)
+{
+   if (ctrl == NULL)
+   return;
+   ptr_to_ptr(ctrl, ctrl->stores[store ? store : ctrl->cur_store],
+   ctrl->new);
+   ctrl->is_new = true;
+}
+
 /* Return non-zero if one or more of the controls in the cluster has a new
value that differs from the current value. */
 static int cluster_changed(struct v4l2_ctrl *master)
@@ -3087,6 +3096,64 @@ int v4l2_ctrl_s_ctrl_int64(struct v4l2_ctrl *ctrl, s64 
val)
 }
 EXPORT_SYMBOL(v4l2_ctrl_s_ctrl_int64);
 
+int v4l2_ctrl_apply_store(struct v4l2_ctrl_handler *hdl, unsigned store)
+{
+   struct v4l2_ctrl_ref *ref;
+   bool found_store = false;
+   unsigned i;
+
+   if (hdl == NULL || store == 0)
+   return -EINVAL;
+
+   mutex_lock(hdl->lock);
+
+   list_for_each_entry(ref, &hdl->ctrl_refs, node) {
+   struct v4l2_ctrl *master;
+
+   if (store > ref->ctrl->nstores)
+   continue;
+   found_store = true;
+   if (ref->ctrl->cur_store) {
+   if (ref->ctrl->cur_store == store)
+   continue;
+   ref->ctrl->cur_store = store;
+   ref->ctrl->stores[0] = ref->ctrl->stores[store];
+   send_event(NULL, ref->ctrl, V4L2_EVENT_CTRL_CH_VALUE);
+   continue;
+   }
+   master = ref->ctrl->cluster[0];
+   if (ref->ctrl != master)
+   continue;
+   if (master->handler != hdl)
+   v4l2_ctrl_lock(master);
+   for (i = 0; i < master->ncontrols; i++)
+   store_to_new(master->cluster[i], store);
+
+   /* For volatile autoclusters that are currently in auto mode
+  we need to discover if it will be set to manual mode.
+  If so, then we have to copy the current volatile values
+  first since those will become the new manual values (which
+  may be overwritten by explicit new values from this set
+  of controls). */
+   if (master->is_auto && master->has_volatiles &&
+   !is_cur_manual(master)) {
+   s32 new_auto_val = *master->stores[store].p_s32;
+
+   /* If the new value == the manual value, then copy
+  the current volatile values. */
+   if (new_auto_val == master->manual_mode_value)
+   update_from_auto_cluster(master);
+   }
+
+   try_or_set_cluster(NULL, master, 0, true, 0);
+   if (master->handler != hdl)
+   v4l2_ctrl_unlock(master);
+   }
+   mutex_unlock(hdl->lock);
+   return found_store ? 0 : -EINVAL;
+}
+EXPORT_SYMBOL(v4l2_ctrl_apply_store);
+
 void v4l2_ctrl_notify(struct v4l2_ctrl *ctrl, v4l2_ctrl_notify_fnc notify, 
void *priv)
 {
if (ctrl == NULL)
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 911b22a..2b9b2da 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -704,6 +704,8 @@ s64 v4l2_ctrl_g_ctrl_int64(struct v4l2_ctrl *ctrl);
   */
 int v4l2_ctrl_s_ctrl_int64(struct v4l2_ctrl *ctrl, s64 val);
 
+int v4l2_ctrl_apply_store(struct v4l2_ctrl_handler *hdl, unsigned store);
+
 /* Internal helper functions that deal with control events. */
 extern const struct v4l2_subscribed_event_ops v4l2_ctrl_sub_ev_ops;
 void v4l2_ctrl_replace(struct v4l2_event *old, const struct v4l2_event *new);
-- 
1.8.5.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

[RFCv1 PATCH 25/27] v4l2-ctrls: add support for u8, u16 and prop_selection types.

[Hans Verkuil]

From: Hans Verkuil 

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 30 ++
 include/media/v4l2-ctrls.h   |  3 +++
 2 files changed, 33 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 2191451..6c7640b 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1165,6 +1165,10 @@ static bool std_equal(const struct v4l2_ctrl *ctrl, u32 
idx,
return !strcmp(ptr1.p_char + idx, ptr2.p_char + idx);
case V4L2_CTRL_TYPE_INTEGER64:
return ptr1.p_s64[idx] == ptr2.p_s64[idx];
+   case V4L2_PROP_TYPE_U8:
+   return ptr1.p_u8[idx] == ptr2.p_u8[idx];
+   case V4L2_PROP_TYPE_U16:
+   return ptr1.p_u16[idx] == ptr2.p_u16[idx];
default:
if (ctrl->is_int)
return ptr1.p_s32[idx] == ptr2.p_s32[idx];
@@ -1192,6 +1196,12 @@ static void std_init(const struct v4l2_ctrl *ctrl, u32 
idx,
case V4L2_CTRL_TYPE_BOOLEAN:
ptr.p_s32[idx] = ctrl->default_value;
break;
+   case V4L2_PROP_TYPE_U8:
+   ptr.p_u8[idx] = ctrl->default_value;
+   break;
+   case V4L2_PROP_TYPE_U16:
+   ptr.p_u16[idx] = ctrl->default_value;
+   break;
default:
idx *= ctrl->elem_size;
memset(ptr.p + idx, 0, ctrl->elem_size);
@@ -1228,6 +1238,18 @@ static void std_log(const struct v4l2_ctrl *ctrl)
case V4L2_CTRL_TYPE_STRING:
pr_cont("%s", ptr.p_char);
break;
+   case V4L2_PROP_TYPE_U8:
+   pr_cont("%u", (unsigned)*ptr.p_u8);
+   break;
+   case V4L2_PROP_TYPE_U16:
+   pr_cont("%u", (unsigned)*ptr.p_u16);
+   break;
+   case V4L2_PROP_TYPE_SELECTION:
+   pr_cont("%ux%u@%dx%d (0x%x)",
+   ptr.p_sel->r.width, ptr.p_sel->r.height,
+   ptr.p_sel->r.left, ptr.p_sel->r.top,
+   ptr.p_sel->flags);
+   break;
default:
pr_cont("unknown type %d", ctrl->type);
break;
@@ -1258,6 +1280,10 @@ static int std_validate(const struct v4l2_ctrl *ctrl, 
u32 idx,
return ROUND_TO_RANGE(ptr.p_s32[idx], u32, ctrl);
case V4L2_CTRL_TYPE_INTEGER64:
return ROUND_TO_RANGE(ptr.p_s64[idx], u64, ctrl);
+   case V4L2_PROP_TYPE_U8:
+   return ROUND_TO_RANGE(ptr.p_u8[idx], u8, ctrl);
+   case V4L2_PROP_TYPE_U16:
+   return ROUND_TO_RANGE(ptr.p_u16[idx], u16, ctrl);
 
case V4L2_CTRL_TYPE_BOOLEAN:
ptr.p_s32[idx] = !!ptr.p_s32[idx];
@@ -1504,6 +1530,8 @@ static int check_range(enum v4l2_ctrl_type type,
if (step != 1 || max > 1 || min < 0)
return -ERANGE;
/* fall through */
+   case V4L2_PROP_TYPE_U8:
+   case V4L2_PROP_TYPE_U16:
case V4L2_CTRL_TYPE_INTEGER:
case V4L2_CTRL_TYPE_INTEGER64:
if (step == 0 || min > max || def < min || def > max)
@@ -3259,6 +3287,8 @@ int v4l2_ctrl_modify_range(struct v4l2_ctrl *ctrl,
case V4L2_CTRL_TYPE_MENU:
case V4L2_CTRL_TYPE_INTEGER_MENU:
case V4L2_CTRL_TYPE_BITMASK:
+   case V4L2_PROP_TYPE_U8:
+   case V4L2_PROP_TYPE_U16:
if (ctrl->is_matrix)
return -EINVAL;
ret = check_range(ctrl->type, min, max, step, def);
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 514c427..09257e4 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -45,7 +45,10 @@ struct poll_table_struct;
 union v4l2_ctrl_ptr {
s32 *p_s32;
s64 *p_s64;
+   u8 *p_u8;
+   u16 *p_u16;
char *p_char;
+   struct v4l2_prop_selection *p_sel;
void *p;
 };
 
-- 
1.8.5.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

[RFCv1 PATCH 27/27] vivi: add matrix and selection test code.

[Hans Verkuil]

From: Hans Verkuil 

Signed-off-by: Hans Verkuil 
---
 drivers/media/platform/vivi.c | 312 ++
 1 file changed, 286 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/vivi.c b/drivers/media/platform/vivi.c
index 2ec8511..0532d2b 100644
--- a/drivers/media/platform/vivi.c
+++ b/drivers/media/platform/vivi.c
@@ -48,8 +48,9 @@
  */
 #define FPS_MAX 1000
 
-#define MAX_WIDTH 1920
-#define MAX_HEIGHT 1200
+#define MAX_ZOOM 4
+#define MAX_WIDTH (1920 * MAX_ZOOM)
+#define MAX_HEIGHT (1200 * MAX_ZOOM)
 
 #define VIVI_VERSION "0.8.1"
 
@@ -86,6 +87,9 @@ static const struct v4l2_fract
 #define dprintk(dev, level, fmt, arg...) \
v4l2_dbg(level, debug, &dev->v4l2_dev, fmt, ## arg)
 
+#define VIVI_CID_CUSTOM_BASE   (V4L2_CID_USER_BASE | 0xf000)
+#define VIVI_PID_CUSTOM_BASE   (V4L2_CID_PROPS_CLASS_BASE | 0xf000)
+
 /* --
Basic structures
--*/
@@ -207,6 +211,8 @@ struct vivi_dmaqueue {
 
 static LIST_HEAD(vivi_devlist);
 
+#define VIVI_UNSET (-1000)
+
 struct vivi_dev {
struct list_head   vivi_devlist;
struct v4l2_device v4l2_dev;
@@ -233,6 +239,16 @@ struct vivi_dev {
struct v4l2_ctrl   *string;
struct v4l2_ctrl   *bitmask;
struct v4l2_ctrl   *int_menu;
+   struct v4l2_ctrl   *store;
+   struct v4l2_ctrl   *prop1;
+   struct v4l2_ctrl   *prop2;
+   struct v4l2_ctrl   *prop3;
+   struct { /* crop/compose selection cluster */
+   struct v4l2_ctrl   *crop;
+   struct v4l2_ctrl   *compose;
+   };
+   struct v4l2_prop_selection *crop_sel;
+   struct v4l2_prop_selection *compose_sel;
 
spinlock_t slock;
struct mutex   mutex;
@@ -253,6 +269,7 @@ struct vivi_dev {
const struct vivi_fmt  *fmt;
struct v4l2_fract  timeperframe;
unsigned int   width, height;
+   unsigned int   canvas_width, canvas_height;
struct vb2_queue   vb_vidq;
unsigned int   field_count;
 
@@ -314,6 +331,7 @@ static const struct bar_std bars[] = {
 };
 
 #define NUM_INPUTS ARRAY_SIZE(bars)
+#define NUM_STORES NUM_INPUTS
 
 #define TO_Y(r, g, b) \
(((16829 * r + 33039 * g + 6416 * b  + 32768) >> 16) + 16)
@@ -535,19 +553,26 @@ static void precalculate_line(struct vivi_dev *dev)
unsigned pixsize  = dev->pixelsize;
unsigned pixsize2 = 2*pixsize;
int colorpos;
-   u8 *pos;
+   unsigned width = dev->canvas_width * dev->compose_sel->r.width / 
dev->crop_sel->r.width;
+   u8 *pos = dev->line;
 
for (colorpos = 0; colorpos < 16; ++colorpos) {
u8 pix[8];
-   int wstart =  colorpos* dev->width / 8;
-   int wend   = (colorpos+1) * dev->width / 8;
+   int wstart = (colorpos) * width / 8;
+   int wend   = ((colorpos+1)) * width / 8;
int w;
 
+   wstart = wstart & ~1;
+   wend = wend & ~1;
+   if (colorpos == 15)
+   wend = MAX_WIDTH * MAX_ZOOM;
gen_twopix(dev, &pix[0],colorpos % 8, 0);
gen_twopix(dev, &pix[pixsize],  colorpos % 8, 1);
 
-   for (w = wstart/2*2, pos = dev->line + w*pixsize; w < wend; w 
+= 2, pos += pixsize2)
+   for (w = wstart; w < wend; w += 2) {
memcpy(pos, pix, pixsize2);
+   pos += pixsize2;
+   }
}
 }
 
@@ -605,21 +630,26 @@ static void gen_text(struct vivi_dev *dev, char *basep,
 static void vivi_fillbuff(struct vivi_dev *dev, struct vivi_buffer *buf)
 {
int stride = dev->width * dev->pixelsize;
-   int hmax = dev->height;
+   int hmax = dev->compose_sel->r.height;
void *vbuf = vb2_plane_vaddr(&buf->vb, 0);
+   u8 *matrix = dev->prop3->cur.p_u8;
unsigned ms;
char str[100];
int h, line = 1;
+   unsigned start;
u8 *linestart;
s32 gain;
 
if (!vbuf)
return;
 
-   linestart = dev->line + (dev->mv_count % dev->width) * dev->pixelsize;
+   start = (dev->mv_count + dev->crop_sel->r.left) % dev->canvas_width;
+   start = (start * dev->compose_sel->r.width) / dev->crop_sel->r.width;
+   start &= ~1;
+   linestart = dev->line + start * dev->pixelsize;
 
for (h = 0; h < hmax; h++)
-   memcpy(vbuf + h * stride, linestart, stride);
+   memcpy(vbuf + h * stride, linestart, dev->compose_sel->r.width 
* dev->pixelsize);
 
/* Updates stream time */
 
@@ -636,7 +666,7 @@ static void vivi_fillbuff(struct vivi_dev *dev, struct 
vivi_buffer *buf)
ms % 1000

[RFCv1 PATCH 21/27] videodev2.h: rename reserved2 to config_store in v4l2_buffer.

[Hans Verkuil]

From: Hans Verkuil 

When queuing buffers allow for passing the configuration store ID that
should be associated with this buffer. Use the 'reserved2' field for this.

Signed-off-by: Hans Verkuil 
---
 drivers/media/usb/cpia2/cpia2_v4l.c   | 2 +-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 4 ++--
 drivers/media/v4l2-core/videobuf2-core.c  | 2 +-
 include/uapi/linux/videodev2.h| 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c 
b/drivers/media/usb/cpia2/cpia2_v4l.c
index d5d42b6..51b7759 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -952,7 +952,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, struct 
v4l2_buffer *buf)
buf->sequence = cam->buffers[buf->index].seq;
buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer;
buf->length = cam->frame_size;
-   buf->reserved2 = 0;
+   buf->config_store = 0;
buf->reserved = 0;
memset(&buf->timecode, 0, sizeof(buf->timecode));
 
diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 0d9b97e..a381c92 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -322,7 +322,7 @@ struct v4l2_buffer32 {
__s32   fd;
} m;
__u32   length;
-   __u32   reserved2;
+   __u32   config_store;
__u32   reserved;
 };
 
@@ -487,7 +487,7 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct 
v4l2_buffer32 __user
put_user(kp->timestamp.tv_usec, &up->timestamp.tv_usec) ||
copy_to_user(&up->timecode, &kp->timecode, sizeof(struct 
v4l2_timecode)) ||
put_user(kp->sequence, &up->sequence) ||
-   put_user(kp->reserved2, &up->reserved2) ||
+   put_user(kp->config_store, &up->config_store) ||
put_user(kp->reserved, &up->reserved))
return -EFAULT;
 
diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 12df9fd..5f35e1d 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -435,7 +435,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, 
struct v4l2_buffer *b)
 
/* Copy back data such as timestamp, flags, etc. */
memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m));
-   b->reserved2 = vb->v4l2_buf.reserved2;
+   b->config_store = vb->v4l2_buf.config_store;
b->reserved = vb->v4l2_buf.reserved;
 
if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) {
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 789f876..78aba44 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -640,7 +640,7 @@ struct v4l2_plane {
  * @length:size in bytes of the buffer (NOT its payload) for single-plane
  * buffers (when type != *_MPLANE); number of elements in the
  * planes array for multi-plane buffers
- * @input: input number from which the video data has has been captured
+ * @config_store: this buffer should use this configuration store
  *
  * Contains data exchanged by application and driver using one of the Streaming
  * I/O methods.
@@ -664,7 +664,7 @@ struct v4l2_buffer {
__s32   fd;
} m;
__u32   length;
-   __u32   reserved2;
+   __u32   config_store;
__u32   reserved;
 };
 
-- 
1.8.5.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

[RFCv1 PATCH 23/27] videodev2.h: add new property types.

[Hans Verkuil]

From: Hans Verkuil 

Add support for a selection property and for u8 and u16 matrices.

Signed-off-by: Hans Verkuil 
---
 include/uapi/linux/videodev2.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index afa335d..1ceaed1 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1220,6 +1220,13 @@ struct v4l2_control {
__s32value;
 };
 
+/* Property types */
+struct v4l2_prop_selection {
+   __u32 flags;
+   struct v4l2_rect r;
+   __u32 reserved[9];
+};
+
 struct v4l2_ext_control {
__u32 id;
__u32 size;
@@ -1228,6 +1235,9 @@ struct v4l2_ext_control {
__s32 value;
__s64 value64;
char *string;
+   __u8 *p_u8;
+   __u16 *p_u16;
+   struct v4l2_prop_selection *p_sel;
void *p;
};
 } __attribute__ ((packed));
@@ -1260,6 +1270,9 @@ enum v4l2_ctrl_type {
 
/* Property types are >= 0x0100 */
V4L2_PROP_TYPES  = 0x0100,
+   V4L2_PROP_TYPE_U8= 0x0100,
+   V4L2_PROP_TYPE_U16   = 0x0101,
+   V4L2_PROP_TYPE_SELECTION = 0x0102,
 };
 
 /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
-- 
1.8.5.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

[RFCv1 PATCH 18/27] v4l2-ctrls: prepare for matrix support.

[Hans Verkuil]

From: Hans Verkuil 

Add core support for matrices.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 57 +++-
 include/media/v4l2-ctrls.h   |  8 +++--
 2 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index bb7f4ccb..b6edd81 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1118,7 +1118,7 @@ static void send_event(struct v4l2_fh *fh, struct 
v4l2_ctrl *ctrl, u32 changes)
v4l2_event_queue_fh(sev->fh, &ev);
 }
 
-static bool std_equal(const struct v4l2_ctrl *ctrl,
+static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
  union v4l2_ctrl_ptr ptr1,
  union v4l2_ctrl_ptr ptr2)
 {
@@ -1137,7 +1137,7 @@ static bool std_equal(const struct v4l2_ctrl *ctrl,
}
 }
 
-static void std_init(const struct v4l2_ctrl *ctrl,
+static void std_init(const struct v4l2_ctrl *ctrl, u32 idx,
 union v4l2_ctrl_ptr ptr)
 {
switch (ctrl->type) {
@@ -1164,6 +1164,9 @@ static void std_log(const struct v4l2_ctrl *ctrl)
 {
union v4l2_ctrl_ptr ptr = ctrl->stores[ctrl->cur_store];
 
+   if (ctrl->is_matrix)
+   pr_cont("[%u][%u] ", ctrl->rows, ctrl->cols);
+
switch (ctrl->type) {
case V4L2_CTRL_TYPE_INTEGER:
pr_cont("%d", *ptr.p_s32);
@@ -1206,7 +1209,7 @@ static void std_log(const struct v4l2_ctrl *ctrl)
 })
 
 /* Validate a new control */
-static int std_validate(const struct v4l2_ctrl *ctrl,
+static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr)
 {
size_t len;
@@ -1446,7 +1449,7 @@ static int cluster_changed(struct v4l2_ctrl *master)
 
if (ctrl == NULL)
continue;
-   ctrl->has_changed = !ctrl->type_ops->equal(ctrl,
+   ctrl->has_changed = !ctrl->type_ops->equal(ctrl, 0,
ctrl->stores[0], ctrl->new);
changed |= ctrl->has_changed;
}
@@ -1504,15 +1507,15 @@ static int validate_new(const struct v4l2_ctrl *ctrl,
case V4L2_CTRL_TYPE_BUTTON:
case V4L2_CTRL_TYPE_CTRL_CLASS:
ptr.p_s32 = &c->value;
-   return ctrl->type_ops->validate(ctrl, ptr);
+   return ctrl->type_ops->validate(ctrl, 0, ptr);
 
case V4L2_CTRL_TYPE_INTEGER64:
ptr.p_s64 = &c->value64;
-   return ctrl->type_ops->validate(ctrl, ptr);
+   return ctrl->type_ops->validate(ctrl, 0, ptr);
 
default:
ptr.p = c->p;
-   return ctrl->type_ops->validate(ctrl, ptr);
+   return ctrl->type_ops->validate(ctrl, 0, ptr);
}
 }
 
@@ -1735,7 +1738,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
const s64 *qmenu_int, void *priv)
 {
struct v4l2_ctrl *ctrl;
-   unsigned sz_extra;
+   bool is_matrix;
+   unsigned sz_extra, tot_prop_size;
void *props;
int err;
int s;
@@ -1747,6 +1751,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
cols = 1;
if (rows == 0)
rows = 1;
+   is_matrix = cols > 1 || rows > 1;
 
if (type == V4L2_CTRL_TYPE_INTEGER64)
elem_size = sizeof(s64);
@@ -1754,10 +1759,11 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
elem_size = max + 1;
else if (type < V4L2_PROP_TYPES)
elem_size = sizeof(s32);
+   tot_prop_size = elem_size * cols * rows;
 
/* Sanity checks */
-   if (id == 0 || name == NULL || id >= V4L2_CID_PRIVATE_BASE ||
-   elem_size == 0 ||
+   if (id == 0 || name == NULL || !elem_size ||
+   id >= V4L2_CID_PRIVATE_BASE ||
(type == V4L2_CTRL_TYPE_MENU && qmenu == NULL) ||
(type == V4L2_CTRL_TYPE_INTEGER_MENU && qmenu_int == NULL)) {
handler_set_err(hdl, -ERANGE);
@@ -1772,7 +1778,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
handler_set_err(hdl, -EINVAL);
return NULL;
}
-   if (!(flags & V4L2_CTRL_FLAG_PROPERTY) && type >= V4L2_PROP_TYPES) {
+   if (!(flags & V4L2_CTRL_FLAG_PROPERTY) &&
+   (is_matrix || type >= V4L2_PROP_TYPES)) {
handler_set_err(hdl, -EINVAL);
return NULL;
}
@@ -1791,6 +1798,12 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
handler_set_err(hdl, -EINVAL);
return NULL;
}
+   if (is_matrix &&
+   (type == V4L2_CTRL_TYPE_BUTTON ||
+type == V4L2_CTRL_TYPE_CTRL_CLASS)) {
+   handler_set_err(hdl, -EINVAL);
+

[RFCv1 PATCH 26/27] v4l2-common.h: add new target

[Hans Verkuil]

From: Hans Verkuil 

This target can be used to set the frame size for devices that do not
support S_STD or S_DV_TIMINGS.

Signed-off-by: Hans Verkuil 
---
 include/uapi/linux/v4l2-common.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/v4l2-common.h b/include/uapi/linux/v4l2-common.h
index 4f0667e..88c9f89 100644
--- a/include/uapi/linux/v4l2-common.h
+++ b/include/uapi/linux/v4l2-common.h
@@ -50,6 +50,8 @@
 /* Current composing area plus all padding pixels */
 #define V4L2_SEL_TGT_COMPOSE_PADDED0x0103
 
+#define V4L2_SEL_TGT_FRAME_SIZE0x0200
+
 /* Backward compatibility target definitions --- to be removed. */
 #define V4L2_SEL_TGT_CROP_ACTIVE   V4L2_SEL_TGT_CROP
 #define V4L2_SEL_TGT_COMPOSE_ACTIVEV4L2_SEL_TGT_COMPOSE
-- 
1.8.5.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

[RFCv1 PATCH 24/27] v4l2-controls.h: add new property class and new properties.

[Hans Verkuil]

From: Hans Verkuil 

Add a class for properties (for demonstration purposes only, will need to be
renamed) and capture/output crop/compose and motion detection matrix
properties.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c |  3 +++
 include/uapi/linux/v4l2-controls.h   | 16 
 2 files changed, 19 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 0014324..2191451 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -852,6 +852,8 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_FM_RX_CLASS:  return "FM Radio Receiver 
Controls";
case V4L2_CID_TUNE_DEEMPHASIS:  return "De-Emphasis";
case V4L2_CID_RDS_RECEPTION:return "RDS Reception";
+
+   case V4L2_CID_PROPS_CLASS:  return "Properties";
default:
return NULL;
}
@@ -987,6 +989,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, const char 
**unit,
case V4L2_CID_IMAGE_PROC_CLASS:
case V4L2_CID_DV_CLASS:
case V4L2_CID_FM_RX_CLASS:
+   case V4L2_CID_PROPS_CLASS:
*type = V4L2_CTRL_TYPE_CTRL_CLASS;
/* You can neither read not write these */
*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
diff --git a/include/uapi/linux/v4l2-controls.h 
b/include/uapi/linux/v4l2-controls.h
index 1666aab..6c3616e 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -60,6 +60,7 @@
 #define V4L2_CTRL_CLASS_IMAGE_PROC 0x009f  /* Image processing 
controls */
 #define V4L2_CTRL_CLASS_DV 0x00a0  /* Digital Video 
controls */
 #define V4L2_CTRL_CLASS_FM_RX  0x00a1  /* FM Receiver controls 
*/
+#define V4L2_CTRL_CLASS_PROPS  0x00a2  /* Properties */
 
 /* User-class control IDs */
 
@@ -886,4 +887,19 @@ enum v4l2_deemphasis {
 
 #define V4L2_CID_RDS_RECEPTION (V4L2_CID_FM_RX_CLASS_BASE + 2)
 
+
+/* Properties */
+
+#define V4L2_CID_PROPS_CLASS_BASE  (V4L2_CTRL_CLASS_PROPS | 0x900)
+#define V4L2_CID_PROPS_CLASS   (V4L2_CTRL_CLASS_PROPS | 1)
+
+#define V4L2_CID_CAPTURE_CROP  (V4L2_CID_PROPS_CLASS_BASE + 0)
+#define V4L2_CID_CAPTURE_COMPOSE   (V4L2_CID_PROPS_CLASS_BASE + 1)
+#define V4L2_CID_OUTPUT_CROP   (V4L2_CID_PROPS_CLASS_BASE + 2)
+#define V4L2_CID_OUTPUT_COMPOSE
(V4L2_CID_PROPS_CLASS_BASE + 3)
+
+/* TODO: use a Motion Detection property class */
+#define V4L2_CID_MD_REGION (V4L2_CID_PROPS_CLASS_BASE + 4)
+#define V4L2_CID_MD_THRESHOLD  (V4L2_CID_PROPS_CLASS_BASE + 5)
+
 #endif
-- 
1.8.5.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

[RFCv1 PATCH 16/27] v4l2-ctrls: replace cur by a union v4l2_ctrl_ptr.

[Hans Verkuil]

From: Hans Verkuil 

Instead of having to maintain the 'cur' union this patch replaces it by
a v4l2_ctrl_ptr union to be consistent with configuration stores, which
also use that union. The number of drivers that use 'cur' is fairly small,
so it is easy enough to convert them all.

Unfortunately, the union for the new value cannot be dropped as easily
since it is used pretty much everywhere.

Signed-off-by: Hans Verkuil 
---
 Documentation/video4linux/v4l2-controls.txt   |  4 ++--
 drivers/media/common/cx2341x.c|  4 ++--
 drivers/media/i2c/adp1653.c   | 10 +-
 drivers/media/i2c/as3645a.c   | 22 ++---
 drivers/media/i2c/lm3560.c|  2 +-
 drivers/media/i2c/m5mols/m5mols_controls.c|  6 +++---
 drivers/media/i2c/msp3400-driver.c|  4 ++--
 drivers/media/i2c/mt9p031.c   |  4 ++--
 drivers/media/i2c/mt9t001.c   |  4 ++--
 drivers/media/i2c/s5c73m3/s5c73m3-ctrls.c |  6 +++---
 drivers/media/i2c/smiapp/smiapp-core.c| 12 ++--
 drivers/media/pci/cx18/cx18-av-core.c |  2 +-
 drivers/media/pci/cx18/cx18-driver.c  | 10 +-
 drivers/media/platform/exynos4-is/fimc-core.c |  6 +++---
 drivers/media/platform/vivi.c | 28 +--
 drivers/media/radio/radio-isa.c   |  2 +-
 drivers/media/radio/radio-sf16fmr2.c  |  4 ++--
 drivers/media/usb/gspca/conex.c   |  8 
 drivers/media/usb/gspca/sn9c20x.c |  4 ++--
 drivers/media/usb/gspca/topro.c   |  4 ++--
 drivers/media/v4l2-core/v4l2-ctrls.c  | 19 +-
 include/media/v4l2-ctrls.h|  9 ++---
 22 files changed, 84 insertions(+), 90 deletions(-)

diff --git a/Documentation/video4linux/v4l2-controls.txt 
b/Documentation/video4linux/v4l2-controls.txt
index 06cf3ac..1c353c2 100644
--- a/Documentation/video4linux/v4l2-controls.txt
+++ b/Documentation/video4linux/v4l2-controls.txt
@@ -362,8 +362,8 @@ will result in a deadlock since these helpers lock the 
handler as well.
 You can also take the handler lock yourself:
 
mutex_lock(&state->ctrl_handler.lock);
-   printk(KERN_INFO "String value is '%s'\n", ctrl1->cur.string);
-   printk(KERN_INFO "Integer value is '%s'\n", ctrl2->cur.val);
+   pr_info("String value is '%s'\n", ctrl1->cur.p_char);
+   pr_info("Integer value is '%d'\n", *ctrl2->cur.p_s32);
mutex_unlock(&state->ctrl_handler.lock);
 
 
diff --git a/drivers/media/common/cx2341x.c b/drivers/media/common/cx2341x.c
index 103ef6b..909d334 100644
--- a/drivers/media/common/cx2341x.c
+++ b/drivers/media/common/cx2341x.c
@@ -1261,10 +1261,10 @@ static int cx2341x_hdl_api(struct cx2341x_handler *hdl,
return hdl->func(hdl->priv, cmd, args, 0, data);
 }
 
-/* ctrl->handler->lock is held, so it is safe to access cur.val */
+/* ctrl->handler->lock is held, so it is safe to access *cur.p_s32 */
 static inline int cx2341x_neq(struct v4l2_ctrl *ctrl)
 {
-   return ctrl && ctrl->val != ctrl->cur.val;
+   return ctrl && ctrl->val != *ctrl->cur.p_s32;
 }
 
 static int cx2341x_try_ctrl(struct v4l2_ctrl *ctrl)
diff --git a/drivers/media/i2c/adp1653.c b/drivers/media/i2c/adp1653.c
index 873fe19..7d478dc 100644
--- a/drivers/media/i2c/adp1653.c
+++ b/drivers/media/i2c/adp1653.c
@@ -158,16 +158,16 @@ static int adp1653_get_ctrl(struct v4l2_ctrl *ctrl)
if (IS_ERR_VALUE(rval))
return rval;
 
-   ctrl->cur.val = 0;
+   *ctrl->cur.p_s32 = 0;
 
if (flash->fault & ADP1653_REG_FAULT_FLT_SCP)
-   ctrl->cur.val |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
+   *ctrl->cur.p_s32 |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
if (flash->fault & ADP1653_REG_FAULT_FLT_OT)
-   ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_TEMPERATURE;
+   *ctrl->cur.p_s32 |= V4L2_FLASH_FAULT_OVER_TEMPERATURE;
if (flash->fault & ADP1653_REG_FAULT_FLT_TMR)
-   ctrl->cur.val |= V4L2_FLASH_FAULT_TIMEOUT;
+   *ctrl->cur.p_s32 |= V4L2_FLASH_FAULT_TIMEOUT;
if (flash->fault & ADP1653_REG_FAULT_FLT_OV)
-   ctrl->cur.val |= V4L2_FLASH_FAULT_OVER_VOLTAGE;
+   *ctrl->cur.p_s32 |= V4L2_FLASH_FAULT_OVER_VOLTAGE;
 
flash->fault = 0;
 
diff --git a/drivers/media/i2c/as3645a.c b/drivers/media/i2c/as3645a.c
index 301084b..4c6041c 100644
--- a/drivers/media/i2c/as3645a.c
+++ b/drivers/media/i2c/as3645a.c
@@ -334,24 +334,24 @@ static int as3645a_get_ctrl(struct v4l2_ctrl *ctrl)
if (value < 0)
return value;
 
-   ctrl->cur.val = 0;
+   *ctrl->cur.p_s32 = 0;
if (value & AS_FAULT_INFO_SHORT_CIRCUIT)
-   ctrl->cur.val |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
+   *ctrl->cur.p_s32 |= V4L2_FLASH_FAULT_SHORT_CIRCUIT;
if (value & AS

[RFCv1 PATCH 08/27] videodev2.h: add V4L2_CTRL_FLAG_CAN_STORE

[Hans Verkuil]

From: Hans Verkuil 

Controls/properties that have a configuration store will set this flag.

Signed-off-by: Hans Verkuil 
---
 include/uapi/linux/videodev2.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 2dda52d..0803da9 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1323,6 +1323,7 @@ struct v4l2_querymenu {
 #define V4L2_CTRL_FLAG_VOLATILE0x0080
 #define V4L2_CTRL_FLAG_PROPERTY0x0100
 #define V4L2_CTRL_FLAG_IS_PTR  0x0200
+#define V4L2_CTRL_FLAG_CAN_STORE   0x0400
 
 /*  Query flags, to be ORed with the control ID */
 #define V4L2_CTRL_FLAG_NEXT_CTRL   0x8000
-- 
1.8.5.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

[RFCv1 PATCH 04/27] videodev2.h: add initial support for properties.

[Hans Verkuil]

From: Hans Verkuil 

Properties are controls that are not shown in GUIs and can be used for
compound and array types.

This allows for more complex datastructures to be used with the
control framework.

Properties will have the V4L2_CTRL_FLAG_PROPERTY flag set. The existing
V4L2_CTRL_FLAG_NEXT_CTRL flag will only enumerate controls, so a new
V4L2_CTRL_FLAG_NEXT_PROP flag is added to enumerate properties. Set both
flags to enumerate both controls and properties.

Property-specific types will start at V4L2_PROP_TYPES. In addition, any
control or property that uses the new 'p' field (or the existing 'string'
field) will have flag V4L2_CTRL_FLAG_IS_PTR set.

While not strictly necessary, adding that flag makes life for applications
a lot simpler. If the flag is not set, then the control value is set
through the value or value64 fields of struct v4l2_ext_control, otherwise
a pointer points to the value.

Signed-off-by: Hans Verkuil 
---
 include/uapi/linux/videodev2.h | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 437f1b0..c8e2259 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1228,6 +1228,7 @@ struct v4l2_ext_control {
__s32 value;
__s64 value64;
char *string;
+   void *p;
};
 } __attribute__ ((packed));
 
@@ -1252,7 +1253,10 @@ enum v4l2_ctrl_type {
V4L2_CTRL_TYPE_CTRL_CLASS= 6,
V4L2_CTRL_TYPE_STRING= 7,
V4L2_CTRL_TYPE_BITMASK   = 8,
-   V4L2_CTRL_TYPE_INTEGER_MENU = 9,
+   V4L2_CTRL_TYPE_INTEGER_MENU  = 9,
+
+   /* Property types are >= 0x0100 */
+   V4L2_PROP_TYPES  = 0x0100,
 };
 
 /*  Used in the VIDIOC_QUERYCTRL ioctl for querying controls */
@@ -1288,9 +1292,12 @@ struct v4l2_querymenu {
 #define V4L2_CTRL_FLAG_SLIDER  0x0020
 #define V4L2_CTRL_FLAG_WRITE_ONLY  0x0040
 #define V4L2_CTRL_FLAG_VOLATILE0x0080
+#define V4L2_CTRL_FLAG_PROPERTY0x0100
+#define V4L2_CTRL_FLAG_IS_PTR  0x0200
 
-/*  Query flag, to be ORed with the control ID */
+/*  Query flags, to be ORed with the control ID */
 #define V4L2_CTRL_FLAG_NEXT_CTRL   0x8000
+#define V4L2_CTRL_FLAG_NEXT_PROP   0x4000
 
 /*  User-class control IDs defined by V4L2 */
 #define V4L2_CID_MAX_CTRLS 1024
-- 
1.8.5.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

[RFCv1 PATCH 11/27] v4l2-ctrls: rewrite copy routines to operate on union v4l2_ctrl_ptr.

[Hans Verkuil]

From: Hans Verkuil 

In order to implement stores we need to have more generic copy
routines. The v4l2_ctrl_ptr union was designed for this.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 129 +++
 1 file changed, 56 insertions(+), 73 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 688ca31..9b0362e 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1261,48 +1261,64 @@ static const struct v4l2_ctrl_type_ops std_type_ops = {
.validate = std_validate,
 };
 
-/* Helper function: copy the current control value back to the caller */
-static int cur_to_user(struct v4l2_ext_control *c,
-  struct v4l2_ctrl *ctrl)
+/* Helper function: copy the given control value back to the caller */
+static int ptr_to_user(struct v4l2_ext_control *c,
+  struct v4l2_ctrl *ctrl,
+  union v4l2_ctrl_ptr ptr)
 {
u32 len;
 
if (ctrl->is_ptr && !ctrl->is_string)
-   return copy_to_user(c->p, ctrl->cur.p, ctrl->elem_size);
+   return copy_to_user(c->p, ptr.p, ctrl->elem_size);
 
switch (ctrl->type) {
case V4L2_CTRL_TYPE_STRING:
-   len = strlen(ctrl->cur.string);
+   len = strlen(ptr.p_char);
if (c->size < len + 1) {
c->size = len + 1;
return -ENOSPC;
}
-   return copy_to_user(c->string, ctrl->cur.string,
-   len + 1) ? -EFAULT : 0;
+   return copy_to_user(c->string, ptr.p_char, len + 1) ?
+   -EFAULT : 0;
case V4L2_CTRL_TYPE_INTEGER64:
-   c->value64 = ctrl->cur.val64;
+   c->value64 = *ptr.p_s64;
break;
default:
-   c->value = ctrl->cur.val;
+   c->value = *ptr.p_s32;
break;
}
return 0;
 }
 
-/* Helper function: copy the caller-provider value as the new control value */
-static int user_to_new(struct v4l2_ext_control *c,
+/* Helper function: copy the current control value back to the caller */
+static int cur_to_user(struct v4l2_ext_control *c,
   struct v4l2_ctrl *ctrl)
 {
+   return ptr_to_user(c, ctrl, ctrl->stores[0]);
+}
+
+/* Helper function: copy the new control value back to the caller */
+static int new_to_user(struct v4l2_ext_control *c,
+  struct v4l2_ctrl *ctrl)
+{
+   return ptr_to_user(c, ctrl, ctrl->new);
+}
+
+/* Helper function: copy the caller-provider value to the given control value 
*/
+static int user_to_ptr(struct v4l2_ext_control *c,
+  struct v4l2_ctrl *ctrl,
+  union v4l2_ctrl_ptr ptr)
+{
int ret;
u32 size;
 
ctrl->is_new = 1;
if (ctrl->is_ptr && !ctrl->is_string)
-   return copy_from_user(ctrl->p, c->p, ctrl->elem_size);
+   return copy_from_user(ptr.p, c->p, ctrl->elem_size);
 
switch (ctrl->type) {
case V4L2_CTRL_TYPE_INTEGER64:
-   ctrl->val64 = c->value64;
+   *ptr.p_s64 = c->value64;
break;
case V4L2_CTRL_TYPE_STRING:
size = c->size;
@@ -1310,83 +1326,64 @@ static int user_to_new(struct v4l2_ext_control *c,
return -ERANGE;
if (size > ctrl->maximum + 1)
size = ctrl->maximum + 1;
-   ret = copy_from_user(ctrl->string, c->string, size);
+   ret = copy_from_user(ptr.p_char, c->string, size);
if (!ret) {
-   char last = ctrl->string[size - 1];
+   char last = ptr.p_char[size - 1];
 
-   ctrl->string[size - 1] = 0;
+   ptr.p_char[size - 1] = 0;
/* If the string was longer than ctrl->maximum,
   then return an error. */
-   if (strlen(ctrl->string) == ctrl->maximum && last)
+   if (strlen(ptr.p_char) == ctrl->maximum && last)
return -ERANGE;
}
return ret ? -EFAULT : 0;
default:
-   ctrl->val = c->value;
+   *ptr.p_s32 = c->value;
break;
}
return 0;
 }
 
-/* Helper function: copy the new control value back to the caller */
-static int new_to_user(struct v4l2_ext_control *c,
+/* Helper function: copy the caller-provider value as the new control value */
+static int user_to_new(struct v4l2_ext_control *c,
   struct v4l2_ctrl *ctrl)
 {
-   u32 len;
-
-   if (ctrl->is_ptr && !ctrl->is_string)
-   return copy_to_user(c->p, ctrl->p, ctrl->elem_si

Re: [RFCv1 PATCH 00/27] Add property & configuration store support

[Hans Verkuil]

Oops, forgot to mention that the git tree containing these patches can be found
here:

http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/propapi6

Regards,

Hans

On 01/06/2014 03:20 PM, Hans Verkuil wrote:
> This patch series adds support for properties, matrices and configuration
> stores to the control framework.
> 
> See this RFCv2 for a more detailed discussion:
> 
> http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/71822
> 
> Changes since that RFCv2 are:
> 
> - I dropped the 'property' bit in the control ID, instead a new flag is
>   added: V4L2_CTRL_FLAG_PROPERTY.
> - A V4L2_CTRL_FLAG_IS_PTR flag is added to simplify applications: if set, then
>   applications need to use the 'p' field instead of 'val' or 'val64'. This can
>   be deduced from various other fields as well, but that leads to ugly code.
>   This flag is cheap to set and very helpful in applications.
> - Matrix types have been dropped. If cols or rows are > 1, then you have a
>   matrix, so there is no need for specific matrix types.
> - As a result it is no longer possible to set just a sub-rectangle of a
>   matrix. It is however possible to just set the first X elements of
>   a matrix/array. It became too complex to deal with the sub-rectangle,
>   both in the framework, for drivers and for applications, and there are
>   not enough benefits to warrant that effort.
> 
> Other than those changes this patch series implements all the ideas described
> in RFCv2.
> 
> The first 21 patches are pretty definitive and the only thing missing are
> the DocBook patches and a v4l2-controls.txt patch.
> 
> Before I write those I would like to get feedback for this API enhancement.
> The actual API changes are surprisingly small, and most of the work done in
> the patches has more to do with data structure changes needed to simplify
> handling the more complex control types than with actual new code.
> 
> Patch 22 adds a new event that can deal with the new 64-bit ranges and that
> adds a config_store field. However, I am not yet convinced that this is
> really needed. Feedback would be welcome.
> 
> Patches 23-27 add test code for vivi to test matrices and to test the new
> selection properties. This code needs more work, particularly with regards
> to naming.
> 
> A working v4l2-ctl that can handle the new stuff is available here:
> 
> http://git.linuxtv.org/hverkuil/v4l-utils.git/shortlog/refs/heads/propapi
> 
> Regards,
> 
>   Hans
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
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

[RFCv1 PATCH 07/27] v4l2: integrate support for VIDIOC_QUERY_EXT_CTRL.

[Hans Verkuil]

From: Hans Verkuil 

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |  1 +
 drivers/media/v4l2-core/v4l2-dev.c|  2 ++
 drivers/media/v4l2-core/v4l2-ioctl.c  | 31 +++
 drivers/media/v4l2-core/v4l2-subdev.c |  3 +++
 include/media/v4l2-ioctl.h|  2 ++
 5 files changed, 39 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 8f7a6a4..0d9b97e 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -1089,6 +1089,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int 
cmd, unsigned long arg)
case VIDIOC_ENUM_FREQ_BANDS:
case VIDIOC_SUBDEV_G_EDID32:
case VIDIOC_SUBDEV_S_EDID32:
+   case VIDIOC_QUERY_EXT_CTRL:
ret = do_video_ioctl(file, cmd, arg);
break;
 
diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
b/drivers/media/v4l2-core/v4l2-dev.c
index 1cc1749..0418871 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -585,6 +585,8 @@ static void determine_valid_ioctls(struct video_device 
*vdev)
   be valid if the filehandle passed the control handler. */
if (vdev->ctrl_handler || ops->vidioc_queryctrl)
set_bit(_IOC_NR(VIDIOC_QUERYCTRL), valid_ioctls);
+   if (vdev->ctrl_handler || ops->vidioc_query_ext_ctrl)
+   set_bit(_IOC_NR(VIDIOC_QUERY_EXT_CTRL), valid_ioctls);
if (vdev->ctrl_handler || ops->vidioc_g_ctrl || ops->vidioc_g_ext_ctrls)
set_bit(_IOC_NR(VIDIOC_G_CTRL), valid_ioctls);
if (vdev->ctrl_handler || ops->vidioc_s_ctrl || ops->vidioc_s_ext_ctrls)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 68e6b5e..8d3b2d3 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -512,6 +512,19 @@ static void v4l_print_queryctrl(const void *arg, bool 
write_only)
p->step, p->default_value, p->flags);
 }
 
+static void v4l_print_query_ext_ctrl(const void *arg, bool write_only)
+{
+   const struct v4l2_query_ext_ctrl *p = arg;
+
+   pr_cont("config_store=%d, id=0x%x, type=%d, name=%.*s, unit=%.*s, 
min/max=%lld/%lld, "
+   "step=%lld, default=%lld, flags=0x%08x, cols=%u, rows=%u\n",
+   p->config_store, p->id, p->type, (int)sizeof(p->name), 
p->name,
+   (int)sizeof(p->unit), p->unit,
+   p->min.val, p->max.val,
+   p->step.val, p->def.val, p->flags,
+   p->cols, p->rows);
+}
+
 static void v4l_print_querymenu(const void *arg, bool write_only)
 {
const struct v4l2_querymenu *p = arg;
@@ -1502,6 +1515,23 @@ static int v4l_queryctrl(const struct v4l2_ioctl_ops 
*ops,
return -ENOTTY;
 }
 
+static int v4l_query_ext_ctrl(const struct v4l2_ioctl_ops *ops,
+   struct file *file, void *fh, void *arg)
+{
+   struct video_device *vfd = video_devdata(file);
+   struct v4l2_query_ext_ctrl *p = arg;
+   struct v4l2_fh *vfh =
+   test_bit(V4L2_FL_USES_V4L2_FH, &vfd->flags) ? fh : NULL;
+
+   if (vfh && vfh->ctrl_handler)
+   return v4l2_query_ext_ctrl(vfh->ctrl_handler, p);
+   if (vfd->ctrl_handler)
+   return v4l2_query_ext_ctrl(vfd->ctrl_handler, p);
+   if (ops->vidioc_query_ext_ctrl)
+   return ops->vidioc_query_ext_ctrl(file, fh, p);
+   return -ENOTTY;
+}
+
 static int v4l_querymenu(const struct v4l2_ioctl_ops *ops,
struct file *file, void *fh, void *arg)
 {
@@ -2055,6 +2085,7 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
IOCTL_INFO_STD(VIDIOC_DV_TIMINGS_CAP, vidioc_dv_timings_cap, 
v4l_print_dv_timings_cap, INFO_FL_CLEAR(v4l2_dv_timings_cap, type)),
IOCTL_INFO_FNC(VIDIOC_ENUM_FREQ_BANDS, v4l_enum_freq_bands, 
v4l_print_freq_band, 0),
IOCTL_INFO_FNC(VIDIOC_DBG_G_CHIP_INFO, v4l_dbg_g_chip_info, 
v4l_print_dbg_chip_info, INFO_FL_CLEAR(v4l2_dbg_chip_info, match)),
+   IOCTL_INFO_FNC(VIDIOC_QUERY_EXT_CTRL, v4l_query_ext_ctrl, 
v4l_print_query_ext_ctrl, INFO_FL_CTRL | INFO_FL_CLEAR(v4l2_query_ext_ctrl, 
id)),
 };
 #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)
 
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
b/drivers/media/v4l2-core/v4l2-subdev.c
index 996c248..9242daa 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -139,6 +139,9 @@ static long subdev_do_ioctl(struct file *file, unsigned int 
cmd, void *arg)
case VIDIOC_QUERYCTRL:
return v4l2_queryctrl(vfh->ctrl_handler, arg);
 
+   case VIDIOC_QUERY_EXT_CTRL:
+   return v4l2_query_ext_ctrl(vfh->ctrl_handler, arg);
+
case VIDIOC_QUERYMENU:
retur

[RFCv1 PATCH 02/27] v4l2-ctrls: add unit string.

[Hans Verkuil]

From: Hans Verkuil 

The upcoming VIDIOC_QUERY_EXT_CTRL adds support for a unit string. This
allows userspace to show the unit belonging to a particular control.

This patch adds support for the unit string to the control framework.

Signed-off-by: Hans Verkuil 
---
 drivers/media/v4l2-core/v4l2-common.c |  3 ++-
 drivers/media/v4l2-core/v4l2-ctrls.c  | 36 +--
 include/media/v4l2-ctrls.h| 13 +
 3 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-common.c 
b/drivers/media/v4l2-core/v4l2-common.c
index ccaa38f..ee8ea66 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -114,12 +114,13 @@ EXPORT_SYMBOL(v4l2_ctrl_check);
 int v4l2_ctrl_query_fill(struct v4l2_queryctrl *qctrl, s32 _min, s32 _max, s32 
_step, s32 _def)
 {
const char *name;
+   const char *unit = NULL;
s64 min = _min;
s64 max = _max;
u64 step = _step;
s64 def = _def;
 
-   v4l2_ctrl_fill(qctrl->id, &name, &qctrl->type,
+   v4l2_ctrl_fill(qctrl->id, &name, &unit, &qctrl->type,
   &min, &max, &step, &def, &qctrl->flags);
 
if (name == NULL)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index d36d7f5..bb63d2a 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -858,8 +858,9 @@ const char *v4l2_ctrl_get_name(u32 id)
 }
 EXPORT_SYMBOL(v4l2_ctrl_get_name);
 
-void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
-   s64 *min, s64 *max, u64 *step, s64 *def, u32 *flags)
+void v4l2_ctrl_fill(u32 id, const char **name, const char **unit,
+   enum v4l2_ctrl_type *type, s64 *min, s64 *max,
+   u64 *step, s64 *def, u32 *flags)
 {
*name = v4l2_ctrl_get_name(id);
*flags = 0;
@@ -1622,7 +1623,8 @@ unlock:
 /* Add a new control */
 static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
const struct v4l2_ctrl_ops *ops,
-   u32 id, const char *name, enum v4l2_ctrl_type type,
+   u32 id, const char *name, const char *unit,
+   enum v4l2_ctrl_type type,
s64 min, s64 max, u64 step, s64 def,
u32 flags, const char * const *qmenu,
const s64 *qmenu_int, void *priv)
@@ -1670,6 +1672,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct 
v4l2_ctrl_handler *hdl,
ctrl->ops = ops;
ctrl->id = id;
ctrl->name = name;
+   ctrl->unit = unit;
ctrl->type = type;
ctrl->flags = flags;
ctrl->minimum = min;
@@ -1704,6 +1707,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct 
v4l2_ctrl_handler *hdl,
bool is_menu;
struct v4l2_ctrl *ctrl;
const char *name = cfg->name;
+   const char *unit = cfg->unit;
const char * const *qmenu = cfg->qmenu;
const s64 *qmenu_int = cfg->qmenu_int;
enum v4l2_ctrl_type type = cfg->type;
@@ -1714,8 +1718,8 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct 
v4l2_ctrl_handler *hdl,
s64 def = cfg->def;
 
if (name == NULL)
-   v4l2_ctrl_fill(cfg->id, &name, &type, &min, &max, &step,
-   &def, &flags);
+   v4l2_ctrl_fill(cfg->id, &name, &unit, &type,
+  &min, &max, &step, &def, &flags);
 
is_menu = (cfg->type == V4L2_CTRL_TYPE_MENU ||
   cfg->type == V4L2_CTRL_TYPE_INTEGER_MENU);
@@ -1731,7 +1735,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_custom(struct 
v4l2_ctrl_handler *hdl,
return NULL;
}
 
-   ctrl = v4l2_ctrl_new(hdl, cfg->ops, cfg->id, name,
+   ctrl = v4l2_ctrl_new(hdl, cfg->ops, cfg->id, name, unit,
type, min, max,
is_menu ? cfg->menu_skip_mask : step,
def, flags, qmenu, qmenu_int, priv);
@@ -1747,16 +1751,17 @@ struct v4l2_ctrl *v4l2_ctrl_new_std(struct 
v4l2_ctrl_handler *hdl,
u32 id, s64 min, s64 max, u64 step, s64 def)
 {
const char *name;
+   const char *unit = NULL;
enum v4l2_ctrl_type type;
u32 flags;
 
-   v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def, &flags);
+   v4l2_ctrl_fill(id, &name, &unit, &type, &min, &max, &step, &def, 
&flags);
if (type == V4L2_CTRL_TYPE_MENU
|| type == V4L2_CTRL_TYPE_INTEGER_MENU) {
handler_set_err(hdl, -EINVAL);
return NULL;
}
-   return v4l2_ctrl_new(hdl, ops, id, name, type,
+   return v4l2_ctrl_new(hdl, ops, id, name, unit, type,
 min, max, step, def, flags, NULL, NULL, NULL);
 }
 EXPORT_SYMBOL(v4l2_ctrl_new_std);
@@ -1770,6 +1775,7 @@ struct v4l2_ctrl *v

[RFCv1 PATCH 05/27] videodev2.h: add struct v4l2_query_ext_ctrl and VIDIOC_QUERY_EXT_CTRL.

[Hans Verkuil]

From: Hans Verkuil 

Add a new struct and ioctl to extend the amount of information you can
get for a control/property.

It allows you to query controls in a specific configuration store, it
gives back a unit string, the range is now a s64 type, and the matrix
and element size can be reported through cols/rows/elem_size.

Signed-off-by: Hans Verkuil 
---
 include/uapi/linux/videodev2.h | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index c8e2259..2dda52d 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -1272,6 +1272,35 @@ struct v4l2_queryctrl {
__u32reserved[2];
 };
 
+/*  Used in the VIDIOC_QUERY_EXT_CTRL ioctl for querying extended controls */
+struct v4l2_query_ext_ctrl {
+   __u32config_store;
+   __u32id;
+   __u32type;
+   char name[32];
+   char unit[32];
+   union {
+   __s64 val;
+   __u32 reserved[4];
+   } min;
+   union {
+   __s64 val;
+   __u32 reserved[4];
+   } max;
+   union {
+   __u64 val;
+   __u32 reserved[4];
+   } step;
+   union {
+   __s64 val;
+   __u32 reserved[4];
+   } def;
+   __u32flags;
+   __u32cols, rows;
+   __u32elem_size;
+   __u32reserved[16];
+};
+
 /*  Used in the VIDIOC_QUERYMENU ioctl for querying menu items */
 struct v4l2_querymenu {
__u32   id;
@@ -1965,6 +1994,8 @@ struct v4l2_create_buffers {
Never use these in applications! */
 #define VIDIOC_DBG_G_CHIP_INFO  _IOWR('V', 102, struct v4l2_dbg_chip_info)
 
+#define VIDIOC_QUERY_EXT_CTRL  _IOWR('V', 103, struct v4l2_query_ext_ctrl)
+
 /* Reminder: when adding new ioctls please add support for them to
drivers/media/video/v4l2-compat-ioctl32.c as well! */
 
-- 
1.8.5.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

Add private controls to ctrl_handler

[Tom]

Hello,

I want to add some driver specific ctrls to my ctrl-handler which are not
defined in the "/include/uapi/linux/v4l2-controls.h".
I read that I would need the "V4L2_CID_PRIVATE_BASE" to define the new IDs,
but I don't get how I can add them to my ctrl-handler so that they are
accessible by calling VIDIOC_S_CTRL. 

Can someone give me a hint how I can add my own controls to my ctrl-handler?

what I tried:

#define V4L2_SENS_TEST1 (V4L2_CID_PRIVATE_BASE + 1)
#define V4L2_SENS_TEST2 (V4L2_CID_PRIVATE_BASE + 2)
#define V4L2_SENS_TEST3 (V4L2_CID_PRIVATE_BASE + 3)

static int ov3640_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
.
.
.
//I can add the standard controls as usual...

v4l2_ctrl_handler_init(&ov3640->ctrl_handler, 19);

v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
V4L2_CID_BRIGHTNESS, -48, 48, 1, 0);
v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
V4L2_CID_CONTRAST, -12, 12, 1, 0);
v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
V4L2_CID_SATURATION, -32, 32, 1, 0);
.
.
.
//so far so good...

v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
V4L2_SENS_TEST1, 0, 1, 1, 0);
v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
V4L2_SENS_TEST2, 0, 1, 1, 0);
v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
V4L2_SENS_TEST3, 0, 1, 1, 0);

//but I cannot add these three controls like this. 

if (ov3640->ctrl_handler.error) {
dev_err(&client->dev, "control initialization error %d\n",
ov3640->ctrl_handler.error);
ret = ov3640->ctrl_handler.error;
goto done;
}

//the ctrl-handler will give me an ERROR back when adding these 3 controls

}

static int ov3640_ctrl(struct v4l2_ctrl *ctrl, int command)
{
struct ov3640 *ov3640 = container_of(ctrl->handler, struct ov3640,
ctrl_handler);
int ret = 0;

switch (ctrl->id) {
case V4L2_CID_BRIGHTNESS:
ret = ov3640_set_brightness(ov3640, ctrl->val, command);
break;
case V4L2_CID_CONTRAST:
ret = ov3640_set_contrast(ov3640, ctrl->val, command);
break;
case V4L2_CID_SATURATION:
ret = ov3640_set_saturation(ov3640, ctrl->val, command);
break;
case V4L2_SENS_TEST1://!
case V4L2_SENS_TEST2://!
case V4L2_SENS_TEST3://!
ret = ov3640_test(ov3640, ctrl->val, ctrl->id, command);
break;
}

return ret;
}

static int ov3640_set_ctrl(struct v4l2_ctrl *ctrl)
{
int ret;
int command = SET_DATA;

ret = ov3640_ctrl(ctrl, command);
return ret;
}

static int ov3640_get_ctrl(struct v4l2_ctrl *ctrl)
{
int ret;
int command = GET_DATA;

ret = ov3640_ctrl(ctrl, command);
return ret;
}

static struct v4l2_ctrl_ops ov3640_ctrl_ops = {
.s_ctrl = ov3640_set_ctrl,
.g_volatile_ctrl = ov3640_get_ctrl,
};

Best Regards, Tom

--
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: Add private controls to ctrl_handler

[Tom]

Tom  gmx.net> writes:

sorry I forgot to tell that I am using linux version 3.10.

Best Regards, Tom


--
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: Add private controls to ctrl_handler

[Hans Verkuil]

On 01/06/2014 03:30 PM, Tom wrote:
> Hello,
> 
> I want to add some driver specific ctrls to my ctrl-handler which are not
> defined in the "/include/uapi/linux/v4l2-controls.h".
> I read that I would need the "V4L2_CID_PRIVATE_BASE" to define the new IDs,
> but I don't get how I can add them to my ctrl-handler so that they are
> accessible by calling VIDIOC_S_CTRL. 

Don't use V4L2_CID_PRIVATE_BASE, that doesn't work with the control framework
(for good but somewhat obscure reasons).

Instead use (V4L2_CID_USER_BASE | 0x1000) as the base for your private controls.
If you want to upstream the code, then you should define a range for the private
controls of this driver in v4l2-controls.h. Search for e.g. 
V4L2_CID_USER_S2255_BASE
in that header to see how it is done.

Regards,

Hans

> 
> Can someone give me a hint how I can add my own controls to my ctrl-handler?
> 
> what I tried:
> 
> #define V4L2_SENS_TEST1   (V4L2_CID_PRIVATE_BASE + 1)
> #define V4L2_SENS_TEST2   (V4L2_CID_PRIVATE_BASE + 2)
> #define V4L2_SENS_TEST3   (V4L2_CID_PRIVATE_BASE + 3)
> 
> static int ov3640_probe(struct i2c_client *client,
>   const struct i2c_device_id *id)
> {
>   .
>   .
>   .
> //I can add the standard controls as usual...
> 
>   v4l2_ctrl_handler_init(&ov3640->ctrl_handler, 19);
> 
>   v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
>   V4L2_CID_BRIGHTNESS, -48, 48, 1, 0);
>   v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
>   V4L2_CID_CONTRAST, -12, 12, 1, 0);
>   v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
>   V4L2_CID_SATURATION, -32, 32, 1, 0);
>   .
>   .
>   .
> //so far so good...
> 
>   v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
>   V4L2_SENS_TEST1, 0, 1, 1, 0);
>   v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
>   V4L2_SENS_TEST2, 0, 1, 1, 0);
>   v4l2_ctrl_new_std(&ov3640->ctrl_handler, &ov3640_ctrl_ops,
>   V4L2_SENS_TEST3, 0, 1, 1, 0);
> 
> //but I cannot add these three controls like this. 
> 
>   if (ov3640->ctrl_handler.error) {
>   dev_err(&client->dev, "control initialization error %d\n",
>   ov3640->ctrl_handler.error);
>   ret = ov3640->ctrl_handler.error;
>   goto done;
>   }
> 
> //the ctrl-handler will give me an ERROR back when adding these 3 controls
> 
> }
> 
> static int ov3640_ctrl(struct v4l2_ctrl *ctrl, int command)
> {
>   struct ov3640 *ov3640 = container_of(ctrl->handler, struct ov3640,
> ctrl_handler);
>   int ret = 0;
> 
>   switch (ctrl->id) {
>   case V4L2_CID_BRIGHTNESS:
>   ret = ov3640_set_brightness(ov3640, ctrl->val, command);
>   break;
>   case V4L2_CID_CONTRAST:
>   ret = ov3640_set_contrast(ov3640, ctrl->val, command);
>   break;
>   case V4L2_CID_SATURATION:
>   ret = ov3640_set_saturation(ov3640, ctrl->val, command);
>   break;
>   case V4L2_SENS_TEST1://!
>   case V4L2_SENS_TEST2://!
>   case V4L2_SENS_TEST3://!
>   ret = ov3640_test(ov3640, ctrl->val, ctrl->id, command);
>   break;
>   }
> 
>   return ret;
> }
> 
> static int ov3640_set_ctrl(struct v4l2_ctrl *ctrl)
> {
>   int ret;
>   int command = SET_DATA;
> 
>   ret = ov3640_ctrl(ctrl, command);
>   return ret;
> }
> 
> static int ov3640_get_ctrl(struct v4l2_ctrl *ctrl)
> {
>   int ret;
>   int command = GET_DATA;
> 
>   ret = ov3640_ctrl(ctrl, command);
>   return ret;
> }
> 
> static struct v4l2_ctrl_ops ov3640_ctrl_ops = {
>   .s_ctrl = ov3640_set_ctrl,
>   .g_volatile_ctrl = ov3640_get_ctrl,
> };
> 
> Best Regards, Tom
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] tuner-xc2028: Fix xc3028 timeouts

[Mauro Carvalho Chehab]

Em Mon, 06 Jan 2014 08:01:31 -0200
Mauro Carvalho Chehab  escreveu:

> When xc2028/3028 is powered down, it won't response to any command, until
> a firmware is loaded. That means that reading frontend status will fail
> with a timeout.
> 
> Also, any trial to put the device to sleep twice will fail.
> 
> This small series fix those two bugs.
> 
> Mauro Carvalho Chehab (2):
>   tuner-xc2028: Don't try to sleep twice
>   tuner-xc2028: Don't read status if device is powered down
> 
>  drivers/media/tuners/tuner-xc2028.c | 29 ++---
>  1 file changed, 26 insertions(+), 3 deletions(-)
> 


It turns that the errors I was experiencing with tvp5150 on HVR-950 seem
to be a reflex of the xc3028 errors, as they disappeared after this fix.

I suspect that the tvp5150 reads were sent too fast to em28xx, while it 
was still trying to do a read on the powered down xc3028.

That could likely mean that the code at em28xx I2C read would need to
wait for a longer time, or that we may need to send an ACK to em28xx
I2C, via register 0x06 bit 7.

Anyway, as everything is working fine, I'm just dropping the I2C
retry logic. I don't intend to work on any other fixup patch for I2C.

However, as the bug seems to still be hidden somewhere there, better to
have an option to turn on debug messages for timeouts.

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 v4 21/22] [media] em28xx-audio: allocate URBs at device driver init

[Mauro Carvalho Chehab]

Em Sun, 05 Jan 2014 19:25:57 -0200
Mauro Carvalho Chehab  escreveu:

> Em Sun, 05 Jan 2014 22:02:40 +0100
> Frank Schäfer  escreveu:
> 
> > Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab:
> > > From: Mauro Carvalho Chehab 
> > Is this line still correct ? ;)
> > 
> > > Instead of allocating/deallocating URBs and transfer buffers
> > > every time stream is started/stopped, just do it once.
> > >
> > > That reduces the memory allocation pressure and makes the
> > > code that start/stop streaming a way simpler.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab 
> > > Signed-off-by: Mauro Carvalho Chehab 
> > Two Signed-off-by lines ? ;)
> > 
> > > ---
> > >  drivers/media/usb/em28xx/em28xx-audio.c | 128 
> > > ++--
> > >  1 file changed, 73 insertions(+), 55 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/em28xx/em28xx-audio.c 
> > > b/drivers/media/usb/em28xx/em28xx-audio.c
> > > index e5120430ec80..30ee389a07f0 100644
> > > --- a/drivers/media/usb/em28xx/em28xx-audio.c
> > > +++ b/drivers/media/usb/em28xx/em28xx-audio.c
> > > @@ -3,7 +3,7 @@
> > >   *
> > >   *  Copyright (C) 2006 Markus Rechberger 
> > >   *
> > > - *  Copyright (C) 2007-2011 Mauro Carvalho Chehab 
> > > + *  Copyright (C) 2007-2014 Mauro Carvalho Chehab
> > >   *   - Port to work with the in-kernel driver
> > >   *   - Cleanups, fixes, alsa-controls, etc.
> > >   *
> > > @@ -70,16 +70,6 @@ static int em28xx_deinit_isoc_audio(struct em28xx *dev)
> > >   usb_kill_urb(urb);
> > >   else
> > >   usb_unlink_urb(urb);
> > > -
> > > - usb_free_coherent(dev->udev,
> > > -   urb->transfer_buffer_length,
> > > -   dev->adev.transfer_buffer[i],
> > > -   urb->transfer_dma);
> > > -
> > > - dev->adev.transfer_buffer[i] = NULL;
> > > -
> > > - usb_free_urb(urb);
> > > - dev->adev.urb[i] = NULL;
> > >   }
> > >  
> > >   return 0;
> > > @@ -174,53 +164,14 @@ static void em28xx_audio_isocirq(struct urb *urb)
> > >  static int em28xx_init_audio_isoc(struct em28xx *dev)
> > >  {
> > >   int   i, errCode;
> > > - const int sb_size = EM28XX_NUM_AUDIO_PACKETS *
> > > - EM28XX_AUDIO_MAX_PACKET_SIZE;
> > >  
> > >   dprintk("Starting isoc transfers\n");
> > >  
> > > + /* Start streaming */
> > >   for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
> > > - struct urb *urb;
> > > - int j, k;
> > > - void *buf;
> > > -
> > > - urb = usb_alloc_urb(EM28XX_NUM_AUDIO_PACKETS, GFP_ATOMIC);
> > > - if (!urb) {
> > > - em28xx_errdev("usb_alloc_urb failed!\n");
> > > - for (j = 0; j < i; j++) {
> > > - usb_free_urb(dev->adev.urb[j]);
> > > - kfree(dev->adev.transfer_buffer[j]);
> > > - }
> > > - return -ENOMEM;
> > > - }
> > > -
> > > - buf = usb_alloc_coherent(dev->udev, sb_size, GFP_ATOMIC,
> > > -  &urb->transfer_dma);
> > > - if (!buf)
> > > - return -ENOMEM;
> > > - dev->adev.transfer_buffer[i] = buf;
> > > - memset(buf, 0x80, sb_size);
> > > -
> > > - urb->dev = dev->udev;
> > > - urb->context = dev;
> > > - urb->pipe = usb_rcvisocpipe(dev->udev, EM28XX_EP_AUDIO);
> > > - urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
> > > - urb->transfer_buffer = dev->adev.transfer_buffer[i];
> > > - urb->interval = 1;
> > > - urb->complete = em28xx_audio_isocirq;
> > > - urb->number_of_packets = EM28XX_NUM_AUDIO_PACKETS;
> > > - urb->transfer_buffer_length = sb_size;
> > > -
> > > - for (j = k = 0; j < EM28XX_NUM_AUDIO_PACKETS;
> > > -  j++, k += EM28XX_AUDIO_MAX_PACKET_SIZE) {
> > > - urb->iso_frame_desc[j].offset = k;
> > > - urb->iso_frame_desc[j].length =
> > > - EM28XX_AUDIO_MAX_PACKET_SIZE;
> > > - }
> > > - dev->adev.urb[i] = urb;
> > > - }
> > > + memset(dev->adev.transfer_buffer[i], 0x80,
> > > +dev->adev.urb[i]->transfer_buffer_length);
> > >  
> > > - for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
> > >   errCode = usb_submit_urb(dev->adev.urb[i], GFP_ATOMIC);
> > >   if (errCode) {
> > >   em28xx_errdev("submit of audio urb failed\n");
> > > @@ -643,13 +594,36 @@ static struct snd_pcm_ops snd_em28xx_pcm_capture = {
> > >   .page  = snd_pcm_get_vmalloc_page,
> > >  };
> > >  
> > > +static void em28xx_audio_free_urb(struct em28xx *dev)
> > > +{
> > > + int i;
> > > +
> > > + for (i = 0; i < EM28XX_AUDIO_BUFS; i++) {
> > > + struct urb *urb = dev->adev.urb[i];
> > > +
> > > + if (!dev->adev.urb[i])
> > > + continue;
> > > +
> > > + usb_free_coherent(dev->ude

[PATCH 3/6] [media] em28xx: use a better value for I2C timeouts

[Mauro Carvalho Chehab]

In the lack of a better spec, let's assume the timeout
values compatible with SMBus spec:
http://smbus.org/specs/smbus110.pdf

at chapter 8 - Electrical Characteristics of SMBus devices

Ok, SMBus is a subset of I2C, and not all devices will be
following it, but the timeout value before this patch was not
even following the spec.

So, while we don't have a better guess for it, use 35 + 1
ms as the timeout.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx.h | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx.h 
b/drivers/media/usb/em28xx/em28xx.h
index 7ae05ebc13c1..949372e11887 100644
--- a/drivers/media/usb/em28xx/em28xx.h
+++ b/drivers/media/usb/em28xx/em28xx.h
@@ -182,8 +182,21 @@
 
 #define EM28XX_INTERLACED_DEFAULT 1
 
-/* time in msecs to wait for i2c writes to finish */
-#define EM2800_I2C_XFER_TIMEOUT20
+/*
+ * Time in msecs to wait for i2c xfers to finish.
+ * 35ms is the maximum time a SMBUS device could wait when
+ * clock stretching is used. As the transfer itself will take
+ * some time to happen, set it to 35 ms.
+ *
+ * Ok, I2C doesn't specify any limit. So, eventually, we may need
+ * to increase this timeout.
+ *
+ * FIXME: this assumes that an I2C message is not longer than 1ms.
+ * This is actually dependent on the I2C bus speed, although most
+ * devices use a 100kHz clock. So, this assumtion is true most of
+ * the time.
+ */
+#define EM28XX_I2C_XFER_TIMEOUT36
 
 /* max. number of button state polling addresses */
 #define EM28XX_NUM_BUTTON_ADDRESSES_MAX5
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/6] [media] em28xx: rename I2C timeout to EM28XX_I2C_XFER_TIMEOUT

[Mauro Carvalho Chehab]

This macro is used by all em28xx devices, and not just em2800.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 91f9f0a3f05e..4e9915a24488 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -49,7 +49,7 @@ MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]");
  */
 static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 {
-   unsigned long timeout = jiffies + 
msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
+   unsigned long timeout = jiffies + 
msecs_to_jiffies(EM28XX_I2C_XFER_TIMEOUT);
int ret;
u8 b2[6];
 
@@ -99,7 +99,7 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, 
u8 *buf, u16 len)
  */
 static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 {
-   unsigned long timeout = jiffies + 
msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
+   unsigned long timeout = jiffies + 
msecs_to_jiffies(EM28XX_I2C_XFER_TIMEOUT);
u8 buf2[4];
int ret;
int i;
@@ -169,7 +169,7 @@ static int em2800_i2c_check_for_device(struct em28xx *dev, 
u8 addr)
 static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 u16 len, int stop)
 {
-   unsigned long timeout = jiffies + 
msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
+   unsigned long timeout = jiffies + 
msecs_to_jiffies(EM28XX_I2C_XFER_TIMEOUT);
int ret;
 
if (len < 1 || len > 64)
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/6] em28xx: add timeout debug information if i2c_debug enabled

[Mauro Carvalho Chehab]

If i2c_debug is enabled, we splicitly want to know when a
device fails with timeout.

If i2c_debug==2, this is already provided, for each I2C transfer
that fails.

However, most of the time, we don't need to go that far. We just
want to know that I2C transfers fail.

So, add such errors for normal (ret == 0x10) I2C aborted timeouts.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 27 ---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 2410ed09e877..d50334ca8175 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -80,6 +80,9 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, 
u8 *buf, u16 len)
if (ret == 0x80 + len - 1)
return len;
if (ret == 0x94 + len - 1) {
+   if (i2c_debug == 1)
+   em28xx_warn("R05 returned 0x%02x: I2C timeout",
+   ret);
return -ETIMEDOUT;
}
if (ret < 0) {
@@ -124,6 +127,9 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 
addr, u8 *buf, u16 len)
if (ret == 0x84 + len - 1)
break;
if (ret == 0x94 + len - 1) {
+   if (i2c_debug == 1)
+   em28xx_warn("R05 returned 0x%02x: I2C timeout",
+   ret);
return -ETIMEDOUT;
}
if (ret < 0) {
@@ -203,6 +209,9 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 
addr, u8 *buf,
if (ret == 0) /* success */
return len;
if (ret == 0x10) {
+   if (i2c_debug == 1)
+   em28xx_warn("I2C transfer timeout on writing to 
addr 0x%02x",
+   addr);
return -ETIMEDOUT;
}
if (ret < 0) {
@@ -263,8 +272,12 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 
addr, u8 *buf, u16 len)
ret);
return ret;
}
-   if (ret == 0x10)
+   if (ret == 0x10) {
+   if (i2c_debug == 1)
+   em28xx_warn("I2C transfer timeout on writing to addr 
0x%02x",
+   addr);
return -ETIMEDOUT;
+   }
 
em28xx_warn("unknown i2c error (status=%i)\n", ret);
return -ENXIO;
@@ -322,8 +335,12 @@ static int em25xx_bus_B_send_bytes(struct em28xx *dev, u16 
addr, u8 *buf,
 */
if (!ret)
return len;
-   else if (ret > 0)
+   else if (ret > 0) {
+   if (i2c_debug == 1)
+   em28xx_warn("Bus B R08 returned 0x%02x: I2C timeout",
+   ret);
return -ETIMEDOUT;
+   }
 
return ret;
/*
@@ -373,8 +390,12 @@ static int em25xx_bus_B_recv_bytes(struct em28xx *dev, u16 
addr, u8 *buf,
 */
if (!ret)
return len;
-   else if (ret > 0)
+   else if (ret > 0) {
+   if (i2c_debug == 1)
+   em28xx_warn("Bus B R08 returned 0x%02x: I2C timeout",
+   ret);
return -ETIMEDOUT;
+   }
 
return ret;
/*
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/6] em28xx: improve I2C code

[Mauro Carvalho Chehab]

This is a series of cleanup patches for em28xx I2C. It was originally
part of a series of patches meant to split em28xx V4L2 module, but it
makes sense to submit as a separate patch set.

This series basically does:

- make em28xx compilant with standard I2C errors, as defined
  at Documentation/i2c/fault-codes;

- Use jiffies for the timeouts, to make the timeout reliable;

- Better define a value for the I2C timeout, according with
  the SMBUS spec. This value works properly on the devices
  tested (WinTV 2 USB, Kworld 305U and HVR-950);

- Add 2 levels of I2C debug messages. On the first level,
  it displays only the I2C I/O error messages. On the second
  level, it displays the entire I2C transfer data;

- Enable I2C timeout debug errors, if i2c_debug=1.

Thanks to Frank Sch??fer  for his
pach review.

Mauro Carvalho Chehab (6):
  [media] em28xx: convert i2c wait completion logic to use jiffies
  [media] em28xx: rename I2C timeout to EM28XX_I2C_XFER_TIMEOUT
  [media] em28xx: use a better value for I2C timeouts
  em28xx-i2c: Fix error code for I2C error transfers
  [media] em28xx: cleanup I2C debug messages
  em28xx: add timeout debug information if i2c_debug enabled

 drivers/media/usb/em28xx/em28xx-i2c.c | 191 --
 drivers/media/usb/em28xx/em28xx.h |  17 ++-
 2 files changed, 125 insertions(+), 83 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/6] [media] em28xx: convert i2c wait completion logic to use jiffies

[Mauro Carvalho Chehab]

The I2C wait completion/timeout logic currently assumes that
msleep(5) will wait exaclty 5 ms. This is not true at all,
as it depends on CONFIG_HZ.

Convert it to use jiffies, in order to not wait for more time
than needed.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 61 ++-
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
b/drivers/media/usb/em28xx/em28xx-i2c.c
index c4ff9739a7ae..91f9f0a3f05e 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "em28xx.h"
 #include "tuner-xc2028.h"
@@ -48,8 +49,8 @@ MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]");
  */
 static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 {
+   unsigned long timeout = jiffies + 
msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
int ret;
-   int write_timeout;
u8 b2[6];
 
if (len < 1 || len > 4)
@@ -74,14 +75,14 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 
addr, u8 *buf, u16 len)
return (ret < 0) ? ret : -EIO;
}
/* wait for completion */
-   for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0;
-write_timeout -= 5) {
+   while (time_is_after_jiffies(timeout)) {
ret = dev->em28xx_read_reg(dev, 0x05);
-   if (ret == 0x80 + len - 1) {
+   if (ret == 0x80 + len - 1)
return len;
-   } else if (ret == 0x94 + len - 1) {
+   if (ret == 0x94 + len - 1) {
return -ENODEV;
-   } else if (ret < 0) {
+   }
+   if (ret < 0) {
em28xx_warn("failed to get i2c transfer status from 
bridge register (error=%i)\n",
ret);
return ret;
@@ -98,9 +99,9 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, 
u8 *buf, u16 len)
  */
 static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
 {
+   unsigned long timeout = jiffies + 
msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
u8 buf2[4];
int ret;
-   int read_timeout;
int i;
 
if (len < 1 || len > 4)
@@ -117,14 +118,14 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 
addr, u8 *buf, u16 len)
}
 
/* wait for completion */
-   for (read_timeout = EM2800_I2C_XFER_TIMEOUT; read_timeout > 0;
-read_timeout -= 5) {
+   while (time_is_after_jiffies(timeout)) {
ret = dev->em28xx_read_reg(dev, 0x05);
-   if (ret == 0x84 + len - 1) {
+   if (ret == 0x84 + len - 1)
break;
-   } else if (ret == 0x94 + len - 1) {
+   if (ret == 0x94 + len - 1) {
return -ENODEV;
-   } else if (ret < 0) {
+   }
+   if (ret < 0) {
em28xx_warn("failed to get i2c transfer status from 
bridge register (error=%i)\n",
ret);
return ret;
@@ -168,7 +169,8 @@ static int em2800_i2c_check_for_device(struct em28xx *dev, 
u8 addr)
 static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
 u16 len, int stop)
 {
-   int write_timeout, ret;
+   unsigned long timeout = jiffies + 
msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
+   int ret;
 
if (len < 1 || len > 64)
return -EOPNOTSUPP;
@@ -191,16 +193,16 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 
addr, u8 *buf,
}
}
 
-   /* Check success of the i2c operation */
-   for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0;
-write_timeout -= 5) {
+   /* wait for completion */
+   while (time_is_after_jiffies(timeout)) {
ret = dev->em28xx_read_reg(dev, 0x05);
-   if (ret == 0) { /* success */
+   if (ret == 0) /* success */
return len;
-   } else if (ret == 0x10) {
+   if (ret == 0x10) {
return -ENODEV;
-   } else if (ret < 0) {
-   em28xx_warn("failed to read i2c transfer status from 
bridge (error=%i)\n",
+   }
+   if (ret < 0) {
+   em28xx_warn("failed to get i2c transfer status from 
bridge register (error=%i)\n",
ret);
return ret;
}
@@ -211,6 +213,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 
addr, u8 *buf,
 * (even with high payload) ...
 */
}
+
em28xx_warn("write to i2c device at 0x%x timed out

[PATCH 4/6] em28xx-i2c: Fix error code for I2C error transfers

[Mauro Carvalho Chehab]

Follow the error codes for I2C as described at Documentation/i2c/fault-codes.

In the case of the I2C status register (0x05), this is mapped into:

- ETIMEDOUT - when reg 05 returns 0x10
- ENXIO - when the device is not temporarily not responding
  (e. g. reg 05 returning something not 0x10 or 0x00)
- EIO - for generic I/O errors that don't fit into the above.

In the specific case of 0-byte reads, used only during I2C device
probing, it keeps returning -ENODEV.

TODO: return EBUSY when reg 05 returns 0x20 on em2874 and upper.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 41 +++
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 4e9915a24488..8d14be06f088 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -80,7 +80,7 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, 
u8 *buf, u16 len)
if (ret == 0x80 + len - 1)
return len;
if (ret == 0x94 + len - 1) {
-   return -ENODEV;
+   return -ETIMEDOUT;
}
if (ret < 0) {
em28xx_warn("failed to get i2c transfer status from 
bridge register (error=%i)\n",
@@ -90,7 +90,7 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, 
u8 *buf, u16 len)
msleep(5);
}
em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
-   return -EIO;
+   return -ETIMEDOUT;
 }
 
 /*
@@ -123,7 +123,7 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 
addr, u8 *buf, u16 len)
if (ret == 0x84 + len - 1)
break;
if (ret == 0x94 + len - 1) {
-   return -ENODEV;
+   return -ETIMEDOUT;
}
if (ret < 0) {
em28xx_warn("failed to get i2c transfer status from 
bridge register (error=%i)\n",
@@ -199,7 +199,7 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 
addr, u8 *buf,
if (ret == 0) /* success */
return len;
if (ret == 0x10) {
-   return -ENODEV;
+   return -ETIMEDOUT;
}
if (ret < 0) {
em28xx_warn("failed to get i2c transfer status from 
bridge register (error=%i)\n",
@@ -213,9 +213,8 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 
addr, u8 *buf,
 * (even with high payload) ...
 */
}
-
-   em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
-   return -EIO;
+   em28xx_warn("write to i2c device at 0x%x timed out (status=%i)\n", 
addr, ret);
+   return -ENXIO;
 }
 
 /*
@@ -245,7 +244,7 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 
addr, u8 *buf, u16 len)
 * bytes if we are on bus B AND there was no write attempt to the
 * specified slave address before AND no device is present at the
 * requested slave address.
-* Anyway, the next check will fail with -ENODEV in this case, so avoid
+* Anyway, the next check will fail with -ETIMEDOUT in this case, so 
avoid
 * spamming the system log on device probing and do nothing here.
 */
 
@@ -259,10 +258,10 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 
addr, u8 *buf, u16 len)
return ret;
}
if (ret == 0x10)
-   return -ENODEV;
+   return -ETIMEDOUT;
 
em28xx_warn("unknown i2c error (status=%i)\n", ret);
-   return -EIO;
+   return -ENXIO;
 }
 
 /*
@@ -306,7 +305,7 @@ static int em25xx_bus_B_send_bytes(struct em28xx *dev, u16 
addr, u8 *buf,
} else {
em28xx_warn("%i bytes write to i2c device at 0x%x 
requested, but %i bytes written\n",
len, addr, ret);
-   return -EIO;
+   return -ENXIO;
}
}
/* Check success */
@@ -318,7 +317,7 @@ static int em25xx_bus_B_send_bytes(struct em28xx *dev, u16 
addr, u8 *buf,
if (!ret)
return len;
else if (ret > 0)
-   return -ENODEV;
+   return -ETIMEDOUT;
 
return ret;
/*
@@ -356,7 +355,7 @@ static int em25xx_bus_B_recv_bytes(struct em28xx *dev, u16 
addr, u8 *buf,
 * bytes if we are on bus B AND there was no write attempt to the
 * specified slave address before AND no device is present at the
 * requested slave address.
-* Anyway, the next check will fail with -ENODEV in this case, so avoid
+* Anyway, the next check will fail with -ETIMEDOUT in this case, so 
avoid

[PATCH 5/6] [media] em28xx: cleanup I2C debug messages

[Mauro Carvalho Chehab]

The I2C output messages is too polluted. Clean it a little
bit, by:
- use the proper core support for memory dumps;
- hide most stuff under the i2c_debug umbrella;
- add the missing KERN_CONT where needed;
- use 2 levels or verbosity. Only the second one
  will show the I2C transfer data.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/em28xx/em28xx-i2c.c | 84 ++-
 1 file changed, 44 insertions(+), 40 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 8d14be06f088..2410ed09e877 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -41,7 +41,7 @@ MODULE_PARM_DESC(i2c_scan, "scan i2c bus at insmod time");
 
 static unsigned int i2c_debug;
 module_param(i2c_debug, int, 0644);
-MODULE_PARM_DESC(i2c_debug, "enable debug messages [i2c]");
+MODULE_PARM_DESC(i2c_debug, "i2c debug message level (1: normal debug, 2: show 
I2C transfers)");
 
 /*
  * em2800_i2c_send_bytes()
@@ -89,7 +89,8 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, 
u8 *buf, u16 len)
}
msleep(5);
}
-   em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
+   if (i2c_debug)
+   em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
return -ETIMEDOUT;
 }
 
@@ -132,8 +133,11 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 
addr, u8 *buf, u16 len)
}
msleep(5);
}
-   if (ret != 0x84 + len - 1)
-   em28xx_warn("read from i2c device at 0x%x timed out\n", addr);
+   if (ret != 0x84 + len - 1) {
+   if (i2c_debug)
+   em28xx_warn("read from i2c device at 0x%x timed out\n",
+   addr);
+   }
 
/* get the received message */
ret = dev->em28xx_read_reg_req_len(dev, 0x00, 4-len, buf2, len);
@@ -213,7 +217,9 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 
addr, u8 *buf,
 * (even with high payload) ...
 */
}
-   em28xx_warn("write to i2c device at 0x%x timed out (status=%i)\n", 
addr, ret);
+   if (i2c_debug)
+   em28xx_warn("write to i2c device at 0x%x timed out 
(status=%i)\n",
+   addr, ret);
return -ENXIO;
 }
 
@@ -409,10 +415,6 @@ static inline int i2c_check_for_device(struct 
em28xx_i2c_bus *i2c_bus, u16 addr)
rc = em2800_i2c_check_for_device(dev, addr);
else if (i2c_bus->algo_type == EM28XX_I2C_ALGO_EM25XX_BUS_B)
rc = em25xx_bus_B_check_for_device(dev, addr);
-   if (rc == -ETIMEDOUT) {
-   if (i2c_debug)
-   printk(" timeout\n");
-   }
return rc;
 }
 
@@ -421,7 +423,7 @@ static inline int i2c_recv_bytes(struct em28xx_i2c_bus 
*i2c_bus,
 {
struct em28xx *dev = i2c_bus->dev;
u16 addr = msg.addr << 1;
-   int byte, rc = -EOPNOTSUPP;
+   int rc = -EOPNOTSUPP;
 
if (i2c_bus->algo_type == EM28XX_I2C_ALGO_EM28XX)
rc = em28xx_i2c_recv_bytes(dev, addr, msg.buf, msg.len);
@@ -429,10 +431,6 @@ static inline int i2c_recv_bytes(struct em28xx_i2c_bus 
*i2c_bus,
rc = em2800_i2c_recv_bytes(dev, addr, msg.buf, msg.len);
else if (i2c_bus->algo_type == EM28XX_I2C_ALGO_EM25XX_BUS_B)
rc = em25xx_bus_B_recv_bytes(dev, addr, msg.buf, msg.len);
-   if (i2c_debug) {
-   for (byte = 0; byte < msg.len; byte++)
-   printk(" %02x", msg.buf[byte]);
-   }
return rc;
 }
 
@@ -441,12 +439,8 @@ static inline int i2c_send_bytes(struct em28xx_i2c_bus 
*i2c_bus,
 {
struct em28xx *dev = i2c_bus->dev;
u16 addr = msg.addr << 1;
-   int byte, rc = -EOPNOTSUPP;
+   int rc = -EOPNOTSUPP;
 
-   if (i2c_debug) {
-   for (byte = 0; byte < msg.len; byte++)
-   printk(" %02x", msg.buf[byte]);
-   }
if (i2c_bus->algo_type == EM28XX_I2C_ALGO_EM28XX)
rc = em28xx_i2c_send_bytes(dev, addr, msg.buf, msg.len, stop);
else if (i2c_bus->algo_type == EM28XX_I2C_ALGO_EM2800)
@@ -491,7 +485,7 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
}
for (i = 0; i < num; i++) {
addr = msgs[i].addr << 1;
-   if (i2c_debug)
+   if (i2c_debug > 1)
printk(KERN_DEBUG "%s at %s: %s %s addr=%02x len=%d:",
   dev->name, __func__ ,
   (msgs[i].flags & I2C_M_RD) ? "read" : "write",
@@ -503,25 +497,41 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
 * This code is only called during device probe.
 */
rc = i2c_check_for_device(i2c_bus, addr);
-

Re: [PATCH v4 04/22] [media] em28xx: make em28xx-video to be a separate module

[Frank Schäfer]

Am 05.01.2014 13:56, schrieb Mauro Carvalho Chehab:
> Em Sun, 05 Jan 2014 11:47:00 +0100
> Frank Schäfer  escreveu:
>
>> Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab:
>>> Now that all analog-specific code are at em28xx-video, convert
>>> it into an em28xx extension and load it as a separate module.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab 
>>> ---
>>>  drivers/media/usb/em28xx/Kconfig |  6 ++-
>>>  drivers/media/usb/em28xx/Makefile|  5 ++-
>>>  drivers/media/usb/em28xx/em28xx-camera.c |  1 +
>>>  drivers/media/usb/em28xx/em28xx-cards.c  | 45 ---
>>>  drivers/media/usb/em28xx/em28xx-core.c   | 12 ++
>>>  drivers/media/usb/em28xx/em28xx-v4l.h| 20 +
>>>  drivers/media/usb/em28xx/em28xx-vbi.c|  1 +
>>>  drivers/media/usb/em28xx/em28xx-video.c  | 74 
>>> +++-
>>>  drivers/media/usb/em28xx/em28xx.h| 23 ++
>>>  9 files changed, 107 insertions(+), 80 deletions(-)
>>>  create mode 100644 drivers/media/usb/em28xx/em28xx-v4l.h
>>>
>>> diff --git a/drivers/media/usb/em28xx/Kconfig 
>>> b/drivers/media/usb/em28xx/Kconfig
>>> index d6ba514d31eb..838fc9dbb747 100644
>>> --- a/drivers/media/usb/em28xx/Kconfig
>>> +++ b/drivers/media/usb/em28xx/Kconfig
>>> @@ -1,8 +1,12 @@
>>>  config VIDEO_EM28XX
>>> -   tristate "Empia EM28xx USB video capture support"
>>> +   tristate "Empia EM28xx USB devices support"
>>> depends on VIDEO_DEV && I2C
>>> select VIDEO_TUNER
>>> select VIDEO_TVEEPROM
>>> +
>>> +config VIDEO_EM28XX_V4L2
>>> +   tristate "Empia EM28xx analog TV, video capture and/or webcam support"
>>> +   depends on VIDEO_EM28XX && I2C
>> VIDEO_EM28XX already depends on I2C.
>>
>>> select VIDEOBUF2_VMALLOC
>>> select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
>>> select VIDEO_TVP5150 if MEDIA_SUBDRV_AUTOSELECT
>>> diff --git a/drivers/media/usb/em28xx/Makefile 
>>> b/drivers/media/usb/em28xx/Makefile
>>> index ad6d48557940..3f850d5063d0 100644
>>> --- a/drivers/media/usb/em28xx/Makefile
>>> +++ b/drivers/media/usb/em28xx/Makefile
>>> @@ -1,10 +1,11 @@
>>> -em28xx-y +=em28xx-video.o em28xx-i2c.o em28xx-cards.o
>>> -em28xx-y +=em28xx-core.o  em28xx-vbi.o em28xx-camera.o
>>> +em28xx-y +=em28xx-core.o em28xx-i2c.o em28xx-cards.o 
>>> em28xx-camera.o
>>>  
>>> +em28xx-v4l-objs := em28xx-video.o em28xx-vbi.o
>>>  em28xx-alsa-objs := em28xx-audio.o
>>>  em28xx-rc-objs := em28xx-input.o
>>>  
>>>  obj-$(CONFIG_VIDEO_EM28XX) += em28xx.o
>>> +obj-$(CONFIG_VIDEO_EM28XX_V4L2) += em28xx-v4l.o
>>>  obj-$(CONFIG_VIDEO_EM28XX_ALSA) += em28xx-alsa.o
>>>  obj-$(CONFIG_VIDEO_EM28XX_DVB) += em28xx-dvb.o
>>>  obj-$(CONFIG_VIDEO_EM28XX_RC) += em28xx-rc.o
>>> diff --git a/drivers/media/usb/em28xx/em28xx-camera.c 
>>> b/drivers/media/usb/em28xx/em28xx-camera.c
>>> index d666741797d4..c29f5c4e7b40 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-camera.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-camera.c
>>> @@ -454,3 +454,4 @@ int em28xx_init_camera(struct em28xx *dev)
>>>  
>>> return ret;
>>>  }
>>> +EXPORT_SYMBOL_GPL(em28xx_init_camera);
>> em28xx-camera should also be part of the em28xx-v4l module.
> I tried that. Moving em28xx-camera to em28xx-v4l would cause a recursive
> dependency, due to the camera probing logic, that should be called by
> em28xx-cards.c.
I see...

> Moving that probing part to em28xx-v4l is too complex, due to the code
> that detects the board.
>
> One solution would be to break em28xx-camera into two parts: the detection
> code, to be merged with the core, and the sensor part, to be merged with
> em28xx-v4l, but that sounds ugly.
>
> Other solution would be to use something like the dvb_attach() macro,
> to avoid the recursive dependency, but that would be a dirty solution.
Another solution would be to move the sensor probing to em28xx-v4l, too.
Then we need to rely on dev->board.is_webcam to load the v4l module.
This would of course also require to split the board hint stuff.
Even more ugly I think...

> As the code there is not big, the amount of overhead of having everything
> at em28xx-core is not high. So, I opted to keep the simplest path here,
> avoiding the risk of breaking something with a complex changeset.
I agree.

>
>>> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
>>> b/drivers/media/usb/em28xx/em28xx-cards.c
>>> index 175cd776e0a1..938daaabd8e0 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-cards.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
>>> @@ -2159,6 +2159,8 @@ struct em28xx_board em28xx_boards[] = {
>>> .ir_codes  = RC_MAP_PINNACLE_PCTV_HD,
>>> },
>>>  };
>>> +EXPORT_SYMBOL_GPL(em28xx_boards);
>>> +
>>>  const unsigned int em28xx_bcount = ARRAY_SIZE(em28xx_boards);
>>>  
>>>  /* table of devices that work with this driver */
>>> @@ -2827,10 +2829,6 @@ static void em28xx_card_setup(struct em28xx *dev)
>>> "tuner", dev->tuner_addr, NULL);
>>> }
>>> }
>>> -
>>> -   em2

Re: [PATCH v4 07/22] [media] em28xx: improve extension information messages

[Frank Schäfer]

Am 05.01.2014 14:08, schrieb Mauro Carvalho Chehab:
> Em Sun, 05 Jan 2014 11:55:34 +0100
> Frank Schäfer  escreveu:
>
>> Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab:
>>> Add a message with consistent prints before and after each
>>> extension initialization, and provide a better text for module
>>> load.
>>>
>>> While here, add a missing sanity check for extension finish
>>> code at em28xx-v4l extension.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab 
>>> ---
>>>  drivers/media/usb/em28xx/em28xx-audio.c |  4 +++-
>>>  drivers/media/usb/em28xx/em28xx-core.c  |  2 +-
>>>  drivers/media/usb/em28xx/em28xx-dvb.c   |  7 ---
>>>  drivers/media/usb/em28xx/em28xx-input.c |  4 
>>>  drivers/media/usb/em28xx/em28xx-video.c | 10 --
>>>  5 files changed, 20 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/media/usb/em28xx/em28xx-audio.c 
>>> b/drivers/media/usb/em28xx/em28xx-audio.c
>>> index 2fdb66ee44ab..263886adcf26 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-audio.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-audio.c
>>> @@ -649,7 +649,8 @@ static int em28xx_audio_init(struct em28xx *dev)
>>> return 0;
>>> }
>>>  
>>> -   printk(KERN_INFO "em28xx-audio.c: probing for em28xx Audio Vendor 
>>> Class\n");
>>> +   em28xx_info("Binding audio extension\n");
>>> +
>>> printk(KERN_INFO "em28xx-audio.c: Copyright (C) 2006 Markus "
>>>  "Rechberger\n");
>>> printk(KERN_INFO "em28xx-audio.c: Copyright (C) 2007-2011 Mauro 
>>> Carvalho Chehab\n");
>>> @@ -702,6 +703,7 @@ static int em28xx_audio_init(struct em28xx *dev)
>>> adev->sndcard = card;
>>> adev->udev = dev->udev;
>>>  
>>> +   em28xx_info("Audio extension successfully initialized\n");
>>> return 0;
>>>  }
>>>  
>>> diff --git a/drivers/media/usb/em28xx/em28xx-core.c 
>>> b/drivers/media/usb/em28xx/em28xx-core.c
>>> index 1113d4e107d8..33cf26e106b5 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-core.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-core.c
>>> @@ -1069,7 +1069,7 @@ int em28xx_register_extension(struct em28xx_ops *ops)
>>> ops->init(dev);
>>> }
>>> mutex_unlock(&em28xx_devlist_mutex);
>>> -   printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name);
>>> +   printk(KERN_INFO "em28xx: Registered (%s) extension\n", ops->name);
>>> return 0;
>>>  }
>>>  EXPORT_SYMBOL(em28xx_register_extension);
>>> diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c 
>>> b/drivers/media/usb/em28xx/em28xx-dvb.c
>>> index ddc0e609065d..f72663a9b5c5 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-dvb.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-dvb.c
>>> @@ -274,7 +274,7 @@ static int em28xx_stop_feed(struct dvb_demux_feed *feed)
>>>  static int em28xx_dvb_bus_ctrl(struct dvb_frontend *fe, int acquire)
>>>  {
>>> struct em28xx_i2c_bus *i2c_bus = fe->dvb->priv;
>>> -struct em28xx *dev = i2c_bus->dev;
>>> +   struct em28xx *dev = i2c_bus->dev;
>>>  
>>> if (acquire)
>>> return em28xx_set_mode(dev, EM28XX_DIGITAL_MODE);
>>> @@ -992,10 +992,11 @@ static int em28xx_dvb_init(struct em28xx *dev)
>>>  
>>> if (!dev->board.has_dvb) {
>>> /* This device does not support the extension */
>>> -   printk(KERN_INFO "em28xx_dvb: This device does not support the 
>>> extension\n");
>>> return 0;
>>> }
>>>  
>>> +   em28xx_info("Binding DVB extension\n");
>>> +
>>> dvb = kzalloc(sizeof(struct em28xx_dvb), GFP_KERNEL);
>>>  
>>> if (dvb == NULL) {
>>> @@ -1407,7 +1408,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
>>> /* MFE lock */
>>> dvb->adapter.mfe_shared = mfe_shared;
>>>  
>>> -   em28xx_info("Successfully loaded em28xx-dvb\n");
>>> +   em28xx_info("DVB extension successfully initialized\n");
>>>  ret:
>>> em28xx_set_mode(dev, EM28XX_SUSPEND);
>>> mutex_unlock(&dev->lock);
>>> diff --git a/drivers/media/usb/em28xx/em28xx-input.c 
>>> b/drivers/media/usb/em28xx/em28xx-input.c
>>> index 93a7d02b9cb4..eed7dd79f734 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-input.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-input.c
>>> @@ -692,6 +692,8 @@ static int em28xx_ir_init(struct em28xx *dev)
>>> return 0;
>>> }
>>>  
>>> +   em28xx_info("Registering input extension\n");
>>> +
>>> ir = kzalloc(sizeof(*ir), GFP_KERNEL);
>>> rc = rc_allocate_device();
>>> if (!ir || !rc)
>>> @@ -785,6 +787,8 @@ static int em28xx_ir_init(struct em28xx *dev)
>>> if (err)
>>> goto error;
>>>  
>>> +   em28xx_info("Input extension successfully initalized\n");
>>> +
>>> return 0;
>>>  
>>>  error:
>>> diff --git a/drivers/media/usb/em28xx/em28xx-video.c 
>>> b/drivers/media/usb/em28xx/em28xx-video.c
>>> index 56d1b46164a0..b767262c642b 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-video.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-video.c
>>> @@ -1884,6 +1884,11 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
>>>  
>>> /*FIXME: I2C IR should be discon

Re: [PATCH v4 08/22] [media] em28xx: convert i2c wait completion logic to use jiffies

[Frank Schäfer]

Am 05.01.2014 14:10, schrieb Mauro Carvalho Chehab:
> Em Sun, 05 Jan 2014 12:03:51 +0100
> Frank Schäfer  escreveu:
>
>> Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab:
>>> The I2C wait completion/timeout logic currently assumes that
>>> msleep(5) will wait exaclty 5 ms. This is not true at all,
>>> as it depends on CONFIG_HZ.
>>>
>>> Convert it to use jiffies, in order to not wait for more time
>>> than needed.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab 
>>> ---
>>>  drivers/media/usb/em28xx/em28xx-i2c.c | 65 
>>> ++-
>>>  1 file changed, 34 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c 
>>> b/drivers/media/usb/em28xx/em28xx-i2c.c
>>> index 9e6a11d01858..9fa7ed51e5b1 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-i2c.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-i2c.c
>>> @@ -26,6 +26,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  
>>>  #include "em28xx.h"
>>>  #include "tuner-xc2028.h"
>>> @@ -48,8 +49,8 @@ MODULE_PARM_DESC(i2c_debug, "enable debug messages 
>>> [i2c]");
>>>   */
>>>  static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 
>>> len)
>>>  {
>>> +   unsigned long timeout = jiffies + 
>>> msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
>>> int ret;
>>> -   int write_timeout;
>>> u8 b2[6];
>>>  
>>> if (len < 1 || len > 4)
>>> @@ -74,15 +75,15 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 
>>> addr, u8 *buf, u16 len)
>>> return (ret < 0) ? ret : -EIO;
>>> }
>>> /* wait for completion */
>>> -   for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0;
>>> -write_timeout -= 5) {
>>> +   while (time_is_after_jiffies(timeout)) {
>> AFAIU, it must be time_is_before_jiffies(timeout).
> This is tricky, but it is right. 
>
> See its description at jiffies.h:
>
>   /* time_is_after_jiffies(a) return true if a is after jiffies */
>   #define time_is_after_jiffies(a) time_before(jiffies, a)
Urgh... yes, you are right.
I've read this, but didn't notice that we check the timeout jiffies and
not the current jiffies. :/
Sorry for the noise.

>
>>> ret = dev->em28xx_read_reg(dev, 0x05);
>>> -   if (ret == 0x80 + len - 1) {
>>> +   if (ret == 0x80 + len - 1)
>>> return len;
>>> -   } else if (ret == 0x94 + len - 1) {
>>> +   if (ret == 0x94 + len - 1) {
>>> em28xx_warn("R05 returned 0x%02x: I2C timeout", ret);
>>> return -ENODEV;
>>> -   } else if (ret < 0) {
>>> +   }
>>> +   if (ret < 0) {
>>> em28xx_warn("failed to get i2c transfer status from 
>>> bridge register (error=%i)\n",
>>> ret);
>>> return ret;
>>> @@ -99,9 +100,9 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8 
>>> addr, u8 *buf, u16 len)
>>>   */
>>>  static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 
>>> len)
>>>  {
>>> +   unsigned long timeout = jiffies + 
>>> msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
>>> u8 buf2[4];
>>> int ret;
>>> -   int read_timeout;
>>> int i;
>>>  
>>> if (len < 1 || len > 4)
>>> @@ -118,15 +119,15 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev, 
>>> u8 addr, u8 *buf, u16 len)
>>> }
>>>  
>>> /* wait for completion */
>>> -   for (read_timeout = EM2800_I2C_XFER_TIMEOUT; read_timeout > 0;
>>> -read_timeout -= 5) {
>>> +   while (time_is_after_jiffies(timeout)) {
>> The same here...
>>
>>> ret = dev->em28xx_read_reg(dev, 0x05);
>>> -   if (ret == 0x84 + len - 1) {
>>> +   if (ret == 0x84 + len - 1)
>>> break;
>>> -   } else if (ret == 0x94 + len - 1) {
>>> +   if (ret == 0x94 + len - 1) {
>>> em28xx_warn("R05 returned 0x%02x: I2C timeout", ret);
>>> return -ENODEV;
>>> -   } else if (ret < 0) {
>>> +   }
>>> +   if (ret < 0) {
>>> em28xx_warn("failed to get i2c transfer status from 
>>> bridge register (error=%i)\n",
>>> ret);
>>> return ret;
>>> @@ -170,7 +171,8 @@ static int em2800_i2c_check_for_device(struct em28xx 
>>> *dev, u8 addr)
>>>  static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
>>>  u16 len, int stop)
>>>  {
>>> -   int write_timeout, ret;
>>> +   unsigned long timeout = jiffies + 
>>> msecs_to_jiffies(EM2800_I2C_XFER_TIMEOUT);
>>> +   int ret;
>>>  
>>> if (len < 1 || len > 64)
>>> return -EOPNOTSUPP;
>>> @@ -193,17 +195,18 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, 
>>> u16 addr, u8 *buf,
>>> }
>>> }
>>>  
>>> -   /* Check success of the i2c operation */
>>> -   for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0;
>>> -write_timeout -= 5)

Re: [PATCH v4 07/22] [media] em28xx: improve extension information messages

[Mauro Carvalho Chehab]

Em Mon, 06 Jan 2014 18:44:02 +0100
Frank Schäfer  escreveu:

> Am 05.01.2014 14:08, schrieb Mauro Carvalho Chehab:
> > Em Sun, 05 Jan 2014 11:55:34 +0100
> > Frank Schäfer  escreveu:
> >
> >> Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab:
> >>> Add a message with consistent prints before and after each
> >>> extension initialization, and provide a better text for module
> >>> load.
> >>>
> >>> While here, add a missing sanity check for extension finish
> >>> code at em28xx-v4l extension.
> >>>
> >>> Signed-off-by: Mauro Carvalho Chehab 
> >>> ---
> >>>  drivers/media/usb/em28xx/em28xx-audio.c |  4 +++-
> >>>  drivers/media/usb/em28xx/em28xx-core.c  |  2 +-
> >>>  drivers/media/usb/em28xx/em28xx-dvb.c   |  7 ---
> >>>  drivers/media/usb/em28xx/em28xx-input.c |  4 
> >>>  drivers/media/usb/em28xx/em28xx-video.c | 10 --
> >>>  5 files changed, 20 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/drivers/media/usb/em28xx/em28xx-audio.c 
> >>> b/drivers/media/usb/em28xx/em28xx-audio.c
> >>> index 2fdb66ee44ab..263886adcf26 100644
> >>> --- a/drivers/media/usb/em28xx/em28xx-audio.c
> >>> +++ b/drivers/media/usb/em28xx/em28xx-audio.c
> >>> @@ -649,7 +649,8 @@ static int em28xx_audio_init(struct em28xx *dev)
> >>>   return 0;
> >>>   }
> >>>  
> >>> - printk(KERN_INFO "em28xx-audio.c: probing for em28xx Audio Vendor 
> >>> Class\n");
> >>> + em28xx_info("Binding audio extension\n");
> >>> +
> >>>   printk(KERN_INFO "em28xx-audio.c: Copyright (C) 2006 Markus "
> >>>"Rechberger\n");
> >>>   printk(KERN_INFO "em28xx-audio.c: Copyright (C) 2007-2011 Mauro 
> >>> Carvalho Chehab\n");
> >>> @@ -702,6 +703,7 @@ static int em28xx_audio_init(struct em28xx *dev)
> >>>   adev->sndcard = card;
> >>>   adev->udev = dev->udev;
> >>>  
> >>> + em28xx_info("Audio extension successfully initialized\n");
> >>>   return 0;
> >>>  }
> >>>  
> >>> diff --git a/drivers/media/usb/em28xx/em28xx-core.c 
> >>> b/drivers/media/usb/em28xx/em28xx-core.c
> >>> index 1113d4e107d8..33cf26e106b5 100644
> >>> --- a/drivers/media/usb/em28xx/em28xx-core.c
> >>> +++ b/drivers/media/usb/em28xx/em28xx-core.c
> >>> @@ -1069,7 +1069,7 @@ int em28xx_register_extension(struct em28xx_ops 
> >>> *ops)
> >>>   ops->init(dev);
> >>>   }
> >>>   mutex_unlock(&em28xx_devlist_mutex);
> >>> - printk(KERN_INFO "Em28xx: Initialized (%s) extension\n", ops->name);
> >>> + printk(KERN_INFO "em28xx: Registered (%s) extension\n", ops->name);
> >>>   return 0;
> >>>  }
> >>>  EXPORT_SYMBOL(em28xx_register_extension);
> >>> diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c 
> >>> b/drivers/media/usb/em28xx/em28xx-dvb.c
> >>> index ddc0e609065d..f72663a9b5c5 100644
> >>> --- a/drivers/media/usb/em28xx/em28xx-dvb.c
> >>> +++ b/drivers/media/usb/em28xx/em28xx-dvb.c
> >>> @@ -274,7 +274,7 @@ static int em28xx_stop_feed(struct dvb_demux_feed 
> >>> *feed)
> >>>  static int em28xx_dvb_bus_ctrl(struct dvb_frontend *fe, int acquire)
> >>>  {
> >>>   struct em28xx_i2c_bus *i2c_bus = fe->dvb->priv;
> >>> -struct em28xx *dev = i2c_bus->dev;
> >>> + struct em28xx *dev = i2c_bus->dev;
> >>>  
> >>>   if (acquire)
> >>>   return em28xx_set_mode(dev, EM28XX_DIGITAL_MODE);
> >>> @@ -992,10 +992,11 @@ static int em28xx_dvb_init(struct em28xx *dev)
> >>>  
> >>>   if (!dev->board.has_dvb) {
> >>>   /* This device does not support the extension */
> >>> - printk(KERN_INFO "em28xx_dvb: This device does not support the 
> >>> extension\n");
> >>>   return 0;
> >>>   }
> >>>  
> >>> + em28xx_info("Binding DVB extension\n");
> >>> +
> >>>   dvb = kzalloc(sizeof(struct em28xx_dvb), GFP_KERNEL);
> >>>  
> >>>   if (dvb == NULL) {
> >>> @@ -1407,7 +1408,7 @@ static int em28xx_dvb_init(struct em28xx *dev)
> >>>   /* MFE lock */
> >>>   dvb->adapter.mfe_shared = mfe_shared;
> >>>  
> >>> - em28xx_info("Successfully loaded em28xx-dvb\n");
> >>> + em28xx_info("DVB extension successfully initialized\n");
> >>>  ret:
> >>>   em28xx_set_mode(dev, EM28XX_SUSPEND);
> >>>   mutex_unlock(&dev->lock);
> >>> diff --git a/drivers/media/usb/em28xx/em28xx-input.c 
> >>> b/drivers/media/usb/em28xx/em28xx-input.c
> >>> index 93a7d02b9cb4..eed7dd79f734 100644
> >>> --- a/drivers/media/usb/em28xx/em28xx-input.c
> >>> +++ b/drivers/media/usb/em28xx/em28xx-input.c
> >>> @@ -692,6 +692,8 @@ static int em28xx_ir_init(struct em28xx *dev)
> >>>   return 0;
> >>>   }
> >>>  
> >>> + em28xx_info("Registering input extension\n");
> >>> +
> >>>   ir = kzalloc(sizeof(*ir), GFP_KERNEL);
> >>>   rc = rc_allocate_device();
> >>>   if (!ir || !rc)
> >>> @@ -785,6 +787,8 @@ static int em28xx_ir_init(struct em28xx *dev)
> >>>   if (err)
> >>>   goto error;
> >>>  
> >>> + em28xx_info("Input extension successfully initalized\n");
> >>> +
> >>>   return 0;
> >>>  
> >>>  error:
> >>> diff --git a/drivers/media/usb/em28xx/em28xx-video.c 
> >>> b/drivers/media/usb/em28xx/em28xx-video.c
> >>> index 56d1b46164a0..b767262c64

How to enable "CONFIG_V4L2_MEM2MEM_DEV"

[m silverstri]

I have added 'CONFIG_V4L2_MEM2MEM_DEV=y'  to my 'imx_v6_v7_defconfig'
and do a "make ARCH=arm CROSS_COMPILE=/usr/bin/arm-linux-gnueabi-
imx_v6_v7_defconfig", I don't see CONFIG_V4L2_MEM2MEM_DEV being set to
'y' in .config that was automatically generated.

I think I am making changes to the right 'imx_v6_v7_defconfig' file
since when i add
'CONFIG_V4L_TEST_DRIVERS=y' to 'imx_v6_v7_defconfig', I see
'CONFIG_V4L_TEST_DRIVERS=y' in .config when I do "make ARCH=arm
CROSS_COMPILE=/usr/bin/arm-linux-gnueabi- imx_v6_v7_defconfig"

I am not sure why CONFIG_V4L2_MEM2MEM_DEV is not being set when I put
'CONFIG_V4L2_MEM2MEM_DEV=y'

And from the  Kconfig, V4L2_MEM2MEM_DEV only depends on VIDEOBUF2_CORE
# Used by drivers that need v4l2-mem2mem.ko
config V4L2_MEM2MEM_DEV
tristate
depends on VIDEOBUF2_CORE

and I have CONFIG_VIDEOBUF2_CORE=y


Thank you.
--
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 v4 03/22] [media] em28xx: move analog-specific init to em28xx-video

[Frank Schäfer]

Am 05.01.2014 15:40, schrieb Mauro Carvalho Chehab:
> Em Sun, 05 Jan 2014 11:26:14 +0100
> Frank Schäfer  escreveu:
>
>> Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab:
>>> There are several init code inside em28xx-cards that are actually
>>> part of analog initialization. Move the code to em28x-video, in
>>> order to remove part of the mess.
>>>
>>> In thesis, no functional changes so far.
>>>
>>> Signed-off-by: Mauro Carvalho Chehab 
>>> ---
>>>  drivers/media/usb/em28xx/em28xx-cards.c | 71 -
>>>  drivers/media/usb/em28xx/em28xx-video.c | 91 
>>> ++---
>>>  2 files changed, 84 insertions(+), 78 deletions(-)
>>>
>>> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
>>> b/drivers/media/usb/em28xx/em28xx-cards.c
>>> index 551cbc294190..175cd776e0a1 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-cards.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
>>> @@ -2907,7 +2907,6 @@ static int em28xx_init_dev(struct em28xx *dev, struct 
>>> usb_device *udev,
>>>struct usb_interface *interface,
>>>int minor)
>>>  {
>>> -   struct v4l2_ctrl_handler *hdl = &dev->ctrl_handler;
>>> int retval;
>>> static const char *default_chip_name = "em28xx";
>>> const char *chip_name = default_chip_name;
>>> @@ -3034,15 +3033,6 @@ static int em28xx_init_dev(struct em28xx *dev, 
>>> struct usb_device *udev,
>>> }
>>> }
>>>  
>>> -   retval = v4l2_device_register(&interface->dev, &dev->v4l2_dev);
>>> -   if (retval < 0) {
>>> -   em28xx_errdev("Call to v4l2_device_register() failed!\n");
>>> -   return retval;
>>> -   }
>>> -
>>> -   v4l2_ctrl_handler_init(hdl, 8);
>>> -   dev->v4l2_dev.ctrl_handler = hdl;
>>> -
>>> rt_mutex_init(&dev->i2c_bus_lock);
>>>  
>>> /* register i2c bus 0 */
>>> @@ -3071,72 +3061,14 @@ static int em28xx_init_dev(struct em28xx *dev, 
>>> struct usb_device *udev,
>>> }
>>> }
>>>  
>>> -   /*
>>> -* Default format, used for tvp5150 or saa711x output formats
>>> -*/
>>> -   dev->vinmode = 0x10;
>>> -   dev->vinctl  = EM28XX_VINCTRL_INTERLACED |
>>> -  EM28XX_VINCTRL_CCIR656_ENABLE;
>>> -
>>> /* Do board specific init and eeprom reading */
>>> em28xx_card_setup(dev);
>>>  
>> em28xx_card_setup() initializes some v4l2 subdevices, but now the v4l2
>> device (dev->v4l2_dev) isn't ready at this point, because
>> v4l2_device_register() isn't called yet.
>> This introduces oopses.
>> You are fixing this with patch 5/22 later, but patches should never
>> introduce any oopses.
>> The simplest soultion is to move patch 5/22 before this patch.
> After thinking for a while, the better to just fold patch 5 into patch 3,
> and do the necessary changes at the error handling logic.
>
> This makes it simpler to review and test. 
>
> New patch enclosed.
>
> Btw, I tested here with HVR-950, without any noticeable changes.
>
> Cheers,
> Mauro
>
> -
>
> [PATCH] [media] em28xx: move analog-specific init to em28xx-video
>
> There are several init code inside em28xx-cards that are actually
> part of analog initialization. Move the code to em28x-video, in
> order to remove part of the mess.
>
> In thesis, no functional changes so far.
>
> Signed-off-by: Mauro Carvalho Chehab 
>
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
> b/drivers/media/usb/em28xx/em28xx-cards.c
> index 154e6f028fd2..541de6df127b 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -2360,24 +2360,6 @@ static struct em28xx_hash_table em28xx_i2c_hash[] = {
>  };
>  /* NOTE: introduce a separate hash table for devices with 16 bit eeproms */
>  
> -/* I2C possible address to saa7115, tvp5150, msp3400, tvaudio */
> -static unsigned short saa711x_addrs[] = {
> - 0x4a >> 1, 0x48 >> 1,   /* SAA7111, SAA7111A and SAA7113 */
> - 0x42 >> 1, 0x40 >> 1,   /* SAA7114, SAA7115 and SAA7118 */
> - I2C_CLIENT_END };
> -
> -static unsigned short tvp5150_addrs[] = {
> - 0xb8 >> 1,
> - 0xba >> 1,
> - I2C_CLIENT_END
> -};
> -
> -static unsigned short msp3400_addrs[] = {
> - 0x80 >> 1,
> - 0x88 >> 1,
> - I2C_CLIENT_END
> -};
> -
>  int em28xx_tuner_callback(void *ptr, int component, int command, int arg)
>  {
>   struct em28xx_i2c_bus *i2c_bus = ptr;
> @@ -2782,58 +2764,8 @@ static void em28xx_card_setup(struct em28xx *dev)
>   /* Allow override tuner type by a module parameter */
>   if (tuner >= 0)
>   dev->tuner_type = tuner;
> -
> - /* request some modules */
> - if (dev->board.has_msp34xx)
> - v4l2_i2c_new_subdev(&dev->v4l2_dev, 
> &dev->i2c_adap[dev->def_i2c_bus],
> - "msp3400", 0, msp3400_addrs);
> -
> - if (dev->board.decoder == EM28XX_SAA711X)
> - v4l2_i2c_new_subdev(&dev->v4l2_dev, 
> &dev->i2c_adap[dev->def_i2c_bus],
> - "saa7115_auto", 0, saa711x_addrs);
> -
> - if (d

Re: How to enable "CONFIG_V4L2_MEM2MEM_DEV"

[Fabio Estevam]

On Mon, Jan 6, 2014 at 7:22 PM, m silverstri
 wrote:
> I have added 'CONFIG_V4L2_MEM2MEM_DEV=y'  to my 'imx_v6_v7_defconfig'
> and do a "make ARCH=arm CROSS_COMPILE=/usr/bin/arm-linux-gnueabi-
> imx_v6_v7_defconfig", I don't see CONFIG_V4L2_MEM2MEM_DEV being set to
> 'y' in .config that was automatically generated.
>
> I think I am making changes to the right 'imx_v6_v7_defconfig' file
> since when i add
> 'CONFIG_V4L_TEST_DRIVERS=y' to 'imx_v6_v7_defconfig', I see
> 'CONFIG_V4L_TEST_DRIVERS=y' in .config when I do "make ARCH=arm
> CROSS_COMPILE=/usr/bin/arm-linux-gnueabi- imx_v6_v7_defconfig"
>
> I am not sure why CONFIG_V4L2_MEM2MEM_DEV is not being set when I put
> 'CONFIG_V4L2_MEM2MEM_DEV=y'

Can you try the latest 3.13-rc7?

I just tried it here:

make imx_v6_v7_defconfig

Then I inspect the generated .config and CONFIG_V4L2_MEM2MEM_DEV=y

Regards,

Fabio Estevam
--
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 v4 04/22] [media] em28xx: make em28xx-video to be a separate module

[Frank Schäfer]

Am 05.01.2014 16:18, schrieb Mauro Carvalho Chehab:
> Em Sun, 05 Jan 2014 10:56:33 -0200
> Mauro Carvalho Chehab  escreveu:
>
>> Em Sun, 05 Jan 2014 11:47:00 +0100
>> Frank Schäfer  escreveu:
>>
>>> Am 04.01.2014 11:55, schrieb Mauro Carvalho Chehab:
 Now that all analog-specific code are at em28xx-video, convert
 it into an em28xx extension and load it as a separate module.

 Signed-off-by: Mauro Carvalho Chehab 
 ---
  drivers/media/usb/em28xx/Kconfig |  6 ++-
  drivers/media/usb/em28xx/Makefile|  5 ++-
  drivers/media/usb/em28xx/em28xx-camera.c |  1 +
  drivers/media/usb/em28xx/em28xx-cards.c  | 45 ---
  drivers/media/usb/em28xx/em28xx-core.c   | 12 ++
  drivers/media/usb/em28xx/em28xx-v4l.h| 20 +
  drivers/media/usb/em28xx/em28xx-vbi.c|  1 +
  drivers/media/usb/em28xx/em28xx-video.c  | 74 
 +++-
  drivers/media/usb/em28xx/em28xx.h| 23 ++
  9 files changed, 107 insertions(+), 80 deletions(-)
  create mode 100644 drivers/media/usb/em28xx/em28xx-v4l.h

 diff --git a/drivers/media/usb/em28xx/Kconfig 
 b/drivers/media/usb/em28xx/Kconfig
 index d6ba514d31eb..838fc9dbb747 100644
 --- a/drivers/media/usb/em28xx/Kconfig
 +++ b/drivers/media/usb/em28xx/Kconfig
 @@ -1,8 +1,12 @@
  config VIDEO_EM28XX
 -  tristate "Empia EM28xx USB video capture support"
 +  tristate "Empia EM28xx USB devices support"
depends on VIDEO_DEV && I2C
select VIDEO_TUNER
select VIDEO_TVEEPROM
 +
 +config VIDEO_EM28XX_V4L2
 +  tristate "Empia EM28xx analog TV, video capture and/or webcam support"
 +  depends on VIDEO_EM28XX && I2C
>>> VIDEO_EM28XX already depends on I2C.
>>>
select VIDEOBUF2_VMALLOC
select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
select VIDEO_TVP5150 if MEDIA_SUBDRV_AUTOSELECT
 diff --git a/drivers/media/usb/em28xx/Makefile 
 b/drivers/media/usb/em28xx/Makefile
 index ad6d48557940..3f850d5063d0 100644
 --- a/drivers/media/usb/em28xx/Makefile
 +++ b/drivers/media/usb/em28xx/Makefile
 @@ -1,10 +1,11 @@
 -em28xx-y +=   em28xx-video.o em28xx-i2c.o em28xx-cards.o
 -em28xx-y +=   em28xx-core.o  em28xx-vbi.o em28xx-camera.o
 +em28xx-y +=   em28xx-core.o em28xx-i2c.o em28xx-cards.o 
 em28xx-camera.o
  
 +em28xx-v4l-objs := em28xx-video.o em28xx-vbi.o
  em28xx-alsa-objs := em28xx-audio.o
  em28xx-rc-objs := em28xx-input.o
  
  obj-$(CONFIG_VIDEO_EM28XX) += em28xx.o
 +obj-$(CONFIG_VIDEO_EM28XX_V4L2) += em28xx-v4l.o
  obj-$(CONFIG_VIDEO_EM28XX_ALSA) += em28xx-alsa.o
  obj-$(CONFIG_VIDEO_EM28XX_DVB) += em28xx-dvb.o
  obj-$(CONFIG_VIDEO_EM28XX_RC) += em28xx-rc.o
 diff --git a/drivers/media/usb/em28xx/em28xx-camera.c 
 b/drivers/media/usb/em28xx/em28xx-camera.c
 index d666741797d4..c29f5c4e7b40 100644
 --- a/drivers/media/usb/em28xx/em28xx-camera.c
 +++ b/drivers/media/usb/em28xx/em28xx-camera.c
 @@ -454,3 +454,4 @@ int em28xx_init_camera(struct em28xx *dev)
  
return ret;
  }
 +EXPORT_SYMBOL_GPL(em28xx_init_camera);
>>> em28xx-camera should also be part of the em28xx-v4l module.
>> I tried that. Moving em28xx-camera to em28xx-v4l would cause a recursive
>> dependency, due to the camera probing logic, that should be called by
>> em28xx-cards.c.
>>
>> Moving that probing part to em28xx-v4l is too complex, due to the code
>> that detects the board.
>>
>> One solution would be to break em28xx-camera into two parts: the detection
>> code, to be merged with the core, and the sensor part, to be merged with
>> em28xx-v4l, but that sounds ugly.
>>
>> Other solution would be to use something like the dvb_attach() macro,
>> to avoid the recursive dependency, but that would be a dirty solution.
>>
>> As the code there is not big, the amount of overhead of having everything
>> at em28xx-core is not high. So, I opted to keep the simplest path here,
>> avoiding the risk of breaking something with a complex changeset.
>>
 diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
 b/drivers/media/usb/em28xx/em28xx-cards.c
 index 175cd776e0a1..938daaabd8e0 100644
 --- a/drivers/media/usb/em28xx/em28xx-cards.c
 +++ b/drivers/media/usb/em28xx/em28xx-cards.c
 @@ -2159,6 +2159,8 @@ struct em28xx_board em28xx_boards[] = {
.ir_codes  = RC_MAP_PINNACLE_PCTV_HD,
},
  };
 +EXPORT_SYMBOL_GPL(em28xx_boards);
 +
  const unsigned int em28xx_bcount = ARRAY_SIZE(em28xx_boards);
  
  /* table of devices that work with this driver */
 @@ -2827,10 +2829,6 @@ static void em28xx_card_setup(struct em28xx *dev)
"tuner", dev->tuner_addr, NULL);
}
}
 -
 -  em28xx_tuner_setup(dev);
 -
 -  em28xx_init_camera(dev);
  }
>>> Here you a

Re: How to enable "CONFIG_V4L2_MEM2MEM_DEV"

[m silverstri]

Thanks.  I try the latest (I clone linux from
https://github.com/torvalds/linux) and do 'make ARCH=arm
CROSS_COMPILE=/usr/bin/arm-linux-gnueabi- imx_v6_v7_defconfig' again,
I see

" CONFIG_V4L2_MEM2MEM_DEV=y" in the generated .config.

But when I try to add 'CONFIG_VIDEO_SAMSUNG_S5P_JPEG=y' to
imx_v6_v7_defconfig and re'make, I don't see
CONFIG_VIDEO_SAMSUNG_S5P_JPEG=y in the generated .config.

I have looked up Kconfig and find out the dependency is:

config VIDEO_SAMSUNG_S5P_JPEG
tristate "Samsung S5P/Exynos4 JPEG codec driver"
depends on VIDEO_DEV && VIDEO_V4L2 && PLAT_S5P
select VIDEOBUF2_DMA_CONTIG
select V4L2_MEM2MEM_DEV
---help---
 This is a v4l2 driver for Samsung S5P and EXYNOS4 JPEG codec

I have checked that both VIDEO_DEV && VIDEO_V4L2 are set, except
PLAT_S5P. I am not sure how to set 'PLAT_S5P'.

I appreciate if I can get help on this.

Thank you.


On Mon, Jan 6, 2014 at 1:30 PM, Fabio Estevam  wrote:
> On Mon, Jan 6, 2014 at 7:22 PM, m silverstri
>  wrote:
>> I have added 'CONFIG_V4L2_MEM2MEM_DEV=y'  to my 'imx_v6_v7_defconfig'
>> and do a "make ARCH=arm CROSS_COMPILE=/usr/bin/arm-linux-gnueabi-
>> imx_v6_v7_defconfig", I don't see CONFIG_V4L2_MEM2MEM_DEV being set to
>> 'y' in .config that was automatically generated.
>>
>> I think I am making changes to the right 'imx_v6_v7_defconfig' file
>> since when i add
>> 'CONFIG_V4L_TEST_DRIVERS=y' to 'imx_v6_v7_defconfig', I see
>> 'CONFIG_V4L_TEST_DRIVERS=y' in .config when I do "make ARCH=arm
>> CROSS_COMPILE=/usr/bin/arm-linux-gnueabi- imx_v6_v7_defconfig"
>>
>> I am not sure why CONFIG_V4L2_MEM2MEM_DEV is not being set when I put
>> 'CONFIG_V4L2_MEM2MEM_DEV=y'
>
> Can you try the latest 3.13-rc7?
>
> I just tried it here:
>
> make imx_v6_v7_defconfig
>
> Then I inspect the generated .config and CONFIG_V4L2_MEM2MEM_DEV=y
>
> Regards,
>
> Fabio Estevam
--
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 enable "CONFIG_V4L2_MEM2MEM_DEV"

[Fabio Estevam]

On Mon, Jan 6, 2014 at 9:42 PM, m silverstri
 wrote:
> Thanks.  I try the latest (I clone linux from
> https://github.com/torvalds/linux) and do 'make ARCH=arm
> CROSS_COMPILE=/usr/bin/arm-linux-gnueabi- imx_v6_v7_defconfig' again,
> I see
>
> " CONFIG_V4L2_MEM2MEM_DEV=y" in the generated .config.
>
> But when I try to add 'CONFIG_VIDEO_SAMSUNG_S5P_JPEG=y' to
> imx_v6_v7_defconfig and re'make, I don't see
> CONFIG_VIDEO_SAMSUNG_S5P_JPEG=y in the generated .config.

CONFIG_VIDEO_SAMSUNG_S5P_JPEG is to be used with Samsung SoC, not with
Freescale i.mx family.

Regards,

Fabio Estevam
--
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

FOSDEM14: Graphics DevRoom: Deadline approaching fast.

[Luc Verhaegen]

Hi,

There are still 5 slots open for the FOSDEM graphics DevRoom, and the 
deadline is this friday, the 10th. Get a move on.

If you have requested an account reset with me before, but if you then 
haven't bothered filing a talk, you do NOT have a slot. Please file a 
talk ASAP to still secure a place.

For more information on how to file for a devroom, read the email sent 
back in october: 
http://lists.x.org/archives/xorg-devel/2013-October/038185.html

Luc Verhaegen.
--
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

[Hans Verkuil]

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:   Tue Jan  7 04:00:27 CET 2014
git branch: test
git hash:   54a2a84ea9e8640b4f1df4e222e305d03bb64065
gcc version:i686-linux-gcc (GCC) 4.8.2
sparse version: 0.4.5-rc1
host hardware:  x86_64
host os:3.12-6.slh.2-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: ERRORS
linux-git-arm-exynos: WARNINGS
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.31.14-i686: WARNINGS
linux-2.6.32.27-i686: WARNINGS
linux-2.6.33.7-i686: WARNINGS
linux-2.6.34.7-i686: WARNINGS
linux-2.6.35.9-i686: WARNINGS
linux-2.6.36.4-i686: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12-i686: OK
linux-3.13-rc1-i686: OK
linux-2.6.31.14-x86_64: WARNINGS
linux-2.6.32.27-x86_64: WARNINGS
linux-2.6.33.7-x86_64: WARNINGS
linux-2.6.34.7-x86_64: WARNINGS
linux-2.6.35.9-x86_64: WARNINGS
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12-x86_64: WARNINGS
linux-3.13-rc1-x86_64: WARNINGS
apps: OK
spec-git: OK
sparse version: 0.4.5-rc1
sparse: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Tuesday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/4] [media] exynos-scaler: Add new driver for Exynos5 SCALER

[Shaik Ameer Basha]

Hi Mauro,

Thanks for the review comments.

On Fri, Jan 3, 2014 at 1:48 AM, Mauro Carvalho Chehab
 wrote:
> Em Fri,  4 Oct 2013 17:56:31 +0530
> Shaik Ameer Basha  escreveu:
>
>> This patch adds support for SCALER device which is a new device
>> for scaling, blending, color fill  and color space conversion
>> on EXYNOS5410 and EXYNOS5420 SoCs.
>>
>> This device supports the followings as key feature.
>> input image format
>> - YCbCr420 2P(UV/VU), 3P
>> - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P
>> - YCbCr444 2P(UV,VU), 3P
>> - RGB565, ARGB1555, ARGB, ARGB, RGBA
>> - Pre-multiplexed ARGB, L8A8 and L8
>> output image format
>> - YCbCr420 2P(UV/VU), 3P
>> - YCbCr422 1P(YUYV/UYVY/YVYU), 2P(UV,VU), 3P
>> - YCbCr444 2P(UV,VU), 3P
>> - RGB565, ARGB1555, ARGB, ARGB, RGBA
>> - Pre-multiplexed ARGB
>> input rotation
>> - 0/90/180/270 degree, X/Y/XY Flip
>> scale ratio
>> - 1/4 scale down to 16 scale up
>> color space conversion
>> - RGB to YUV / YUV to RGB
>> Size - Exynos5420
>> - Input : 16x16 to 8192x8192
>> - Output:   4x4 to 8192x8192
>> Size - Exynos5410
>> - Input/Output: 4x4 to 4096x4096
>> alpha blending, color fill
>>
>> Signed-off-by: Shaik Ameer Basha 
>> ---
>>  drivers/media/platform/exynos-scaler/scaler-regs.c |  336 
>> 
>>  drivers/media/platform/exynos-scaler/scaler-regs.h |  331 
>> +++
>>  2 files changed, 667 insertions(+)
>>  create mode 100644 drivers/media/platform/exynos-scaler/scaler-regs.c
>>  create mode 100644 drivers/media/platform/exynos-scaler/scaler-regs.h
>>
>> diff --git a/drivers/media/platform/exynos-scaler/scaler-regs.c 
>> b/drivers/media/platform/exynos-scaler/scaler-regs.c
>> new file mode 100644
>> index 000..ae4a548
>> --- /dev/null
>> +++ b/drivers/media/platform/exynos-scaler/scaler-regs.c
>> @@ -0,0 +1,336 @@
>> +/*
>> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
>> + *   http://www.samsung.com
>> + *
>> + * Samsung EXYNOS5 SoC series SCALER driver
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +
>> +#include "scaler-regs.h"
>> +
>> +/* Scaler reset timeout in milliseconds */
>> +#define SCALER_RESET_TIMEOUT 50
>> +
>> +void scaler_hw_set_sw_reset(struct scaler_dev *dev)
>> +{
>> + u32 cfg;
>> +
>> + cfg = scaler_read(dev, SCALER_CFG);
>> + cfg |= SCALER_CFG_SOFT_RESET;
>> +
>> + scaler_write(dev, SCALER_CFG, cfg);
>> +}
>> +
>> +int scaler_wait_reset(struct scaler_dev *dev)
>> +{
>> + unsigned long end = jiffies + msecs_to_jiffies(SCALER_RESET_TIMEOUT);
>> + u32 cfg, reset_done = 0;
>> +
>> + while (time_before(jiffies, end)) {
>> + cfg = scaler_read(dev, SCALER_CFG);
>> + if (!(cfg & SCALER_CFG_SOFT_RESET)) {
>> + reset_done = 1;
>> + break;
>> + }
>> + usleep_range(10, 20);
>
> Hmm... that doesn't seem right... the timeout can take up to 50,000 us, and
> you're sleeping from 10 to 20us... that means that this loop can have up to
> 5000 interactions... It seems that you're wasting power here without need.
>
> I suspect that you should consider sleeping for a longer time here.

I will check what is the average time taken for the reset process.
If it is much more than 20us then i will increase this sleeping time.

>
> Btw, instead of using time_before(jiffies, end), you could do:
> time_is_after_jiffies(end)
>

Ok. I will modify.

> As, from jiffies.h:
> /* time_is_after_jiffies(a) return true if a is after jiffies */
> #define time_is_after_jiffies(a) time_before(jiffies, a)
>
>> + }
>> +
>> + /*
>> +  * Write any value to read/write register and read it back.
>> +  * If the written and read value matches, then the reset process is
>> +  * succeeded.
>> +  */
>> + while (reset_done) {
>
> This is tricky. If the reset fail, it will return busy. Otherwise, it
> can loop forever here. Worse than that, you're don't even sleeping
> before retries, again wasting power.
>
> Why don't you just change it to something similar to:
>
> if (!reset_done)
> return -EBUSY;
>
> end = jiffies + msecs_to_jiffies(SCALER_RESET_TIMEOUT);
> while (time_is_after_jiffies(end)) {
> scaler_write(dev, SCALER_CFG_SOFT_RESET_CHECK_REG,
> SCALER_CFG_SOFT_RESET_CHECK_VAL);
> if (SCALER_CFG_SOFT_RESET_CHECK_VAL ==
> scaler_read(dev, SCALER_CFG_SOFT_RESET_CHECK_REG))
> return 0;
> msleep(10);
> }


My Intention is to comp

Re: [PATCH v4 2/4] [media] exynos-scaler: Add core functionality for the SCALER driver

[Shaik Ameer Basha]

Hi Mauro,

Thanks for the reveiw.

On Fri, Jan 3, 2014 at 1:55 AM, Mauro Carvalho Chehab
 wrote:
> Em Fri,  4 Oct 2013 17:56:32 +0530
> Shaik Ameer Basha  escreveu:
>
>> This patch adds the core functionality for the SCALER driver.
>>
>> Signed-off-by: Shaik Ameer Basha 
>> ---
>>  drivers/media/platform/exynos-scaler/scaler.c | 1238 
>> +
>>  drivers/media/platform/exynos-scaler/scaler.h |  375 
>>  2 files changed, 1613 insertions(+)
>>  create mode 100644 drivers/media/platform/exynos-scaler/scaler.c
>>  create mode 100644 drivers/media/platform/exynos-scaler/scaler.h
>>
>> diff --git a/drivers/media/platform/exynos-scaler/scaler.c 
>> b/drivers/media/platform/exynos-scaler/scaler.c
>> new file mode 100644
>> index 000..57635f2
>> --- /dev/null
>> +++ b/drivers/media/platform/exynos-scaler/scaler.c
>> @@ -0,0 +1,1238 @@
>> +/*
>> + * Copyright (c) 2013 Samsung Electronics Co., Ltd.
>> + *   http://www.samsung.com
>> + *
>> + * Samsung EXYNOS5 SoC series SCALER driver
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "scaler-regs.h"
>> +
>> +#define SCALER_CLOCK_GATE_NAME   "scaler"
>> +
>> +static const struct scaler_fmt scaler_formats[] = {
>> + {
>> + .name   = "YUV 4:2:0 non-contig. 2p, Y/CbCr",
>> + .pixelformat= V4L2_PIX_FMT_NV12M,
>> + .depth  = { 8, 4 },
>> + .color  = SCALER_YUV420,
>> + .color_order= SCALER_CBCR,
>> + .num_planes = 2,
>> + .num_comp   = 2,
>> + .scaler_color   = SCALER_YUV420_2P_Y_UV,
>> + .flags  = (SCALER_FMT_SRC | SCALER_FMT_DST),
>
> Not a big deal, but you don't need parenthesis for any of those .flags
> initialization.
>

Ok. I will fix this. and all other comments given by you in this patch series.

Regards,
Shaik Ameer Basha


[...] Snip

>
> --
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] au8028: Fix cleanup on kzalloc fail

[Tim Mester]

Free what was allocated if there is a failure allocating
transfer buffers.

Stop the feed on a start feed error.  The stop feed is not always called
if start feed fails.  If the feed is not stopped on error, then the driver
will be stuck so that it can never start feeding again.

Signed-off-by: Tim Mester 
---
 linux/drivers/media/usb/au0828/au0828-dvb.c | 70 +
 linux/drivers/media/usb/au0828/au0828.h |  2 +
 2 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/linux/drivers/media/usb/au0828/au0828-dvb.c 
b/linux/drivers/media/usb/au0828/au0828-dvb.c
index 9a6f156..2312381 100644
--- a/linux/drivers/media/usb/au0828/au0828-dvb.c
+++ b/linux/drivers/media/usb/au0828/au0828-dvb.c
@@ -153,9 +153,11 @@ static int stop_urb_transfer(struct au0828_dev *dev)
 
dev->urb_streaming = 0;
for (i = 0; i < URB_COUNT; i++) {
-   usb_kill_urb(dev->urbs[i]);
-   kfree(dev->urbs[i]->transfer_buffer);
-   usb_free_urb(dev->urbs[i]);
+   if (dev->urbs[i]) {
+   usb_kill_urb(dev->urbs[i]);
+   kfree(dev->urbs[i]->transfer_buffer);
+   usb_free_urb(dev->urbs[i]);
+   }
}
 
return 0;
@@ -185,6 +187,8 @@ static int start_urb_transfer(struct au0828_dev *dev)
if (!purb->transfer_buffer) {
usb_free_urb(purb);
dev->urbs[i] = NULL;
+   printk(KERN_ERR "%s: failed big buffer allocation, "
+  "err = %d\n", __func__, ret);
goto err;
}
 
@@ -217,6 +221,27 @@ err:
return ret;
 }
 
+static void au0828_start_transport(struct au0828_dev *dev)
+{
+   au0828_write(dev, 0x608, 0x90);
+   au0828_write(dev, 0x609, 0x72);
+   au0828_write(dev, 0x60a, 0x71);
+   au0828_write(dev, 0x60b, 0x01);
+
+}
+
+static void au0828_stop_transport(struct au0828_dev *dev, int full_stop)
+{
+   if (full_stop) {
+   au0828_write(dev, 0x608, 0x00);
+   au0828_write(dev, 0x609, 0x00);
+   au0828_write(dev, 0x60a, 0x00);
+   }
+   au0828_write(dev, 0x60b, 0x00);
+}
+
+
+
 static int au0828_dvb_start_feed(struct dvb_demux_feed *feed)
 {
struct dvb_demux *demux = feed->demux;
@@ -231,13 +256,17 @@ static int au0828_dvb_start_feed(struct dvb_demux_feed 
*feed)
 
if (dvb) {
mutex_lock(&dvb->lock);
+   dvb->start_count++;
+   dprintk(1, "%s(), start_count: %d, stop_count: %d\n", __func__,
+   dvb->start_count, dvb->stop_count);
if (dvb->feeding++ == 0) {
/* Start transport */
-   au0828_write(dev, 0x608, 0x90);
-   au0828_write(dev, 0x609, 0x72);
-   au0828_write(dev, 0x60a, 0x71);
-   au0828_write(dev, 0x60b, 0x01);
+   au0828_start_transport(dev);
ret = start_urb_transfer(dev);
+   if (ret < 0) {
+   au0828_stop_transport(dev, 0);
+   dvb->feeding--; /* We ran out of memory... */
+   }
}
mutex_unlock(&dvb->lock);
}
@@ -256,10 +285,16 @@ static int au0828_dvb_stop_feed(struct dvb_demux_feed 
*feed)
 
if (dvb) {
mutex_lock(&dvb->lock);
-   if (--dvb->feeding == 0) {
-   /* Stop transport */
-   ret = stop_urb_transfer(dev);
-   au0828_write(dev, 0x60b, 0x00);
+   dvb->stop_count++;
+   dprintk(1, "%s(), start_count: %d, stop_count: %d\n", __func__,
+   dvb->start_count, dvb->stop_count);
+   if (dvb->feeding > 0) {
+   dvb->feeding--;
+   if (dvb->feeding == 0) {
+   /* Stop transport */
+   ret = stop_urb_transfer(dev);
+   au0828_stop_transport(dev, 0);
+   }
}
mutex_unlock(&dvb->lock);
}
@@ -282,16 +317,10 @@ static void au0828_restart_dvb_streaming(struct 
work_struct *work)
 
/* Stop transport */
stop_urb_transfer(dev);
-   au0828_write(dev, 0x608, 0x00);
-   au0828_write(dev, 0x609, 0x00);
-   au0828_write(dev, 0x60a, 0x00);
-   au0828_write(dev, 0x60b, 0x00);
+   au0828_stop_transport(dev, 1);
 
/* Start transport */
-   au0828_write(dev, 0x608, 0x90);
-   au0828_write(dev, 0x609, 0x72);
-   au0828_write(dev, 0x60a, 0x71);
-   au0828_write(dev, 0x60b, 0x01);
+   au0828_start_transport(dev);
start_urb_transfer(dev);
 
mutex_unlock(&dvb->lock);
@@ -375,6 +404,9 @@ static int dvb_r

[PATCH 2/3] au0828: Add option to preallocate digital transfer buffers

[Tim Mester]

Added command line parameter preallocate_big_buffers so that the digital
transfer buffers can be allocated when the driver is registered. They
do not have to be allocated every time a feed is started.

Signed-off-by: Tim Mester 
---
 linux/drivers/media/usb/au0828/au0828-core.c | 13 +---
 linux/drivers/media/usb/au0828/au0828-dvb.c  | 46 ++--
 linux/drivers/media/usb/au0828/au0828.h  |  4 +++
 3 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/linux/drivers/media/usb/au0828/au0828-core.c 
b/linux/drivers/media/usb/au0828/au0828-core.c
index bd9d19a..ab45a6f 100644
--- a/linux/drivers/media/usb/au0828/au0828-core.c
+++ b/linux/drivers/media/usb/au0828/au0828-core.c
@@ -173,9 +173,8 @@ static int au0828_usb_probe(struct usb_interface *interface,
const struct usb_device_id *id)
 {
int ifnum;
-#ifdef CONFIG_VIDEO_AU0828_V4L2
-   int retval;
-#endif
+   int retval = 0;
+
struct au0828_dev *dev;
struct usb_device *usbdev = interface_to_usbdev(interface);
 
@@ -257,7 +256,11 @@ static int au0828_usb_probe(struct usb_interface 
*interface,
 #endif
 
/* Digital TV */
-   au0828_dvb_register(dev);
+   retval = au0828_dvb_register(dev);
+   if (retval)
+   pr_err("%s() au0282_dev_register failed\n",
+  __func__);
+
 
/* Store the pointer to the au0828_dev so it can be accessed in
   au0828_usb_disconnect */
@@ -268,7 +271,7 @@ static int au0828_usb_probe(struct usb_interface *interface,
 
mutex_unlock(&dev->lock);
 
-   return 0;
+   return retval;
 }
 
 static struct usb_driver au0828_usb_driver = {
diff --git a/linux/drivers/media/usb/au0828/au0828-dvb.c 
b/linux/drivers/media/usb/au0828/au0828-dvb.c
index 2312381..1673c88 100644
--- a/linux/drivers/media/usb/au0828/au0828-dvb.c
+++ b/linux/drivers/media/usb/au0828/au0828-dvb.c
@@ -33,6 +33,10 @@
 #include "mxl5007t.h"
 #include "tda18271.h"
 
+int preallocate_big_buffers;
+module_param_named(preallocate_big_buffers, preallocate_big_buffers, int, 
0644);
+MODULE_PARM_DESC(preallocate_big_buffers, "Preallocate the larger transfer 
buffers at module load time");
+
 DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
 
 #define _AU0828_BULKPIPE 0x83
@@ -155,7 +159,9 @@ static int stop_urb_transfer(struct au0828_dev *dev)
for (i = 0; i < URB_COUNT; i++) {
if (dev->urbs[i]) {
usb_kill_urb(dev->urbs[i]);
-   kfree(dev->urbs[i]->transfer_buffer);
+   if (!preallocate_big_buffers)
+   kfree(dev->urbs[i]->transfer_buffer);
+
usb_free_urb(dev->urbs[i]);
}
}
@@ -183,7 +189,12 @@ static int start_urb_transfer(struct au0828_dev *dev)
 
purb = dev->urbs[i];
 
-   purb->transfer_buffer = kzalloc(URB_BUFSIZE, GFP_KERNEL);
+   if (preallocate_big_buffers)
+   purb->transfer_buffer = dev->dig_transfer_buffer[i];
+   else
+   purb->transfer_buffer = kzalloc(URB_BUFSIZE,
+   GFP_KERNEL);
+
if (!purb->transfer_buffer) {
usb_free_urb(purb);
dev->urbs[i] = NULL;
@@ -333,6 +344,22 @@ static int dvb_register(struct au0828_dev *dev)
 
dprintk(1, "%s()\n", __func__);
 
+   if (preallocate_big_buffers) {
+   int i;
+   for (i = 0; i < URB_COUNT; i++) {
+   dev->dig_transfer_buffer[i] = kzalloc(URB_BUFSIZE,
+   GFP_KERNEL);
+
+   if (!dev->dig_transfer_buffer[i]) {
+   result = -ENOMEM;
+
+   printk(KERN_ERR "%s: failed buffer allocation"
+  "(errno = %d)\n", DRIVER_NAME, result);
+   goto fail_adapter;
+   }
+   }
+   }
+
INIT_WORK(&dev->restart_streaming, au0828_restart_dvb_streaming);
 
/* register adapter */
@@ -423,6 +450,13 @@ fail_frontend:
dvb_frontend_detach(dvb->frontend);
dvb_unregister_adapter(&dvb->adapter);
 fail_adapter:
+
+   if (preallocate_big_buffers) {
+   int i;
+   for (i = 0; i < URB_COUNT; i++)
+   kfree(dev->dig_transfer_buffer[i]);
+   }
+
return result;
 }
 
@@ -443,6 +477,14 @@ void au0828_dvb_unregister(struct au0828_dev *dev)
dvb_unregister_frontend(dvb->frontend);
dvb_frontend_detach(dvb->frontend);
dvb_unregister_adapter(&dvb->adapter);
+
+   if (preallocate_big_buffers) {
+   int i;
+   for (i = 0; i < URB_COUNT; i++)
+   kfree(dev->dig_transfer_buffer[i]);
+   }
+
+
 }
 
 /* All the DVB attach calls go here, this function get'

[PATCH 3/3] au8522, au0828: Added demodulator reset

[Tim Mester]

The demodulator can get in a state in ATSC mode where just
restarting the feed alone does not correct the corrupted stream.  The
demodulator reset in addition to the feed restart seems to correct
the condition.

The au8522 driver has been modified so that when set_frontend() is
called with the same frequency and modulation mode, the demodulator
will be reset.  The au0282 drives uses this feature when it attempts
to restart the feed.

Signed-off-by: Tim Mester 
---
 linux/drivers/media/dvb-frontends/au8522_dig.c | 38 +-
 linux/drivers/media/usb/au0828/au0828-dvb.c|  8 ++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/linux/drivers/media/dvb-frontends/au8522_dig.c 
b/linux/drivers/media/dvb-frontends/au8522_dig.c
index a68974f..821cc70 100644
--- a/linux/drivers/media/dvb-frontends/au8522_dig.c
+++ b/linux/drivers/media/dvb-frontends/au8522_dig.c
@@ -469,6 +469,38 @@ static struct {
{ 0x8526, 0x01 },
 };
 
+/*
+ * Reset the demodulator.  Currently this only does something if
+ * configured in ATSC mode.  This reset is needed in marginal signal
+ * levels when the feed (in MPEG-TS format) is correupted.
+ *
+ */
+
+static int au8522_reset_demodulator(struct dvb_frontend *fe)
+{
+   struct au8522_state *state = fe->demodulator_priv;
+
+   switch (state->current_modulation) {
+   case VSB_8:
+   dprintk("%s() VSB_8\n", __func__);
+   au8522_writereg(state, 0x80a4, 0);
+   au8522_writereg(state, 0x80a5, 0);
+   au8522_writereg(state, 0x80a4, 0xe8);
+   au8522_writereg(state, 0x80a5, 0x40);
+   break;
+   case QAM_64:
+   case QAM_256:
+   dprintk("%s() QAM 64/256\n", __func__);
+   break;
+   default:
+   dprintk("%s() Invalid modulation\n", __func__);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
+
 static int au8522_enable_modulation(struct dvb_frontend *fe,
fe_modulation_t m)
 {
@@ -522,8 +554,12 @@ static int au8522_set_frontend(struct dvb_frontend *fe)
dprintk("%s(frequency=%d)\n", __func__, c->frequency);
 
if ((state->current_frequency == c->frequency) &&
-   (state->current_modulation == c->modulation))
+   (state->current_modulation == c->modulation)) {
+
+   au8522_reset_demodulator(fe);
+   msleep(100);
return 0;
+   }
 
if (fe->ops.tuner_ops.set_params) {
if (fe->ops.i2c_gate_ctrl)
diff --git a/linux/drivers/media/usb/au0828/au0828-dvb.c 
b/linux/drivers/media/usb/au0828/au0828-dvb.c
index 1673c88..483c587 100644
--- a/linux/drivers/media/usb/au0828/au0828-dvb.c
+++ b/linux/drivers/media/usb/au0828/au0828-dvb.c
@@ -330,6 +330,14 @@ static void au0828_restart_dvb_streaming(struct 
work_struct *work)
stop_urb_transfer(dev);
au0828_stop_transport(dev, 1);
 
+   /*
+* In ATSC mode, we also need to reset the demodulator in lower signal
+* level cases to help realign the data stream.
+*/
+
+   if (dvb->frontend->ops.set_frontend)
+   dvb->frontend->ops.set_frontend(dvb->frontend);
+
/* Start transport */
au0828_start_transport(dev);
start_urb_transfer(dev);
-- 
1.8.1.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 3/3] au8522, au0828: Added demodulator reset

[Devin Heitmueller]

On Mon, Jan 6, 2014 at 11:29 PM, Tim Mester  wrote:
> The demodulator can get in a state in ATSC mode where just
> restarting the feed alone does not correct the corrupted stream.  The
> demodulator reset in addition to the feed restart seems to correct
> the condition.
>
> The au8522 driver has been modified so that when set_frontend() is
> called with the same frequency and modulation mode, the demodulator
> will be reset.  The au0282 drives uses this feature when it attempts
> to restart the feed.

What is the actual "corruption" that you are seeing?  Can you describe
it in greater detail?  The original fix was specifically related to
the internal FIFO on the au0828 where it can get shifted by one or
more bits (i.e. the leading byte is no longer 0x47 but 0x47 << X).
Hence it's an issue unrelated to the actual au8522.

I suspect this is actually a different problem which out of dumb luck
gets "fixed" by resetting the chip.  Without more details on the
specific behavior you are seeing though I cannot really advise on what
the correct change is.

This patch should not be accepted upstream without more discussion.

Regards,

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] au8522, au0828: Added demodulator reset

[Devin Heitmueller]

> I suspect this is actually a different problem which out of dumb luck
> gets "fixed" by resetting the chip.  Without more details on the
> specific behavior you are seeing though I cannot really advise on what
> the correct change is.

Tim,

It might be worth trying out the following patch series and see if it
addresses the problem you're seeing.  There was a host of problems
with the clock management on the device which could result in the
various sub-blocks getting wedged.  The TS output block was just one
of those cases.

http://git.kernellabs.com/?p=dheitmueller/linuxtv.git;a=shortlog;h=refs/heads/950q_improv

I'm not against the hack you've proposed if it's really warranted, but
a reset is really a last resort and I'm very concerned it's masking
over the real problem.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com
--
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

Initial scan table for au-Melbourne-Selby

[Philip Yarra]

Hi, please find attached a scan table for au-Melbourne-Selby. This file 
is very similar to the scan table file for au-Melbourne-Upwey (which I 
was able to use until quite recently). However the fec_hi value of "2/3" 
for SBS no longer works for me, and I need to use "AUTO" instead. I 
don't know if this change also affects the Upwey repeater.


Details on the geographic locations of these repeaters can be found here:
Upwey: http://www20.sbs.com.au/transmissions/index.php?pid=2&id=795
Selby: http://www20.sbs.com.au/transmissions/index.php?pid=2&id=792

Note that the Selby repeater actually covers the parts of Upwey which 
are not able to get signal from the Upwey repeater, due to hilly 
terrain. Although they use identical frequencies, the polarisation is 
different.


I assume AUTO allows the DVB tuner to choose one of the FEC types 
dynamically, though I don't know if this is supported by all tuners. If 
there's a way I can find out which actual fec_hi is in use, please let 
me know and I will supply it.


I have provided a brief write-up at 
http://pyarra.blogspot.com.au/2014/01/mythtv-and-sbs-in-dandenong-ranges.html 
- please let me know if there is further information I can provide.


Regards,
Philip.
# Australia / Melbourne (Selby Repeater)
# T freq bw fec_hi fec_lo mod transmission-mode guard-interval hierarchy
# ABC
T 66250 7MHz 3/4 NONE QAM64 8k 1/16 NONE
# Seven
T 62050 7MHz 3/4 NONE QAM64 8k 1/16 NONE
# Nine
T 64150 7MHz 3/4 NONE QAM64 8k 1/16 NONE
# Ten
T 71150 7MHz 3/4 NONE QAM64 8k 1/16 NONE
# SBS
T 68350 7MHz AUTO NONE QAM64 8k 1/8 NONE

media: i2c: add new dual LED Flash driver, lm3646.

[Daniel Jeong]

 This patch adds the driver for the LM3646, dual LED Flash driver.
The LM3646 has two 1.5A sync. boost converter with dual white current source.
It is controlled via an I2C compatible interface.
Each flash brightness, torch brightness and enable/disable can be controlled.
UVLO, IVFM and NTC threshhold Faults are added.
Please refer the datasheet http://www.ti.com/lit/ds/snvs962/snvs962.pdf

Signed-off-by: Daniel Jeong 
---
 drivers/media/i2c/Kconfig  |9 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/lm3646.c |  410 
 include/media/lm3646.h |   87 
 include/uapi/linux/v4l2-controls.h |3 +
 5 files changed, 510 insertions(+)
 create mode 100644 drivers/media/i2c/lm3646.c
 create mode 100644 include/media/lm3646.h

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 842654d..654df46 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -630,6 +630,15 @@ config VIDEO_LM3560
  This is a driver for the lm3560 dual flash controllers. It controls
  flash, torch LEDs.
 
+config VIDEO_LM3646
+   tristate "LM3646 dual flash driver support"
+   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
+   depends on MEDIA_CAMERA_SUPPORT
+   select REGMAP_I2C
+   ---help---
+ This is a driver for the lm3646 dual flash controllers. It controls
+ flash, torch LEDs.
+
 comment "Video improvement chips"
 
 config VIDEO_UPD64031A
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index e03f177..a52cda6 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_VIDEO_S5C73M3)   += s5c73m3/
 obj-$(CONFIG_VIDEO_ADP1653)+= adp1653.o
 obj-$(CONFIG_VIDEO_AS3645A)+= as3645a.o
 obj-$(CONFIG_VIDEO_LM3560) += lm3560.o
+obj-$(CONFIG_VIDEO_LM3646) += lm3646.o
 obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o
 obj-$(CONFIG_VIDEO_AK881X) += ak881x.o
 obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
diff --git a/drivers/media/i2c/lm3646.c b/drivers/media/i2c/lm3646.c
new file mode 100644
index 000..f6d8b6d
--- /dev/null
+++ b/drivers/media/i2c/lm3646.c
@@ -0,0 +1,410 @@
+/*
+ * drivers/media/i2c/lm3646.c
+ * General device driver for TI lm3646, Dual FLASH LED Driver
+ *
+ * Copyright (C) 2014 Texas Instruments
+ *
+ * Contact: Daniel Jeong 
+ * Ldd-Mlp 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* registers definitions */
+#define REG_ENABLE 0x01
+#define REG_TORCH_BR   0x05
+#define REG_FLASH_BR   0x05
+#define REG_FLASH_TOUT 0x04
+#define REG_FLAG   0x08
+#define REG_STROBE_SRC 0x06
+#define REG_LED1_FLASH_BR 0x06
+#define REG_LED1_TORCH_BR 0x07
+
+#define MASK_ENABLE0x03
+#define MASK_TORCH_BR  0x70
+#define MASK_FLASH_BR  0x0F
+#define MASK_FLASH_TOUT0x07
+#define MASK_FLAG  0xFF
+#define MASK_STROBE_SRC0x80
+
+/* Fault Mask */
+#define FAULT_TIMEOUT  (1<<0)
+#define FAULT_SHORT_CIRCUIT(1<<1)
+#define FAULT_UVLO (1<<2)
+#define FAULT_IVFM (1<<3)
+#define FAULT_OCP  (1<<4)
+#define FAULT_OVERTEMP (1<<5)
+#define FAULT_NTC_TRIP (1<<6)
+#define FAULT_OVP  (1<<7)
+
+enum led_mode {
+   MODE_SHDN = 0x0,
+   MODE_TORCH = 0x2,
+   MODE_FLASH = 0x3,
+};
+
+/* struct lm3646_flash
+ *
+ * @pdata: platform data
+ * @regmap: reg. map for i2c
+ * @lock: muxtex for serial access.
+ * @led_mode: V4L2 LED mode
+ * @ctrls_led: V4L2 contols
+ * @subdev_led: V4L2 subdev
+ */
+struct lm3646_flash {
+   struct device *dev;
+   struct lm3646_platform_data *pdata;
+   struct regmap *regmap;
+   struct mutex lock;
+
+   enum v4l2_flash_led_mode led_mode;
+   struct v4l2_ctrl_handler ctrls_led;
+   struct v4l2_subdev subdev_led;
+};
+
+#define to_lm3646_flash(_ctrl) \
+   container_of(_ctrl->handler, struct lm3646_flash, ctrls_led)
+
+/* enable mode control */
+static int lm3646_mode_ctrl(struct lm3646_flash *flash)
+{
+   int rval = -EINVAL;
+
+   switch (flash->led_mode) {
+   case V4L2_FLASH_LED_MODE_NONE:
+   rval = regmap_update_bits(flash->regmap,
+ REG_ENABLE, MASK_ENABLE, MODE_SHDN);
+   break;
+   case V4L2_FLASH_LED_MODE_TORCH:
+   rval = regmap_update_bits(flash->regmap,
+ REG_ENABLE, MASK_ENABLE, MODE_TORCH);
+   break;
+   case V4L2_FLASH_LED_MODE_FLASH:
+   rval = regmap_update_bits(flash->regmap,
+ REG_ENABLE, MASK_ENABLE, MODE_FLASH);
+