Re: [PATCH] rtl28xxu: fix control message flaws

2015-10-14 Thread Antti Palosaari

On 10/15/2015 01:11 AM, Daniel Glöckner wrote:

On Sat, Oct 10, 2015 at 07:45:30PM +0300, Antti Palosaari wrote:

Add lock to prevent concurrent access for control message as control
message function uses shared buffer. Without the lock there may be
remote control polling which messes the buffer causing IO errors.


This patch fixes the Problems I had with my Astrometa stick's I2C bus
locking up at the end of each dvbv5-scan run until it is disconnected.
There is another source of IO errors in the current driver, though.
The delayed work closing the I2C gate to the tuner is often executed
after rtl2832_power_ctrl has disabled the PLL. This will cause the
USB transfer accessing the gate control register to fail with -EPIPE.


I saw that few times too, but it does not cause any other harm than 
error printing. It went away when canceled that delayed gate closing 
timer during demod sleep. But that was device which doesn't have slave 
demod at all, so it does not apply to your case as integrated demod 
sleep is not called at all. I think some callback which does opposite 
than "enable_slave_ts()" is needed. Like "disable_slave_ts()" which 
kills that timer before demod is powered off.


regards
Antti

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


[PATCH v4 69/79] include/uapi/linux/dvb/video.h: remove stdint.h include

2015-10-14 Thread Mikko Rapeli
Kernel headers should use linux/types.h instead.

Signed-off-by: Mikko Rapeli 
---
 include/uapi/linux/dvb/video.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/uapi/linux/dvb/video.h b/include/uapi/linux/dvb/video.h
index d3d14a59..4939256 100644
--- a/include/uapi/linux/dvb/video.h
+++ b/include/uapi/linux/dvb/video.h
@@ -26,7 +26,6 @@
 
 #include 
 #ifndef __KERNEL__
-#include 
 #include 
 #endif
 
-- 
2.5.0

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


cron job: media_tree daily build: OK

2015-10-14 Thread 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:   Thu Oct 15 04:00:18 CEST 2015
git branch: test
git hash:   efe98010b80ec4516b2779e1b4e4a8ce16bf89fe
gcc version:i686-linux-gcc (GCC) 5.1.0
sparse version: v0.5.0-51-ga53cea2
smatch version: 0.4.1-3153-g7d56ab3
host hardware:  x86_64
host os:4.0.0-3.slh.1-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: 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.32.27-i686: OK
linux-2.6.33.7-i686: OK
linux-2.6.34.7-i686: OK
linux-2.6.35.9-i686: OK
linux-2.6.36.4-i686: OK
linux-2.6.37.6-i686: OK
linux-2.6.38.8-i686: OK
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: OK
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-4.3-rc1-i686: OK
linux-2.6.32.27-x86_64: OK
linux-2.6.33.7-x86_64: OK
linux-2.6.34.7-x86_64: OK
linux-2.6.35.9-x86_64: OK
linux-2.6.36.4-x86_64: OK
linux-2.6.37.6-x86_64: OK
linux-2.6.38.8-x86_64: OK
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: OK
linux-3.5.7-x86_64: OK
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
linux-4.3-rc1-x86_64: OK
apps: OK
spec-git: OK
sparse: WARNINGS
smatch: ERRORS

Detailed results are available here:

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

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Thursday.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] rtl28xxu: fix control message flaws

2015-10-14 Thread Daniel Glöckner
On Sat, Oct 10, 2015 at 07:45:30PM +0300, Antti Palosaari wrote:
> Add lock to prevent concurrent access for control message as control
> message function uses shared buffer. Without the lock there may be
> remote control polling which messes the buffer causing IO errors.

This patch fixes the Problems I had with my Astrometa stick's I2C bus
locking up at the end of each dvbv5-scan run until it is disconnected.
There is another source of IO errors in the current driver, though.
The delayed work closing the I2C gate to the tuner is often executed
after rtl2832_power_ctrl has disabled the PLL. This will cause the
USB transfer accessing the gate control register to fail with -EPIPE.

Best regards,

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


MC Next Gen au0828_usb_disconnect() softlockups

2015-10-14 Thread Shuah Khan
I am seeing softlockups from au0828_usb_disconnect().
I am looking into this problem. Reporting just in case
you saw this during your testing and problem is fixed.
I am doing USB device removal test when I ran into this.
Could this be an issue with the use of spin_lock() - should
spin_lock_irq() a better choice in media_device_unregister_entity()

Here is the dmesg snippet:

[ 1316.127004] R13: 810d0a2e R14: 818005a1 R15:
810d0a2e
[ 1316.127008] FS:  7f47d8fd9700() GS:88023ec8()
knlGS:
[ 1316.127012] CS:  0010 DS:  ES:  CR0: 8005003b
[ 1316.127015] CR2: 7fff1917afa8 CR3: 000222b4b000 CR4:
000407e0
[ 1316.127017] Stack:
[ 1316.127020]  88023407b958 813bf83f 88023407b988
810b4d4c
[ 1316.127027]  880223329180 880223329180 880223385d58
880223385d78
[ 1316.127034]  88023407b9b8 817fefa9 8161199b
81611b18
[ 1316.127040] Call Trace:
[ 1316.127049]  [] __delay+0xf/0x20
[ 1316.127056]  [] do_raw_spin_lock+0x8c/0x120
[ 1316.127062]  [] _raw_spin_lock+0x39/0x40
[ 1316.127070]  [] ?
media_device_unregister_entity+0x3b/0x130
[ 1316.127075]  [] ? media_device_unregister+0x88/0x150
[ 1316.127081]  []
media_device_unregister_entity+0x3b/0x130
[ 1316.127087]  [] media_device_unregister+0xbe/0x150
[ 1316.127097]  []
au0828_unregister_media_device+0x45/0x60 [au0828]
[ 1316.127105]  [] au0828_usb_v4l2_release+0x6a/0x90
[au0828]
[ 1316.127111]  [] v4l2_device_release+0x1e/0x30
[ 1316.127116]  [] v4l2_device_put+0x25/0x30
[ 1316.127123]  [] au0828_usb_disconnect+0xb1/0xd0
[au0828]
[ 1316.127129]  [] usb_unbind_interface+0x86/0x280
[ 1316.127135]  [] ? trace_hardirqs_on+0xd/0x10
[ 1316.127141]  [] __device_release_driver+0x96/0x130
[ 1316.127146]  [] device_release_driver+0x25/0x40
[ 1316.127150]  [] bus_remove_device+0x11c/0x1a0
[ 1316.127157]  [] device_del+0x139/0x250
[ 1316.127162]  [] ? remove_intf_ep_devs+0x41/0x60
[ 1316.127168]  [] usb_disable_device+0x89/0x280
[ 1316.127173]  [] usb_disconnect+0x96/0x2b0
[ 1316.127178]  [] hub_event+0x696/0x15a0
[ 1316.127185]  [] process_one_work+0x1c0/0x4b0
[ 1316.127191]  [] ? process_one_work+0x154/0x4b0
[ 1316.127196]  [] worker_thread+0x4b/0x440
[ 1316.127202]  [] ? rescuer_thread+0x2e0/0x2e0
[ 1316.127207]  [] ? rescuer_thread+0x2e0/0x2e0
[ 1316.127212]  [] kthread+0xe4/0x100
[ 1316.127219]  [] ? kthread_create_on_node+0x220/0x220
[ 1316.127224]  [] ret_from_fork+0x3f/0x70
[ 1316.127229]  [] ? kthread_create_on_node+0x220/0x220
[ 1316.127232] Code: c3 0f 1f 40 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f
44 00 00 55 48 89 e5 65 8b 35 80 a8 c4 7e 0f ae f0 0f 31 89 c1 0f ae f0
0f 31 <48> c1 e2 20 89 c0 48 09 c2 89 d0 29 ca 39 fa 73 1c f3 90 65 8b
[ 1323.553480] device: '0:40': device_add
[ 1323.553532] PM: Adding info for No Bus:0:40

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shua...@osg.samsung.com | (970) 217-8978
--
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: v4l2-ctrl is unable to set autogain to 0 with gspca/ov534

2015-10-14 Thread Antonio Ospite
On Mon, 12 Oct 2015 09:56:08 +0200
Hans Verkuil  wrote:

> Hi Antonio,
> 
> On 10/07/2015 10:05 AM, Antonio Ospite wrote:
[...]
> > After a little investigation I figured out some more details: in my use
> > case the autogain is a master control in an auto cluster, and switching
> > it from auto to manual does not work when using VIDIOC_S_CTRL i.e. when
> > calling set_ctrl().
> > 
> > It works with qv4l2 because it uses VIDIOC_S_EXT_CTRLS.
> > 
> > So the difference is between v4l2-ctrls.c::v4l2_s_ctrl() and
> > v4l2-ctrls.c::v4l2_s_ext_ctrls().
> > 
> > Wrt. auto clusters going from auto to manual the two functions do
> > basically this:
> > 
> > 
> >   v4l2_s_ctrl()
> > set_ctrl_lock()
> >   user_to_new()
> >   set_ctrl()
> > update_from_auto_cluster(master)
> > try_or_set_cluster()
> >   cur_to_user()
> > 
> > 
> >   v4l2_s_ext_ctrls()
> > try_set_ext_ctrls()
> >   update_from_auto_cluster(master)
> >   user_to_new() for each control
> >   try_or_set_cluster()
> >   new_to_user()
> > 
> > 
> > I think the problem is that when update_from_auto_cluster(master) is
> > called it overrides the new master control value from userspace by
> > calling cur_to_new(). This also happens when calling VIDIOC_S_EXT_CTRLS
> > (in try_set_ext_ctrls), but in that case, AFTER the call to
> > update_from_auto_cluster(master), the code calls user_to_new() that sets
> > back again the correct new value in the control before making the value
> > permanent with try_or_set_cluster().
> > 
> > The regression may have been introduced in
> > 5d0360a4f027576e5419d4a7c711c9ca0f1be8ca, in fact by just reverting
> > these two interdependent commits:
> > 
> > 7a7f1ab37dc8f66cf0ef10f3d3f1b79ac4bc67fc
> > 5d0360a4f027576e5419d4a7c711c9ca0f1be8ca
> > 
> > the problem goes away, so the regression is about user_to_new() not
> > being called AFTER update_from_auto_cluster(master) anymore in
> > set_ctrl(), as per 5d0360a4f027576e5419d4a7c711c9ca0f1be8ca.
> 
> Excellent analysis!
>

I think we can make the code paths of v4l2_s_ctrl() and
v4l2_s_ext_ctrls() more look alike to ease similar investigations.

In v4l2_s_ext_ctrls():

  1. Call user_to_new() before update_from_auto_cluster(master), I am
 still not 100% sure if this can introduce regressions.

  2. Use cur_to_user() instead of new_to_user(), to match the code path
 of v4l2_s_ctrl(), the effect of using either one or the other
 should be equivalent right _after_ a call to try_or_set_cluster(),
 shouldn't it?

I'll try to test these changes as time permits, but if anyone can
squeeze that in their paid time feel free to anticipate me.

[...] 
> Thanks for looking at this!
>

FWIW I got interested by this in particular:

  $ strace v4l2-ctl --set-ctrl=gain_automatic=0
  ...
  ioctl(3, VIDIOC_S_CTRL, {id=V4L2_CID_AUTOGAIN, value=1}) = 0
   ^

Ciao ciao,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] [media] media/v4l2-ctrls: fix setting autocluster to manual with VIDIOC_S_CTRL

2015-10-14 Thread Antonio Ospite
Since commit 5d0360a4f027576e5419d4a7c711c9ca0f1be8ca it's not possible
anymore to set auto clusters from auto to manual using VIDIOC_S_CTRL.

For example, setting autogain to manual with gspca/ov534 driver and this
sequence of commands does not work:

  v4l2-ctl --set-ctrl=gain_automatic=1
  v4l2-ctl --list-ctrls | grep gain_automatic
  # The following does not work
  v4l2-ctl --set-ctrl=gain_automatic=0
  v4l2-ctl --list-ctrls | grep gain_automatic

Changing the value using VIDIOC_S_EXT_CTRLS (like qv4l2 does) works
fine.

The apparent cause by looking at the changes in 5d0360a and comparing
with the code path for VIDIOC_S_EXT_CTRLS seems to be that the code in
v4l2-ctrls.c::set_ctrl() is not calling user_to_new() anymore after
calling update_from_auto_cluster(master).

However the root cause of the problem is that calling
update_from_auto_cluster(master) overrides also the _master_ control
state calling cur_to_new() while it was supposed to only update the
volatile controls.

Calling user_to_new() after update_from_auto_cluster(master) was just
masking the original bug by restoring the correct new value of the
master control before making the changes permanent.

Fix the original bug by making update_from_auto_cluster() not override
the new master control value.

Signed-off-by: Antonio Ospite 
Cc:   # for v3.17 and up
---

Hi,

I did check the patch with scripts/checkpatch.pl but it gives an error,
I think it's a false positive.

Ciao ciao,
   Antonio

 drivers/media/v4l2-core/v4l2-ctrls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index b6b7dcc..19fc06e 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -3043,7 +3043,7 @@ static void update_from_auto_cluster(struct v4l2_ctrl 
*master)
 {
int i;
 
-   for (i = 0; i < master->ncontrols; i++)
+   for (i = 1; i < master->ncontrols; i++)
cur_to_new(master->cluster[i]);
if (!call_op(master, g_volatile_ctrl))
for (i = 1; i < master->ncontrols; i++)
-- 
2.6.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: [PATCHv9 06/15] rc: Add HDMI CEC protocol handling

2015-10-14 Thread Hans Verkuil
On 10/14/2015 01:09 AM, Russell King - ARM Linux wrote:
> On Mon, Oct 12, 2015 at 01:50:47PM +0200, Hans Verkuil wrote:
>> On 10/06/2015 08:05 PM, Russell King - ARM Linux wrote:
>>> On Mon, Sep 07, 2015 at 03:44:35PM +0200, Hans Verkuil wrote:
 From: Kamil Debski 

 Add handling of remote control events coming from the HDMI CEC bus.
 This patch includes a new keymap that maps values found in the CEC
 messages to the keys pressed and released. Also, a new protocol has
 been added to the core.

 Signed-off-by: Kamil Debski 
 Signed-off-by: Hans Verkuil 
>>>
>>> (Added Mauro)
>>>
>>> Hmm, how is rc-cec supposed to be loaded?
>>
>> Is CONFIG_RC_MAP enabled in your config? Ran 'depmod -a'? (Sorry, I'm sure 
>> you've done
>> that, just checking...)
> 
> CONFIG_RC_MAP=m
> 
> and yes, if depmod hadn't have been run, modprobing rc-cec would not
> have worked - modprobe always looks up in the depmod information to
> find out where the module is located, and also to determine any
> dependencies.
> 
>> It's optional as I understand it, since you could configure the keytable from
>> userspace instead of using this module.
>>
>> For the record (just tried it), it does load fine on my setup.
> 
> Immediately after boot, I have:
> 
> # lsmod
> Module  Size  Used by
> ...
> coda   54685  0
> v4l2_mem2mem   14517  1 coda
> videobuf2_dma_contig 9478  1 coda
> videobuf2_vmalloc   5529  1 coda
> videobuf2_memops1888  2 videobuf2_dma_contig,videobuf2_vmalloc
> cecd_dw_hdmi3129  0
> # modprobe rc-cec
> # lsmod
> Module  Size  Used by
> rc_cec  1785  0
> ...
> coda   54685  0
> v4l2_mem2mem   14517  1 coda
> videobuf2_dma_contig 9478  1 coda
> videobuf2_vmalloc   5529  1 coda
> videobuf2_memops1888  2 videobuf2_dma_contig,videobuf2_vmalloc
> cecd_dw_hdmi3129  0
> 
> So, rc-cec is perfectly loadable, it just doesn't get loaded at boot.
> Manually loading it like this is useless though - I have to unload
> cecd_dw_hdmi and then re-load it after rc-cec is loaded for rc-cec to
> be seen.  At that point, (and with the help of a userspace program)
> things start working as expected.

Did you compile and install the v4l-utils found here:

http://git.linuxtv.org/cgit.cgi/hverkuil/v4l-utils.git/log/?h=cec

I think that the rc_cec module is loaded through some udev rules and
keytables that are installed by v4l-utils. The standard v4l-utils
doesn't know about cec, but my repo above does.

To be honest, I don't really understand how it works, but if you haven't
installed it yet then try it and see if that solves the problem.

>> BTW, I am still on the fence whether using the kernel RC subsystem is
>> the right thing to do. There are a number of CEC RC commands that use
>> extra parameters that cannot be mapped to the RC API, so you still
>> need to handle those manually.
> 
> Even though it is a remote control which is being forwarded for the
> most part, but there are operation codes which aren't related to
> key presses specified by the standard.  I don't think there's anything
> wrong with having a RC interface present, but allowing other interfaces
> as a possibility is a good thing too - it allows a certain amount of
> flexibility.
> 
> For example, with rc-cec loaded and properly bound, I can control at
> least rhythmbox within gnome using the TVs remote control with no
> modifications - and that happens because the X server passes on the
> events it receives via the event device.
> 
> Given the range of media applications, I think that's key - it needs
> to at least have the capability to plug into the existing ways of doing
> things, even if those ways are not perfect.
> 
>> Perhaps I should split it off into a separate patch and keep it out
>> from the initial pull request once we're ready for that.
> 
> I'm biased because it is an enablement feature - it allows CEC to work
> out of the box with at least some existing media apps. :)
> 

OK, useful feedback. I am considering putting the RC code under a kernel
config option though. So if the RC core is not enabled or you don't want
the RC part to be created, then you can opt to disable it.

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