Re: Question about 2 gp8psk patches I noticed, and possible bug.

[VDR User]

Hi Mauro,

Unfortunately the patch doesn't seem to have solved the problem. I do
have the kernel recompiled with debug enabled though per your irc msg.
dmesg gives me:

[   70.741073] usbcore: deregistering interface driver dvb_usb_gp8psk
[   70.741165] [ cut here ]
[   70.741172] WARNING: CPU: 1 PID: 2119 at kernel/module.c:1108
module_put+0x67/0x80
[   70.741174] Modules linked in: dvb_usb_gp8psk(-) dvb_usb dvb_core
nvidia_drm(PO) nvidia_modeset(PO) snd_hda_codec_hdmi snd_hda_intel
snd_hda_codec snd_hwdep snd_hda_core snd_pcm snd_timer snd soundcore
nvidia(PO)
[   70.741186] CPU: 1 PID: 2119 Comm: rmmod Tainted: P   O
4.8.4-build.2 #1
[   70.741187] Hardware name: MSI MS-7309/MS-7309, BIOS V1.12 02/23/2009
[   70.741189]   c12c15f0   c103fc7a c161ecc0
0001 0847
[   70.741194]  c161e50f 0454 c10a4b87 c10a4b87 0009 fb090cc0
 
[   70.741197]  f4e72000 c103fd43 0009   c10a4b87
fb08f6a0 c10a58fa
[   70.741202] Call Trace:
[   70.741206]  [] ? dump_stack+0x44/0x64
[   70.741209]  [] ? __warn+0xfa/0x120
[   70.741211]  [] ? module_put+0x67/0x80
[   70.741213]  [] ? module_put+0x67/0x80
[   70.741215]  [] ? warn_slowpath_null+0x23/0x30
[   70.741217]  [] ? module_put+0x67/0x80
[   70.741221]  [] ? gp8psk_fe_set_frontend+0x460/0x460
[dvb_usb_gp8psk]
[   70.741223]  [] ? symbol_put_addr+0x2a/0x50
[   70.741225]  [] ? gp8psk_usb_disconnect+0x4e/0x90 [dvb_usb_gp8psk]
[   70.741229]  [] ? usb_unbind_interface+0x62/0x250
[   70.741233]  [] ? _raw_spin_unlock_irqrestore+0xf/0x30
[   70.741235]  [] ? __pm_runtime_idle+0x44/0x70
[   70.741239]  [] ? __device_release_driver+0x78/0x120
[   70.741241]  [] ? driver_detach+0x87/0x90
[   70.741243]  [] ? bus_remove_driver+0x38/0x90
[   70.741245]  [] ? usb_deregister+0x58/0xb0
[   70.741248]  [] ? SyS_delete_module+0x130/0x1f0
[   70.741251]  [] ? __do_page_fault+0x1a0/0x440
[   70.741253]  [] ? exit_to_usermode_loop+0x85/0x90
[   70.741254]  [] ? do_fast_syscall_32+0x80/0x130
[   70.741257]  [] ? sysenter_past_esp+0x40/0x6a
[   70.741259] ---[ end trace a387b7eddb538bfb ]---
[   70.743654] dvb-usb: Genpix SkyWalker-2 DVB-S receiver successfully
deinitialized and disconnected.


I read the Bug Hunting url but it's still not clear to me which line
from that dmesg text I should be focused on. It would suggest the
first line (dump_stack+0x44/0x64) but you pointed to
gp8psk_fe_set_frontend so I'm not sure what to do next. When I go into
gdb I see:

gdb $(find /lib/modules/$(uname -r) -name dvb-usb-gp8psk.ko)
...
Reading symbols from
/lib/modules/4.8.4-build.2/kernel/drivers/media/usb/dvb-usb/dvb-usb-gp8psk.ko...(no
debugging symbols found)...done.
(gdb)

gdb /usr/src/linux/vmlinux
...
Reading symbols from /usr/src/linux/vmlinux...done.
(gdb)
(gdb)

"No debugging symbols found" doesn't sound good, but DEBUG is enabled:

$ grep "^CONFIG_DEBUG" /usr/src/linux/.config
CONFIG_DEBUG_RODATA=y
CONFIG_DEBUG_INFO=y
CONFIG_DEBUG_FS=y
CONFIG_DEBUG_KERNEL=y
CONFIG_DEBUG_MEMORY_INIT=y
CONFIG_DEBUG_PREEMPT=y
CONFIG_DEBUG_BUGVERBOSE=y



On Tue, Nov 8, 2016 at 9:55 AM, Mauro Carvalho Chehab
 wrote:
> Em Sat, 5 Nov 2016 19:24:58 -0700
> VDR User  escreveu:
>
>> I have
>> several different Genpix devices that use the gp8psk driver and in all
>> cases the following happens when I unload it:
>>
>> [494896.234616] usbcore: deregistering interface driver dvb_usb_gp8psk
>> [494896.234712] [ cut here ]
>> [494896.234719] WARNING: CPU: 1 PID: 28102 at kernel/module.c:1108
>> module_put+0x57/0x70
>> [494896.234720] Modules linked in: dvb_usb_gp8psk(-) dvb_usb dvb_core
>> nvidia_drm(PO) nvidia_modeset(PO) snd_hda_codec_hdmi snd_hda_intel
>> snd_hda_codec snd_hwdep snd_hda_core snd_pcm snd_timer snd soundcore
>> nvidia(PO) [last unloaded: rc_core]
>> [494896.234732] CPU: 1 PID: 28102 Comm: rmmod Tainted: PWC O
>>  4.8.4-build.1 #1
>> [494896.234733] Hardware name: MSI MS-7309/MS-7309, BIOS V1.12 02/23/2009
>> [494896.234735]   c12ba080   c103ed6a c1616014
>> 0001 6dc6
>> [494896.234739]  c1615862 0454 c109e8a7 c109e8a7 0009 
>>  f13f6a10
>> [494896.234743]  f5f5a600 c103ee33 0009   c109e8a7
>> f80ca4d0 c109f617
>> [494896.234746] Call Trace:
>> [494896.234753]  [] ? dump_stack+0x44/0x64
>> [494896.234756]  [] ? __warn+0xfa/0x120
>> [494896.234758]  [] ? module_put+0x57/0x70
>> [494896.234760]  [] ? module_put+0x57/0x70
>> [494896.234762]  [] ? warn_slowpath_null+0x23/0x30
>> [494896.234763]  [] ? module_put+0x57/0x70
>> [494896.234766]  [] ? gp8psk_fe_set_frontend+0x460/0x460
>> [dvb_usb_gp8psk]
>> [494896.234769]  [] ? symbol_put_addr+0x27/0x50
>> [494896.234771]  [] ?
>> dvb_usb_adapter_frontend_exit+0x3a/0x70 [dvb_usb]
>> [494896.234773]  [] ? dvb_usb_exit+0x2f/0xd0 [dvb_usb]
>> [494896.234776]  [] ? usb_disable_endpoint+0x7c/0xb0
>> 

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:   Wed Nov  9 05:00:18 CET 2016
media-tree git hash:bd676c0c04ec94bd830b9192e2c33f2c4532278d
media_build git hash:   dac8db4dd7fa3cc87715cb19ace554e080690b39
v4l-utils git hash: 788b674f3827607c09c31be11c91638f816aa6ae
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.7.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: 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.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: ERRORS
linux-3.10.1-i686: ERRORS
linux-3.11.1-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: WARNINGS
linux-4.9-rc1-i686: WARNINGS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: ERRORS
linux-3.10.1-x86_64: ERRORS
linux-3.11.1-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9-rc1-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK
smatch: ERRORS
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.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: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference

[Mauro Carvalho Chehab]

Em Tue, 8 Nov 2016 22:15:24 +0100
Benjamin Larsson  escreveu:

> On 11/08/2016 09:22 PM, Mauro Carvalho Chehab wrote:
> > Em Tue, 8 Nov 2016 10:42:03 -0800
> > Linus Torvalds  escreveu:
> >
> >> On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte  wrote:
> >>> Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module.
> >>
> >> Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't
> >> do DMA on stack"), which movced the DMA data array from the stack to
> >> the "private" pointer. In the process it also added serialization in
> >> the form of "data_mutex", but and now it oopses on that mutex because
> >> the private pointer is NULL.
> >>
> >> It looks like the "->private" pointer is allocated in 
> >> dvb_usb_adapter_init()
> >>
> >> cinergyt2_usb_probe ->
> >>   dvb_usb_device_init ->
> >> dvb_usb_init() ->
> >>   dvb_usb_adapter_init()
> >>
> >> but the dvb_usb_init() function calls dvb_usb_device_power_ctrl()
> >> (which calls the "power_ctrl" function, which is
> >> cinergyt2_power_ctrl() for that drive) *before* it initializes the
> >> private field.
> >>
> >> Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps?
> >
> > Calling it earlier won't work, as we need to load the firmware before
> > sending the power control commands on some devices.
> >
> > Probably the best here is to pass an extra optional function parameter
> > that will initialize the mutex before calling any functions.
> >
> > Btw, if it broke here, the DMA fixes will likely break on other drivers.
> > So, after Jörg tests this patch, I'll work on a patch series addressing
> > this issue on the other drivers I touched.
> >
> > Regards,
> > Mauro
> 
> Just for reference I got the following call trace a week ago. I looks 
> like this confirms that other drivers are affected also.

Yeah, I avoided serializing the logic that detects if the firmware is
loaded, but forgot that the power control had the same issue. The
newer dvb usb drivers use the dvb-usb-v2, so I didn't touch this
code for a while.

I'll need to review all touched drivers to be sure that they'll all
do the right thing. The good news is that it will likely simplify
the drivers' logic a little bit.

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

Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference

[Benjamin Larsson]

On 11/08/2016 09:22 PM, Mauro Carvalho Chehab wrote:

Em Tue, 8 Nov 2016 10:42:03 -0800
Linus Torvalds  escreveu:


On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte  wrote:

Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module.


Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't
do DMA on stack"), which movced the DMA data array from the stack to
the "private" pointer. In the process it also added serialization in
the form of "data_mutex", but and now it oopses on that mutex because
the private pointer is NULL.

It looks like the "->private" pointer is allocated in dvb_usb_adapter_init()

cinergyt2_usb_probe ->
  dvb_usb_device_init ->
dvb_usb_init() ->
  dvb_usb_adapter_init()

but the dvb_usb_init() function calls dvb_usb_device_power_ctrl()
(which calls the "power_ctrl" function, which is
cinergyt2_power_ctrl() for that drive) *before* it initializes the
private field.

Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps?


Calling it earlier won't work, as we need to load the firmware before
sending the power control commands on some devices.

Probably the best here is to pass an extra optional function parameter
that will initialize the mutex before calling any functions.

Btw, if it broke here, the DMA fixes will likely break on other drivers.
So, after Jörg tests this patch, I'll work on a patch series addressing
this issue on the other drivers I touched.

Regards,
Mauro


Just for reference I got the following call trace a week ago. I looks 
like this confirms that other drivers are affected also.


MvH
Benjamin Larsson


dvb-usb: found a 'Mygica T230 DVB-T/T2/C' in warm state.
BUG: unable to handle kernel NULL pointer dereference at   (null)
IP: [] __mutex_lock_slowpath+0x98/0x130
PGD 0
Oops: 0002 [#1] SMP
Modules linked in: dvb_usb_cxusb(OE+) dib0070(OE) dvb_usb_rtl28xxu(OE+) 
dvb_usb_dw2102(OE+) dvb_usb(OE) dvb_usb_v2(OE) dvb_core(OE) rc_core(OE) 
ipmi_ssif media(OE) lpc_sch joydev input_leds i2c_ismt ipmi_si 
8250_fintek ipmi_msghandler shpchp mac_hid kvm_intel kvm irqbypass 
ib_iser rdma_cm iw_cm ib_cm ib_sa ib_mad ib_core ib_addr iscsi_tcp 
libiscsi_tcp libiscsi scsi_transport_iscsi autofs4 btrfs raid10 
multipath linear raid456 async_raid6_recov async_memcpy async_pq 
async_xor async_tx xor raid6_pq libcrc32c raid0 raid1 hid_generic igb 
dca usbhid ptp pps_core i2c_algo_bit hid ahci libahci fjes
CPU: 3 PID: 571 Comm: systemd-udevd Tainted: G   OE 
4.4.0-45-generic #66-Ubuntu

Hardware name: Supermicro X9SBAA/X9SBAA, BIOS 1.00 04/29/2014
task: 880231678000 ti: 880232048000 task.ti: 880232048000
RIP: 0010:[]  [] 
__mutex_lock_slowpath+0x98/0x130

RSP: 0018:88023204b920  EFLAGS: 00010282
RAX:  RBX: 88023152fea8 RCX: 
RDX: 0001 RSI: 880231678000 RDI: 88023152feac
RBP: 88023204b970 R08: 880232048000 R09: 
R10: 0325 R11:  R12: 88023152feac
R13: 880231678000 R14:  R15: 88023152feb0
FS:  7fc5390d68c0() GS:88023fd8() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2:  CR3: 00023262f000 CR4: 06e0
Stack:
88023152feb0  88023204b978 27226a0c
01ff8802 88023152fea8 88023152fe40 88023152fea8
8800ba9e4000  88023204b988 8182f73f
Call Trace:
[] mutex_lock+0x1f/0x30
[] cxusb_ctrl_msg+0x5a/0xe0 [dvb_usb_cxusb]
[] cxusb_power_ctrl+0x58/0x60 [dvb_usb_cxusb]
[] cxusb_d680_dmb_power_ctrl+0x2b/0x90 [dvb_usb_cxusb]
[] dvb_usb_device_power_ctrl.part.4+0x31/0x50 [dvb_usb]
[] dvb_usb_device_power_ctrl+0x2a/0x40 [dvb_usb]
[] dvb_usb_device_init+0x244/0x750 [dvb_usb]
[] ? kernfs_activate+0x7a/0xe0
[] cxusb_probe+0x260/0x280 [dvb_usb_cxusb]
[] usb_probe_interface+0x118/0x2f0
[] driver_probe_device+0x222/0x4a0
[] __driver_attach+0x84/0x90
[] ? driver_probe_device+0x4a0/0x4a0
[] bus_for_each_dev+0x6c/0xc0
[] driver_attach+0x1e/0x20
[] bus_add_driver+0x1eb/0x280
[] driver_register+0x60/0xe0
[] usb_register_driver+0x84/0x140
[] ? 0xc03d8000
[] cxusb_driver_init+0x1e/0x1000 [dvb_usb_cxusb]
[] do_one_initcall+0xb3/0x200
[] ? __vunmap+0x91/0xe0
[] ? kmem_cache_alloc_trace+0x183/0x1f0
[] ? kfree+0x13a/0x150
[] do_init_module+0x5f/0x1cf
[] load_module+0x166f/0x1c10
[] ? __symbol_put+0x60/0x60
[] ? kernel_read+0x50/0x80
[] SYSC_finit_module+0xb4/0xe0
[] SyS_finit_module+0xe/0x10
[] entry_SYSCALL_64_fastpath+0x16/0x71
Code: e8 ae 1f 00 00 8b 03 83 f8 01 0f 84 94 00 00 00 48 8b 43 10 4c 8d 
7b 08 48 89 63 10 41 be ff ff ff ff 4c 89 3c 24 48 89 44 24 08 <48> 89 
20 4c 89 6c 24 10 eb 1f 49 c7 45 00 02 00 00 00 4c 89 e7

RIP  [] __mutex_lock_slowpath+0x98/0x130
 RSP 
CR2: 
---[ end trace b28b9190ee8e4917 ]---

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in

Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference

[Mauro Carvalho Chehab]

Em Tue, 8 Nov 2016 10:42:03 -0800
Linus Torvalds  escreveu:

> On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte  wrote:
> > Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module.  
> 
> Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't
> do DMA on stack"), which movced the DMA data array from the stack to
> the "private" pointer. In the process it also added serialization in
> the form of "data_mutex", but and now it oopses on that mutex because
> the private pointer is NULL.
> 
> It looks like the "->private" pointer is allocated in dvb_usb_adapter_init()
> 
> cinergyt2_usb_probe ->
>   dvb_usb_device_init ->
> dvb_usb_init() ->
>   dvb_usb_adapter_init()
> 
> but the dvb_usb_init() function calls dvb_usb_device_power_ctrl()
> (which calls the "power_ctrl" function, which is
> cinergyt2_power_ctrl() for that drive) *before* it initializes the
> private field.
> 
> Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps?

Calling it earlier won't work, as we need to load the firmware before
sending the power control commands on some devices.

Probably the best here is to pass an extra optional function parameter
that will initialize the mutex before calling any functions.

Btw, if it broke here, the DMA fixes will likely break on other drivers.
So, after Jörg tests this patch, I'll work on a patch series addressing
this issue on the other drivers I touched.

Regards,
Mauro

-

[PATCH RFC] cinergyT2-core: initialize the mutex early

NOTE: don't merge this patch as-is... I actually folded two patches
together here, in order to make easier to test, but the best is to
place the changes at the core first, and then the changes at the
drivers that would need an early init.

The mutex used to protect the URB data buffer needs to be
inialized early, as otherwise it will cause an OOPS:

dvb-usb: found a 'TerraTec/qanu USB2.0 Highspeed DVB-T Receiver' in warm state.
BUG: unable to handle kernel NULL pointer dereference at   (null)
IP: [] __mutex_lock_slowpath+0x6f/0x100 PGD 0
Oops: 0002 [#1] SMP
Modules linked in: dvb_usb_cinergyT2(+) dvb_usb
CPU: 0 PID: 2029 Comm: modprobe Not tainted 4.9.0-rc4-dvbmod #24
Hardware name: FUJITSU LIFEBOOK A544/FJNBB35 , BIOS Version 1.17 05/09/2014
task: 88020e943840 task.stack: 8801f36ec000
RIP: 0010:[]  [] 
__mutex_lock_slowpath+0x6f/0x100
RSP: 0018:8801f36efb10  EFLAGS: 00010282
RAX:  RBX: 88021509bdc8 RCX: c100
RDX: 0001 RSI:  RDI: 88021509bdcc
RBP: 8801f36efb58 R08: 88021f216320 R09: 0010
R10: 88021f216320 R11: 0023fee6c5a1 R12: 88020e943840
R13: 88021509bdcc R14:  R15: 88021509bdd0
FS:  7f21adb86740() GS:88021f20() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2:  CR3: 000215bce000 CR4: 001406f0
Stack:
 88021509bdd0   c0137c80
 88021509bdc8 8801f5944000 0001 c0136b00
 880213e52000 88021509bdc8 84661856 88021509bd80
Call Trace:
 [] ? mutex_lock+0x16/0x25
 [] ? cinergyt2_power_ctrl+0x1f/0x60 [dvb_usb_cinergyT2]
 [] ? dvb_usb_device_init+0x21e/0x5d0 [dvb_usb]
 [] ? cinergyt2_usb_probe+0x21/0x50 [dvb_usb_cinergyT2]
 [] ? usb_probe_interface+0xf3/0x2a0
 [] ? driver_probe_device+0x208/0x2b0
 [] ? __driver_attach+0x87/0x90
 [] ? driver_probe_device+0x2b0/0x2b0
 [] ? bus_for_each_dev+0x52/0x80
 [] ? bus_add_driver+0x1a3/0x220
 [] ? driver_register+0x56/0xd0
 [] ? usb_register_driver+0x77/0x130
 [] ? 0xc013a000
 [] ? do_one_initcall+0x46/0x180
 [] ? free_vmap_area_noflush+0x38/0x70
 [] ? kmem_cache_alloc+0x84/0xc0
 [] ? do_init_module+0x50/0x1be
 [] ? load_module+0x1d8b/0x2100
 [] ? find_symbol_in_section+0xa0/0xa0
 [] ? SyS_finit_module+0x89/0x90
 [] ? entry_SYSCALL_64_fastpath+0x13/0x94
Code: e8 a7 1d 00 00 8b 03 83 f8 01 0f 84 97 00 00 00 48 8b 43 10 4c 8d 7b 08 
48 89 63 10 4c 89 3c 24 41 be ff ff ff ff 48 89 44 24 08 <48> 89 20 4c 89 64 24 
10 eb 1a 49 c7 44 24 08 02 00 00 00 c6 43 RIP  [] 
__mutex_lock_slowpath+0x6f/0x100 RSP 
CR2: 

Reported-by: Jörg Otte 
Fixes: 6679a901c380 ("[media] cinergyT2-core: don't do DMA on stack")
Signed-off-by: Mauro Carvalho Chehab 

>From cbc7e48a86e8ffd16e49b10061120dfc417397f8 Mon Sep 17 00:00:00 2001
From: Mauro Carvalho Chehab 
Date: Tue, 8 Nov 2016 18:04:24 -0200
Subject: [PATCH] [media] dvb-usb: allow early initialization of usb device
 priv data

On some drivers, we need to initialize a mutex before calling
power control or firmware download routines. Add an extra
parameter to allow it.

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/drivers/media/usb/dvb-usb/a800.c b/drivers/media/usb/dvb-usb/a800.c
index 

Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference

[Linus Torvalds]

On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte  wrote:
> Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module.

Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't
do DMA on stack"), which movced the DMA data array from the stack to
the "private" pointer. In the process it also added serialization in
the form of "data_mutex", but and now it oopses on that mutex because
the private pointer is NULL.

It looks like the "->private" pointer is allocated in dvb_usb_adapter_init()

cinergyt2_usb_probe ->
  dvb_usb_device_init ->
dvb_usb_init() ->
  dvb_usb_adapter_init()

but the dvb_usb_init() function calls dvb_usb_device_power_ctrl()
(which calls the "power_ctrl" function, which is
cinergyt2_power_ctrl() for that drive) *before* it initializes the
private field.

Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps?

Linus
--
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: Question about 2 gp8psk patches I noticed, and possible bug.

[Mauro Carvalho Chehab]

Em Sat, 5 Nov 2016 19:24:58 -0700
VDR User  escreveu:

> I have
> several different Genpix devices that use the gp8psk driver and in all
> cases the following happens when I unload it:
> 
> [494896.234616] usbcore: deregistering interface driver dvb_usb_gp8psk
> [494896.234712] [ cut here ]
> [494896.234719] WARNING: CPU: 1 PID: 28102 at kernel/module.c:1108
> module_put+0x57/0x70
> [494896.234720] Modules linked in: dvb_usb_gp8psk(-) dvb_usb dvb_core
> nvidia_drm(PO) nvidia_modeset(PO) snd_hda_codec_hdmi snd_hda_intel
> snd_hda_codec snd_hwdep snd_hda_core snd_pcm snd_timer snd soundcore
> nvidia(PO) [last unloaded: rc_core]
> [494896.234732] CPU: 1 PID: 28102 Comm: rmmod Tainted: PWC O
>  4.8.4-build.1 #1
> [494896.234733] Hardware name: MSI MS-7309/MS-7309, BIOS V1.12 02/23/2009
> [494896.234735]   c12ba080   c103ed6a c1616014
> 0001 6dc6
> [494896.234739]  c1615862 0454 c109e8a7 c109e8a7 0009 
>  f13f6a10
> [494896.234743]  f5f5a600 c103ee33 0009   c109e8a7
> f80ca4d0 c109f617
> [494896.234746] Call Trace:
> [494896.234753]  [] ? dump_stack+0x44/0x64
> [494896.234756]  [] ? __warn+0xfa/0x120
> [494896.234758]  [] ? module_put+0x57/0x70
> [494896.234760]  [] ? module_put+0x57/0x70
> [494896.234762]  [] ? warn_slowpath_null+0x23/0x30
> [494896.234763]  [] ? module_put+0x57/0x70
> [494896.234766]  [] ? gp8psk_fe_set_frontend+0x460/0x460
> [dvb_usb_gp8psk]
> [494896.234769]  [] ? symbol_put_addr+0x27/0x50
> [494896.234771]  [] ?
> dvb_usb_adapter_frontend_exit+0x3a/0x70 [dvb_usb]
> [494896.234773]  [] ? dvb_usb_exit+0x2f/0xd0 [dvb_usb]
> [494896.234776]  [] ? usb_disable_endpoint+0x7c/0xb0
> [494896.234778]  [] ? dvb_usb_device_exit+0x2a/0x50 [dvb_usb]
> [494896.234780]  [] ? usb_unbind_interface+0x62/0x250
> [494896.234782]  [] ? __pm_runtime_idle+0x44/0x70
> [494896.234785]  [] ? __device_release_driver+0x78/0x120
> [494896.234787]  [] ? driver_detach+0x87/0x90
> [494896.234789]  [] ? bus_remove_driver+0x38/0x90
> [494896.234791]  [] ? usb_deregister+0x58/0xb0
> [494896.234793]  [] ? SyS_delete_module+0x130/0x1f0
> [494896.234796]  [] ? task_work_run+0x64/0x80
> [494896.234798]  [] ? exit_to_usermode_loop+0x85/0x90
> [494896.234800]  [] ? do_fast_syscall_32+0x80/0x130
> [494896.234803]  [] ? sysenter_past_esp+0x40/0x6a
> [494896.234805] ---[ end trace 6ebc60ef3981792f ]---
> [494896.235890] dvb-usb: Genpix SkyWalker-2 DVB-S receiver
> successfully deinitialized and disconnected.

I suspect that this is due to a race condition at device unregister.
could you please try the enclosed patch?

gp8psk: unregister gp8psk-fe early

Letting gp8psk-fe to unregister late can cause race issues.

Signed-off-by: Mauro Carvalho Chehab 


diff --git a/drivers/media/usb/dvb-usb/gp8psk.c 
b/drivers/media/usb/dvb-usb/gp8psk.c
index 2829e3082d15..04ea2bbbe5ae 100644
--- a/drivers/media/usb/dvb-usb/gp8psk.c
+++ b/drivers/media/usb/dvb-usb/gp8psk.c
@@ -270,6 +270,32 @@ static int gp8psk_usb_probe(struct usb_interface *intf,
return ret;
 }
 
+static void gp8psk_usb_disconnect(struct usb_interface *intf)
+{
+   struct dvb_usb_device *d = usb_get_intfdata(intf);
+   struct dvb_usb_adapter *adap;
+   int i, n;
+
+   /*
+* As gsp8psk-fe can call back this driver, in order to do URB
+* transfers, we need to manually exit the frontend earlier.
+*/
+   for (n = 0; n < d->num_adapters_initialized; n++) {
+   adap = >adapter[n];
+   i = adap->num_frontends_initialized - 1;
+
+   for (; i >= 0; i--) {
+   if (adap->fe_adap[i].fe != NULL) {
+   dvb_unregister_frontend(adap->fe_adap[i].fe);
+   dvb_frontend_detach(adap->fe_adap[i].fe);
+   }
+   }
+   adap->num_frontends_initialized = 0;
+   }
+
+   dvb_usb_device_exit(intf);
+}
+
 static struct usb_device_id gp8psk_usb_table [] = {
{ USB_DEVICE(USB_VID_GENPIX, USB_PID_GENPIX_8PSK_REV_1_COLD) },
{ USB_DEVICE(USB_VID_GENPIX, USB_PID_GENPIX_8PSK_REV_1_WARM) },
@@ -338,7 +364,7 @@ static struct dvb_usb_device_properties gp8psk_properties = 
{
 static struct usb_driver gp8psk_usb_driver = {
.name   = "dvb_usb_gp8psk",
.probe  = gp8psk_usb_probe,
-   .disconnect = dvb_usb_device_exit,
+   .disconnect = gp8psk_usb_disconnect,
.id_table   = gp8psk_usb_table,
 };
 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC v4 01/21] Revert "[media] media: fix media devnode ioctl/syscall and unregister race"

[Mauro Carvalho Chehab]

Em Tue,  8 Nov 2016 15:55:10 +0200
Sakari Ailus  escreveu:

> This reverts commit 6f0dd24a084a ("[media] media: fix media devnode
> ioctl/syscall and unregister race"). The commit was part of an original
> patchset to avoid crashes when an unregistering device is in use.

As I said before: I will only look on such patch series if you don't
randomly revert patches that aren't broken.

Also, if this series require changes on drivers, it is up to you
to do such changes in a way that won't break patch bisectability,
so, each change should be self-contained and touch all drivers that
are affected by the kAPI change.

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

Re: [PATCH v2] media: i2c-polling: add i2c-polling driver

[Matt Ranostay]

On Mon, Nov 7, 2016 at 11:03 PM, Ozgur  wrote:
> Dear Matt;
>
> I think the "inux/slab.h" line is incorrect and I fixed to "linux/slab.h", I
> tested. Success now.
>
> Regards,
>
> ~ Ozgur
>
>>> -#include 
>>> +#include 

Gah wondering how it built for me. will fix in v3


>
>
> 2016-11-08 9:00 GMT+03:00 Matt Ranostay :
>>
>> There are several thermal sensors that only have a low-speed bus
>> interface but output valid video data. This patchset enables support
>> for the AMG88xx "Grid-Eye" sensor family.
>>
>> Cc: Attila Kinali 
>> Cc: Marek Vasut 
>> Cc: Luca Barbato 
>> Signed-off-by: Matt Ranostay 
>> ---
>> Changes from v1:
>> * correct i2c_polling_remove() operations
>> * fixed delay calcuation in buffer_queue()
>> * add include linux/slab.h
>>
>>  drivers/media/i2c/Kconfig   |   8 +
>>  drivers/media/i2c/Makefile  |   1 +
>>  drivers/media/i2c/i2c-polling.c | 469
>> 
>>  3 files changed, 478 insertions(+)
>>  create mode 100644 drivers/media/i2c/i2c-polling.c
>>
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 2669b4bad910..6346eeecfaae 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -768,6 +768,14 @@ config VIDEO_M52790
>>
>>  To compile this driver as a module, choose M here: the
>>  module will be called m52790.
>> +
>> +config VIDEO_I2C_POLLING
>> +   tristate "I2C polling video support"
>> +   depends on VIDEO_V4L2 && I2C
>> +   select VIDEOBUF2_VMALLOC
>> +   ---help---
>> + Enable the I2C polling video support which supports the
>> following:
>> +  * Panasonic AMG88xx Grid-Eye Sensors
>>  endmenu
>>
>>  menu "Sensors used on soc_camera driver"
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 92773b2e6225..8182ec9f66b9 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -79,6 +79,7 @@ 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
>> +obj-$(CONFIG_VIDEO_I2C_POLLING)+= i2c-polling.o
>>  obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
>>  obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
>>  obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
>> diff --git a/drivers/media/i2c/i2c-polling.c
>> b/drivers/media/i2c/i2c-polling.c
>> new file mode 100644
>> index ..807f3aa84245
>> --- /dev/null
>> +++ b/drivers/media/i2c/i2c-polling.c
>> @@ -0,0 +1,469 @@
>> +/*
>> + * i2c_polling.c - Support for polling I2C video devices
>> + *
>> + * Copyright (C) 2016 Matt Ranostay 
>> + *
>> + * Based on the orginal work drivers/media/parport/bw-qcam.c
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * Supported:
>> + * - Panasonic AMG88xx Grid-Eye Sensors
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define I2C_POLLING_DRIVER "i2c-polling"
>> +
>> +struct i2c_polling_chip;
>> +
>> +struct i2c_polling_data {
>> +   struct i2c_client *client;
>> +   const struct i2c_polling_chip *chip;
>> +   struct mutex lock;
>> +   struct mutex queue_lock;
>> +   unsigned int last_update;
>> +
>> +   struct v4l2_device v4l2_dev;
>> +   struct video_device vdev;
>> +   struct vb2_queue vb_vidq;
>> +};
>> +
>> +static struct v4l2_fmtdesc amg88xx_format = {
>> +   .description = "12-bit Greyscale",
>> +   .pixelformat = V4L2_PIX_FMT_Y12,
>> +};
>> +
>> +static struct v4l2_frmsize_discrete amg88xx_size = {
>> +   .width = 8,
>> +   .height = 8,
>> +};
>> +
>> +struct i2c_polling_chip {
>> +   /* video dimensions */
>> +   struct v4l2_fmtdesc *format;
>> +   struct v4l2_frmsize_discrete *size;
>> +
>> +   /* max frames per second */
>> +   unsigned int max_fps;
>> +
>> +   /* pixel buffer size */
>> +   unsigned int buffer_size;
>> +
>> +   /* xfer function */
>> +   int (*xfer)(struct i2c_polling_data *data, char *buf);
>> +};
>> +
>> +enum {
>> +   AMG88XX = 0,
>> +   I2C_POLLING_CHIP_CNT,
>> +};
>> +
>> +static int amg88xx_xfer(struct 

[PATCH v2 1/1] v4l: compat: Prevent allocating excessive amounts of memory

[Sakari Ailus]

get_v4l2_ext_controls32() is used to convert the 32-bit compat struct into
native 64-bit representation. The function multiplies the array length by
the entry length before validating size. Perform the size validation
first.

Also use unsigned values for size computation.

Make similar changes to get_v4l2_buffer32() for multi-plane buffers.

Signed-off-by: Sakari Ailus 
---
since v1:

- Use unsigned loop variables.

- Count from zero upwards rather than downwards until zero.

 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 30 +++
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index bacecbd..eac9565 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -409,7 +409,6 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, struct 
v4l2_buffer32 __user
struct v4l2_plane32 __user *uplane32;
struct v4l2_plane __user *uplane;
compat_caddr_t p;
-   int num_planes;
int ret;
 
if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_buffer32)) ||
@@ -429,12 +428,15 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, 
struct v4l2_buffer32 __user
return -EFAULT;
 
if (V4L2_TYPE_IS_MULTIPLANAR(kp->type)) {
-   num_planes = kp->length;
-   if (num_planes == 0) {
+   unsigned int num_planes;
+
+   if (kp->length == 0) {
kp->m.planes = NULL;
/* num_planes == 0 is legal, e.g. when userspace doesn't
 * need planes array on DQBUF*/
return 0;
+   } else if (kp->length > VIDEO_MAX_PLANES) {
+   return -EINVAL;
}
 
if (get_user(p, >m.planes))
@@ -442,16 +444,16 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, 
struct v4l2_buffer32 __user
 
uplane32 = compat_ptr(p);
if (!access_ok(VERIFY_READ, uplane32,
-   num_planes * sizeof(struct v4l2_plane32)))
+   kp->length * sizeof(struct v4l2_plane32)))
return -EFAULT;
 
/* We don't really care if userspace decides to kill itself
 * by passing a very big num_planes value */
-   uplane = compat_alloc_user_space(num_planes *
-   sizeof(struct v4l2_plane));
+   uplane = compat_alloc_user_space(kp->length *
+sizeof(struct v4l2_plane));
kp->m.planes = (__force struct v4l2_plane *)uplane;
 
-   while (--num_planes >= 0) {
+   for (num_planes = 0; num_planes < kp->length; num_planes++) {
ret = get_v4l2_plane32(uplane, uplane32, kp->memory);
if (ret)
return ret;
@@ -665,7 +667,7 @@ static int get_v4l2_ext_controls32(struct v4l2_ext_controls 
*kp, struct v4l2_ext
 {
struct v4l2_ext_control32 __user *ucontrols;
struct v4l2_ext_control __user *kcontrols;
-   int n;
+   unsigned int n;
compat_caddr_t p;
 
if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_ext_controls32)) ||
@@ -675,20 +677,22 @@ static int get_v4l2_ext_controls32(struct 
v4l2_ext_controls *kp, struct v4l2_ext
copy_from_user(kp->reserved, up->reserved,
   sizeof(kp->reserved)))
return -EFAULT;
-   n = kp->count;
-   if (n == 0) {
+   if (kp->count == 0) {
kp->controls = NULL;
return 0;
+   } else if (kp->count > V4L2_CID_MAX_CTRLS) {
+   return -EINVAL;
}
if (get_user(p, >controls))
return -EFAULT;
ucontrols = compat_ptr(p);
if (!access_ok(VERIFY_READ, ucontrols,
-   n * sizeof(struct v4l2_ext_control32)))
+   kp->count * sizeof(struct v4l2_ext_control32)))
return -EFAULT;
-   kcontrols = compat_alloc_user_space(n * sizeof(struct 
v4l2_ext_control));
+   kcontrols = compat_alloc_user_space(kp->count *
+   sizeof(struct v4l2_ext_control));
kp->controls = (__force struct v4l2_ext_control *)kcontrols;
-   while (--n >= 0) {
+   for (n = 0; n < kp->count; n++) {
u32 id;
 
if (copy_in_user(kcontrols, ucontrols, sizeof(*ucontrols)))
-- 
2.7.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

[RFC v4 06/21] media device: Drop nop release callback

[Sakari Ailus]

The release callback is only used to print a debug message. Drop it. (It
will be re-introduced later in a different form.)

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-device.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index a9d543f..a31329d 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -540,11 +540,6 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
  * Registration/unregistration
  */
 
-static void media_device_release(struct media_devnode *devnode)
-{
-   dev_dbg(devnode->parent, "Media device released\n");
-}
-
 /**
  * media_device_register_entity - Register an entity with a media device
  * @mdev:  The media device
@@ -706,7 +701,6 @@ int __must_check __media_device_register(struct 
media_device *mdev,
/* Register the device node. */
mdev->devnode.fops = _device_fops;
mdev->devnode.parent = mdev->dev;
-   mdev->devnode.release = media_device_release;
 
/* Set version 0 to indicate user-space that the graph is static */
mdev->topology_version = 0;
-- 
2.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

[RFC v4 03/21] Revert "[media] media-device: dynamically allocate struct media_devnode"

[Sakari Ailus]

This reverts commit a087ce704b80 ("[media] media-device: dynamically
allocate struct media_devnode"). The commit was part of an original
patchset to avoid crashes when an unregistering device is in use.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-device.c   | 44 +++---
 drivers/media/media-devnode.c  |  7 +-
 drivers/media/usb/au0828/au0828-core.c |  4 ++--
 drivers/media/usb/uvc/uvc_driver.c |  2 +-
 include/media/media-device.h   |  5 +++-
 include/media/media-devnode.h  | 15 ++--
 6 files changed, 24 insertions(+), 53 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 6f5ed09..0e07300 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -417,7 +417,7 @@ static long media_device_ioctl(struct file *filp, unsigned 
int cmd,
   unsigned long __arg)
 {
struct media_devnode *devnode = media_devnode_data(filp);
-   struct media_device *dev = devnode->media_dev;
+   struct media_device *dev = to_media_device(devnode);
const struct media_ioctl_info *info;
void __user *arg = (void __user *)__arg;
char __karg[256], *karg = __karg;
@@ -493,7 +493,7 @@ static long media_device_compat_ioctl(struct file *filp, 
unsigned int cmd,
  unsigned long arg)
 {
struct media_devnode *devnode = media_devnode_data(filp);
-   struct media_device *dev = devnode->media_dev;
+   struct media_device *dev = to_media_device(devnode);
long ret;
 
switch (cmd) {
@@ -529,8 +529,7 @@ static const struct media_file_operations media_device_fops 
= {
 static ssize_t show_model(struct device *cd,
  struct device_attribute *attr, char *buf)
 {
-   struct media_devnode *devnode = to_media_devnode(cd);
-   struct media_device *mdev = devnode->media_dev;
+   struct media_device *mdev = to_media_device(to_media_devnode(cd));
 
return sprintf(buf, "%.*s\n", (int)sizeof(mdev->model), mdev->model);
 }
@@ -703,34 +702,23 @@ EXPORT_SYMBOL_GPL(media_device_cleanup);
 int __must_check __media_device_register(struct media_device *mdev,
 struct module *owner)
 {
-   struct media_devnode *devnode;
int ret;
 
-   devnode = kzalloc(sizeof(*devnode), GFP_KERNEL);
-   if (!devnode)
-   return -ENOMEM;
-
/* Register the device node. */
-   mdev->devnode = devnode;
-   devnode->fops = _device_fops;
-   devnode->parent = mdev->dev;
-   devnode->release = media_device_release;
+   mdev->devnode.fops = _device_fops;
+   mdev->devnode.parent = mdev->dev;
+   mdev->devnode.release = media_device_release;
 
/* Set version 0 to indicate user-space that the graph is static */
mdev->topology_version = 0;
 
-   ret = media_devnode_register(mdev, devnode, owner);
-   if (ret < 0) {
-   mdev->devnode = NULL;
-   kfree(devnode);
+   ret = media_devnode_register(>devnode, owner);
+   if (ret < 0)
return ret;
-   }
 
-   ret = device_create_file(>dev, _attr_model);
+   ret = device_create_file(>devnode.dev, _attr_model);
if (ret < 0) {
-   mdev->devnode = NULL;
-   media_devnode_unregister(devnode);
-   kfree(devnode);
+   media_devnode_unregister(>devnode);
return ret;
}
 
@@ -781,7 +769,7 @@ void media_device_unregister(struct media_device *mdev)
mutex_lock(>graph_mutex);
 
/* Check if mdev was ever registered at all */
-   if (!media_devnode_is_registered(mdev->devnode)) {
+   if (!media_devnode_is_registered(>devnode)) {
mutex_unlock(>graph_mutex);
return;
}
@@ -804,13 +792,9 @@ void media_device_unregister(struct media_device *mdev)
 
mutex_unlock(>graph_mutex);
 
-   dev_dbg(mdev->dev, "Media device unregistered\n");
-
-   /* Check if mdev devnode was registered */
-   if (media_devnode_is_registered(mdev->devnode)) {
-   device_remove_file(>devnode->dev, _attr_model);
-   media_devnode_unregister(mdev->devnode);
-   }
+   device_remove_file(>devnode.dev, _attr_model);
+   dev_dbg(mdev->dev, "Media device unregistering\n");
+   media_devnode_unregister(>devnode);
 }
 EXPORT_SYMBOL_GPL(media_device_unregister);
 
diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index ecdc02d..7481c96 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -44,7 +44,6 @@
 #include 
 
 #include 
-#include 
 
 #define MEDIA_NUM_DEVICES  256
 #define MEDIA_NAME "media"
@@ -75,8 +74,6 @@ static void media_devnode_release(struct device *cd)
/* Release media_devnode and 

[RFC v4 05/21] media: devnode: Rename mdev argument as devnode

[Sakari Ailus]

Historically, mdev argument name was being used on both struct
media_device and struct media_devnode. Recently most occurrences of mdev
referring to struct media_devnode were replaced by devnode, which makes
more sense. Fix the last remaining occurrence.

Fixes: 163f1e93e9950 ("[media] media-devnode: fix namespace mess")
Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/media-device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index bb19c04..a9d543f 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -540,9 +540,9 @@ static DEVICE_ATTR(model, S_IRUGO, show_model, NULL);
  * Registration/unregistration
  */
 
-static void media_device_release(struct media_devnode *mdev)
+static void media_device_release(struct media_devnode *devnode)
 {
-   dev_dbg(mdev->parent, "Media device released\n");
+   dev_dbg(devnode->parent, "Media device released\n");
 }
 
 /**
-- 
2.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

[RFC v4 04/21] media: Remove useless curly braces and parentheses

[Sakari Ailus]

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/media-device.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 0e07300..bb19c04 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -594,9 +594,8 @@ int __must_check media_device_register_entity(struct 
media_device *mdev,
   >pads[i].graph_obj);
 
/* invoke entity_notify callbacks */
-   list_for_each_entry_safe(notify, next, >entity_notify, list) {
-   (notify)->notify(entity, notify->notify_data);
-   }
+   list_for_each_entry_safe(notify, next, >entity_notify, list)
+   notify->notify(entity, notify->notify_data);
 
if (mdev->entity_internal_idx_max
>= mdev->pm_count_walk.ent_enum.idx_max) {
-- 
2.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

[RFC v4 14/21] media device: Get the media device driver's device

[Sakari Ailus]

The struct device of the media device driver (i.e. not that of the media
devnode) is pointed to by the media device. The struct device pointer is
mostly used for debug prints.

Ensure it will stay around as long as the media device does.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-device.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 2e52e44..648c64c 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -724,6 +724,7 @@ static void media_device_release(struct media_devnode 
*devnode)
mdev->entity_internal_idx_max = 0;
media_entity_graph_walk_cleanup(>pm_count_walk);
mutex_destroy(>graph_mutex);
+   put_device(mdev->dev);
 
kfree(mdev);
 }
@@ -732,9 +733,15 @@ struct media_device *media_device_alloc(struct device *dev)
 {
struct media_device *mdev;
 
+   dev = get_device(dev);
+   if (!dev)
+   return NULL;
+
mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
-   if (!mdev)
+   if (!mdev) {
+   put_device(dev);
return NULL;
+   }
 
mdev->dev = dev;
media_device_init(mdev);
-- 
2.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

[RFC v4 13/21] media device: Deprecate media_device_{init,cleanup}() for drivers

[Sakari Ailus]

Drivers should no longer directly allocate media_device but rely on
media_device_alloc(), media_device_get() and media_device_put() instead.
Deprecate media_device_init() and media_device_cleanup().

Signed-off-by: Sakari Ailus 
---
 include/media/media-device.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/include/media/media-device.h b/include/media/media-device.h
index fc0d82a..ae2bc08 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -203,6 +203,10 @@ static inline __must_check int media_entity_enum_init(
  * So drivers need to first initialize the media device, register any entity
  * within the media device, create pad to pad links and then finally register
  * the media device by calling media_device_register() as a final step.
+ *
+ * Note that using this function in drivers is DEPRECATED. New drivers
+ * must use media_device_alloc() and manage references using
+ * media_device_get() and media_device_put() instead.
  */
 void media_device_init(struct media_device *mdev);
 
@@ -251,6 +255,10 @@ struct media_device *media_device_alloc(struct device 
*dev);
  *
  * This function that will destroy the graph_mutex that is
  * initialized in media_device_init().
+ *
+ * Note that using this function in drivers is DEPRECATED. New drivers
+ * must use media_device_alloc() and manage references using
+ * media_device_get() and media_device_put() instead.
  */
 void media_device_cleanup(struct media_device *mdev);
 
-- 
2.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

[RFC v4 10/21] media: Shuffle functions around

[Sakari Ailus]

As the call paths of the functions in question will change, move them
around in anticipation of that. No other changes.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/media-device.c | 56 ++--
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index c79a9f5..0920c02 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -660,6 +660,34 @@ void media_device_unregister_entity(struct media_entity 
*entity)
 }
 EXPORT_SYMBOL_GPL(media_device_unregister_entity);
 
+int __must_check media_device_register_entity_notify(struct media_device *mdev,
+   struct media_entity_notify *nptr)
+{
+   mutex_lock(>graph_mutex);
+   list_add_tail(>list, >entity_notify);
+   mutex_unlock(>graph_mutex);
+   return 0;
+}
+EXPORT_SYMBOL_GPL(media_device_register_entity_notify);
+
+/*
+ * Note: Should be called with mdev->lock held.
+ */
+static void __media_device_unregister_entity_notify(struct media_device *mdev,
+   struct media_entity_notify *nptr)
+{
+   list_del(>list);
+}
+
+void media_device_unregister_entity_notify(struct media_device *mdev,
+   struct media_entity_notify *nptr)
+{
+   mutex_lock(>graph_mutex);
+   __media_device_unregister_entity_notify(mdev, nptr);
+   mutex_unlock(>graph_mutex);
+}
+EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
+
 /**
  * media_device_init() - initialize a media device
  * @mdev:  The media device
@@ -743,34 +771,6 @@ int __must_check __media_device_register(struct 
media_device *mdev,
 }
 EXPORT_SYMBOL_GPL(__media_device_register);
 
-int __must_check media_device_register_entity_notify(struct media_device *mdev,
-   struct media_entity_notify *nptr)
-{
-   mutex_lock(>graph_mutex);
-   list_add_tail(>list, >entity_notify);
-   mutex_unlock(>graph_mutex);
-   return 0;
-}
-EXPORT_SYMBOL_GPL(media_device_register_entity_notify);
-
-/*
- * Note: Should be called with mdev->lock held.
- */
-static void __media_device_unregister_entity_notify(struct media_device *mdev,
-   struct media_entity_notify *nptr)
-{
-   list_del(>list);
-}
-
-void media_device_unregister_entity_notify(struct media_device *mdev,
-   struct media_entity_notify *nptr)
-{
-   mutex_lock(>graph_mutex);
-   __media_device_unregister_entity_notify(mdev, nptr);
-   mutex_unlock(>graph_mutex);
-}
-EXPORT_SYMBOL_GPL(media_device_unregister_entity_notify);
-
 void media_device_unregister(struct media_device *mdev)
 {
struct media_entity *entity;
-- 
2.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

[RFC v4 11/21] media device: Refcount the media device

[Sakari Ailus]

As the struct media_device embeds struct media_devnode, the lifetime of
that object must be that same than that of the media_device.

References are obtained by media_entity_get() and released by
media_entity_put(). In case a driver uses media_device_alloc() to allocate
its media device, it must release the media device by calling
media_device_put() rather than media_device_cleanup().

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-device.c | 13 +
 include/media/media-device.h | 31 +++
 2 files changed, 44 insertions(+)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 0920c02..e9f6e76 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -712,6 +712,17 @@ void media_device_init(struct media_device *mdev)
 }
 EXPORT_SYMBOL_GPL(media_device_init);
 
+static void media_device_release(struct media_devnode *devnode)
+{
+   struct media_device *mdev = to_media_device(devnode);
+
+   dev_dbg(devnode->parent, "Media device released\n");
+
+   media_device_cleanup(mdev);
+
+   kfree(mdev);
+}
+
 struct media_device *media_device_alloc(struct device *dev)
 {
struct media_device *mdev;
@@ -723,6 +734,8 @@ struct media_device *media_device_alloc(struct device *dev)
mdev->dev = dev;
media_device_init(mdev);
 
+   mdev->devnode.release = media_device_release;
+
return mdev;
 }
 EXPORT_SYMBOL_GPL(media_device_alloc);
diff --git a/include/media/media-device.h b/include/media/media-device.h
index c9b5798..fc0d82a 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -212,10 +212,39 @@ void media_device_init(struct media_device *mdev);
  * @dev:   The associated struct device pointer
  *
  * Allocate and initialise a media device. Returns a media device.
+ * The media device is refcounted, and this function returns a media
+ * device the refcount of which is one (1).
+ *
+ * References are taken and given using media_device_get() and
+ * media_device_put().
  */
 struct media_device *media_device_alloc(struct device *dev);
 
 /**
+ * media_device_get() - Get a reference to a media device
+ *
+ * mdev: media device
+ */
+#define media_device_get(mdev) \
+   do {\
+   dev_dbg((mdev)->dev, "%s: get media device %s\n",   \
+   __func__, (mdev)->bus_info);\
+   get_device(&(mdev)->devnode.dev);   \
+   } while (0)
+
+/**
+ * media_device_put() - Put a reference to a media device
+ *
+ * mdev: media device
+ */
+#define media_device_put(mdev) \
+   do {\
+   dev_dbg((mdev)->dev, "%s: put media device %s\n",   \
+   __func__, (mdev)->bus_info);\
+   put_device(&(mdev)->devnode.dev);   \
+   } while (0)
+
+/**
  * media_device_cleanup() - Cleanups a media device element
  *
  * @mdev:  pointer to struct _device
@@ -464,6 +493,8 @@ static inline struct media_device 
*media_device_alloc(struct device *dev)
 {
return NULL;
 }
+#define media_device_get(mdev) do { } while (0)
+#define media_device_put(mdev) do { } while (0)
 static inline int media_device_register(struct media_device *mdev)
 {
return 0;
-- 
2.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

[RFC v4 12/21] media device: Initialise media devnode in media_device_init()

[Sakari Ailus]

Call media_devnode_init() from media_device_init(). This has the effect of
creating a struct device for the media_devnode before it is registered,
making it possible to obtain a reference to it for e.g. video devices.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-device.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index e9f6e76..2e52e44 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -708,6 +708,8 @@ void media_device_init(struct media_device *mdev)
mutex_init(>graph_mutex);
ida_init(>entity_internal_idx);
 
+   media_devnode_init(>devnode);
+
dev_dbg(mdev->dev, "Media device initialized\n");
 }
 EXPORT_SYMBOL_GPL(media_device_init);
@@ -718,7 +720,10 @@ static void media_device_release(struct media_devnode 
*devnode)
 
dev_dbg(devnode->parent, "Media device released\n");
 
-   media_device_cleanup(mdev);
+   ida_destroy(>entity_internal_idx);
+   mdev->entity_internal_idx_max = 0;
+   media_entity_graph_walk_cleanup(>pm_count_walk);
+   mutex_destroy(>graph_mutex);
 
kfree(mdev);
 }
@@ -746,6 +751,7 @@ void media_device_cleanup(struct media_device *mdev)
mdev->entity_internal_idx_max = 0;
media_entity_graph_walk_cleanup(>pm_count_walk);
mutex_destroy(>graph_mutex);
+   media_device_put(mdev);
 }
 EXPORT_SYMBOL_GPL(media_device_cleanup);
 
@@ -761,26 +767,19 @@ int __must_check __media_device_register(struct 
media_device *mdev,
/* Set version 0 to indicate user-space that the graph is static */
mdev->topology_version = 0;
 
-   media_devnode_init(>devnode);
-
ret = media_devnode_register(>devnode, owner);
if (ret < 0)
-   goto out_put;
+   return ret;
 
ret = device_create_file(>devnode.dev, _attr_model);
-   if (ret < 0)
-   goto out_unregister;
+   if (ret < 0) {
+   media_devnode_unregister(>devnode);
+   return ret;
+   }
 
dev_dbg(mdev->dev, "Media device registered\n");
 
return 0;
-
-out_unregister:
-   media_devnode_unregister(>devnode);
-out_put:
-   put_device(>devnode.dev);
-
-   return ret;
 }
 EXPORT_SYMBOL_GPL(__media_device_register);
 
@@ -823,7 +822,6 @@ void media_device_unregister(struct media_device *mdev)
device_remove_file(>devnode.dev, _attr_model);
dev_dbg(mdev->dev, "Media device unregistering\n");
media_devnode_unregister(>devnode);
-   put_device(>devnode.dev);
 }
 EXPORT_SYMBOL_GPL(media_device_unregister);
 
-- 
2.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

[RFC v4 17/21] v4l: Acquire a reference to the media device for every video device

[Sakari Ailus]

The video device depends on the existence of its media device --- if there
is one. Acquire a reference to it.

Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/v4l2-dev.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c 
b/drivers/media/v4l2-core/v4l2-dev.c
index 8be561a..530d53e 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -171,6 +171,9 @@ static void v4l2_device_release(struct device *cd)
 {
struct video_device *vdev = to_video_device(cd);
struct v4l2_device *v4l2_dev = vdev->v4l2_dev;
+#ifdef CONFIG_MEDIA_CONTROLLER
+   struct media_device *mdev = v4l2_dev->mdev;
+#endif
 
mutex_lock(_lock);
if (WARN_ON(video_device[vdev->minor] != vdev)) {
@@ -193,8 +196,8 @@ static void v4l2_device_release(struct device *cd)
 
mutex_unlock(_lock);
 
-#if defined(CONFIG_MEDIA_CONTROLLER)
-   if (v4l2_dev->mdev) {
+#ifdef CONFIG_MEDIA_CONTROLLER
+   if (mdev) {
/* Remove interfaces and interface links */
media_devnode_remove(vdev->intf_devnode);
if (vdev->entity.function != MEDIA_ENT_F_UNKNOWN)
@@ -220,6 +223,11 @@ static void v4l2_device_release(struct device *cd)
/* Decrease v4l2_device refcount */
if (v4l2_dev)
v4l2_device_put(v4l2_dev);
+
+#ifdef CONFIG_MEDIA_CONTROLLER
+   if (mdev)
+   media_device_put(mdev);
+#endif
 }
 
 static struct class video_class = {
@@ -813,6 +821,7 @@ static int video_register_media_controller(struct 
video_device *vdev, int type)
 
/* FIXME: how to create the other interface links? */
 
+   media_device_get(vdev->v4l2_dev->mdev);
 #endif
return 0;
 }
-- 
2.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

[RFC v4 16/21] media: Add release callback for media device

[Sakari Ailus]

The release callback may be used by the driver to signal the release of
the media device. This way the lifetime of the driver's own memory
allocations may be made dependent on that of the media device.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-device.c | 4 
 include/media/media-device.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 7d9f76d..e9dfc87 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -724,6 +724,10 @@ static void media_device_release(struct media_devnode 
*devnode)
mdev->entity_internal_idx_max = 0;
media_entity_graph_walk_cleanup(>pm_count_walk);
mutex_destroy(>graph_mutex);
+
+   if (mdev->ops && mdev->ops->release)
+   mdev->ops->release(mdev);
+
put_device(mdev->dev);
 
kfree(mdev);
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 94e96ef..81f09ed 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -56,6 +56,7 @@ struct media_entity_notify {
 struct media_device_ops {
int (*link_notify)(struct media_link *link, u32 flags,
   unsigned int notification);
+   void (*release)(struct media_device *mdev);
 };
 
 /**
-- 
2.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

[RFC v4 07/21] media-device: Make devnode.dev->kobj parent of devnode.cdev

[Sakari Ailus]

The struct cdev embedded in struct media_devnode contains its own kobj.
Instead of trying to manage its lifetime separately from struct
media_devnode, make the cdev kobj a parent of the struct media_device.dev
kobj.

The cdev will thus be released during unregistering the media_devnode, not
in media_devnode.dev kobj's release callback.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-devnode.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index 7481c96..a8302fc 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -63,9 +63,6 @@ static void media_devnode_release(struct device *cd)
 
mutex_lock(_devnode_lock);
 
-   /* Delete the cdev on this minor as well */
-   cdev_del(>cdev);
-
/* Mark device node number as free */
clear_bit(devnode->minor, media_devnode_nums);
 
@@ -241,6 +238,7 @@ int __must_check media_devnode_register(struct 
media_devnode *devnode,
 
/* Part 2: Initialize and register the character device */
cdev_init(>cdev, _devnode_fops);
+   devnode->cdev.kobj.parent = >dev.kobj;
devnode->cdev.owner = owner;
 
ret = cdev_add(>cdev, MKDEV(MAJOR(media_dev_t), 
devnode->minor), 1);
@@ -285,6 +283,7 @@ void media_devnode_unregister(struct media_devnode *devnode)
mutex_lock(_devnode_lock);
clear_bit(MEDIA_FLAG_REGISTERED, >flags);
mutex_unlock(_devnode_lock);
+   cdev_del(>cdev);
device_unregister(>dev);
 }
 
-- 
2.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

[RFC v4 15/21] media: Provide a way to the driver to set a private pointer

[Sakari Ailus]

Now that the media device can be allocated dynamically, drivers have no
longer a way to conveniently obtain the driver private data structure.
Provide one again in the form of a private pointer passed to the
media_device_alloc() function.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/media-device.c |  3 ++-
 include/media/media-device.h | 15 ++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 648c64c..7d9f76d 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -729,7 +729,7 @@ static void media_device_release(struct media_devnode 
*devnode)
kfree(mdev);
 }
 
-struct media_device *media_device_alloc(struct device *dev)
+struct media_device *media_device_alloc(struct device *dev, void *priv)
 {
struct media_device *mdev;
 
@@ -745,6 +745,7 @@ struct media_device *media_device_alloc(struct device *dev)
 
mdev->dev = dev;
media_device_init(mdev);
+   mdev->priv = priv;
 
mdev->devnode.release = media_device_release;
 
diff --git a/include/media/media-device.h b/include/media/media-device.h
index ae2bc08..94e96ef 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -62,6 +62,7 @@ struct media_device_ops {
  * struct media_device - Media device
  * @dev:   Parent device
  * @devnode:   Media device node
+ * @priv:  A pointer to driver private data
  * @driver_name: Optional device driver name. If not set, calls to
  * %MEDIA_IOC_DEVICE_INFO will return ``dev->driver->name``.
  * This is needed for USB drivers for example, as otherwise
@@ -128,6 +129,7 @@ struct media_device {
/* dev->driver_data points to this struct. */
struct device *dev;
struct media_devnode devnode;
+   void *priv;
 
char model[32];
char driver_name[32];
@@ -214,6 +216,7 @@ void media_device_init(struct media_device *mdev);
  * media_device_alloc() - Allocate and initialise a media device
  *
  * @dev:   The associated struct device pointer
+ * @priv:  pointer to a driver private data structure
  *
  * Allocate and initialise a media device. Returns a media device.
  * The media device is refcounted, and this function returns a media
@@ -222,7 +225,7 @@ void media_device_init(struct media_device *mdev);
  * References are taken and given using media_device_get() and
  * media_device_put().
  */
-struct media_device *media_device_alloc(struct device *dev);
+struct media_device *media_device_alloc(struct device *dev, void *priv);
 
 /**
  * media_device_get() - Get a reference to a media device
@@ -249,6 +252,16 @@ struct media_device *media_device_alloc(struct device 
*dev);
} while (0)
 
 /**
+ * media_device_priv() - Obtain the driver private pointer
+ *
+ * Returns a pointer passed to the media_device_alloc() function.
+ */
+static inline void *media_device_priv(struct media_device *mdev)
+{
+   return mdev->priv;
+}
+
+/**
  * media_device_cleanup() - Cleanups a media device element
  *
  * @mdev:  pointer to struct _device
-- 
2.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

[RFC v4 19/21] omap3isp: Allocate the media device dynamically

[Sakari Ailus]

Use the new media_device_alloc() API to allocate and release the media
device.

Signed-off-by: Sakari Ailus 
---
 drivers/media/platform/omap3isp/isp.c  | 24 +---
 drivers/media/platform/omap3isp/isp.h  |  2 +-
 drivers/media/platform/omap3isp/ispvideo.c |  2 +-
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index 2e1b17e..8bc7a7c 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -1601,8 +1601,8 @@ static void isp_unregister_entities(struct isp_device 
*isp)
omap3isp_stat_unregister_entities(>isp_hist);
 
v4l2_device_unregister(>v4l2_dev);
-   media_device_unregister(>media_dev);
-   media_device_cleanup(>media_dev);
+   media_device_unregister(isp->media_dev);
+   media_device_put(isp->media_dev);
 }
 
 static int isp_link_entity(
@@ -1680,14 +1680,16 @@ static int isp_register_entities(struct isp_device *isp)
 {
int ret;
 
-   isp->media_dev.dev = isp->dev;
-   strlcpy(isp->media_dev.model, "TI OMAP3 ISP",
-   sizeof(isp->media_dev.model));
-   isp->media_dev.hw_revision = isp->revision;
-   isp->media_dev.ops = _media_ops;
-   media_device_init(>media_dev);
+   isp->media_dev = media_device_alloc(isp->dev, isp);
+   if (!isp->media_dev)
+   return -ENOMEM;
+
+   strlcpy(isp->media_dev->model, "TI OMAP3 ISP",
+   sizeof(isp->media_dev->model));
+   isp->media_dev->hw_revision = isp->revision;
+   isp->media_dev->ops = _media_ops;
 
-   isp->v4l2_dev.mdev = >media_dev;
+   isp->v4l2_dev.mdev = isp->media_dev;
ret = v4l2_device_register(isp->dev, >v4l2_dev);
if (ret < 0) {
dev_err(isp->dev, "%s: V4L2 device registration failed (%d)\n",
@@ -2165,7 +2167,7 @@ static int isp_subdev_notifier_complete(struct 
v4l2_async_notifier *async)
struct isp_bus_cfg *bus;
int ret;
 
-   ret = media_entity_enum_init(>crashed, >media_dev);
+   ret = media_entity_enum_init(>crashed, isp->media_dev);
if (ret)
return ret;
 
@@ -2183,7 +2185,7 @@ static int isp_subdev_notifier_complete(struct 
v4l2_async_notifier *async)
if (ret < 0)
return ret;
 
-   return media_device_register(>media_dev);
+   return media_device_register(isp->media_dev);
 }
 
 /*
diff --git a/drivers/media/platform/omap3isp/isp.h 
b/drivers/media/platform/omap3isp/isp.h
index 7e6f663..7378279 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -176,7 +176,7 @@ struct isp_xclk {
 struct isp_device {
struct v4l2_device v4l2_dev;
struct v4l2_async_notifier notifier;
-   struct media_device media_dev;
+   struct media_device *media_dev;
struct device *dev;
u32 revision;
 
diff --git a/drivers/media/platform/omap3isp/ispvideo.c 
b/drivers/media/platform/omap3isp/ispvideo.c
index 7354469..33e74b9 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -1104,7 +1104,7 @@ isp_video_streamon(struct file *file, void *fh, enum 
v4l2_buf_type type)
pipe = video->video.entity.pipe
 ? to_isp_pipeline(>video.entity) : >pipe;
 
-   ret = media_entity_enum_init(>ent_enum, >isp->media_dev);
+   ret = media_entity_enum_init(>ent_enum, video->isp->media_dev);
if (ret)
goto err_enum_init;
 
-- 
2.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

[RFC v4 18/21] media-device: Postpone graph object removal until free

[Sakari Ailus]

The media device itself will be unregistered based on it being unbound and
driver's remove callback being called. The graph objects themselves may
still be in use; rely on the media device release callback to release
them.

Signed-off-by: Sakari Ailus 
Acked-by: Hans Verkuil 
---
 drivers/media/media-device.c | 44 
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index e9dfc87..7a5884e 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -717,6 +717,26 @@ EXPORT_SYMBOL_GPL(media_device_init);
 static void media_device_release(struct media_devnode *devnode)
 {
struct media_device *mdev = to_media_device(devnode);
+   struct media_entity *entity;
+   struct media_entity *next;
+   struct media_interface *intf, *tmp_intf;
+   struct media_entity_notify *notify, *nextp;
+
+   /* Remove all entities from the media device */
+   list_for_each_entry_safe(entity, next, >entities, graph_obj.list)
+   __media_device_unregister_entity(entity);
+
+   /* Remove all entity_notify callbacks from the media device */
+   list_for_each_entry_safe(notify, nextp, >entity_notify, list)
+   __media_device_unregister_entity_notify(mdev, notify);
+
+   /* Remove all interfaces from the media device */
+   list_for_each_entry_safe(intf, tmp_intf, >interfaces,
+graph_obj.list) {
+   __media_remove_intf_links(intf);
+   media_gobj_destroy(>graph_obj);
+   kfree(intf);
+   }
 
dev_dbg(devnode->parent, "Media device released\n");
 
@@ -797,38 +817,14 @@ EXPORT_SYMBOL_GPL(__media_device_register);
 
 void media_device_unregister(struct media_device *mdev)
 {
-   struct media_entity *entity;
-   struct media_entity *next;
-   struct media_interface *intf, *tmp_intf;
-   struct media_entity_notify *notify, *nextp;
-
if (mdev == NULL)
return;
 
mutex_lock(>graph_mutex);
-
-   /* Check if mdev was ever registered at all */
if (!media_devnode_is_registered(>devnode)) {
mutex_unlock(>graph_mutex);
return;
}
-
-   /* Remove all entities from the media device */
-   list_for_each_entry_safe(entity, next, >entities, graph_obj.list)
-   __media_device_unregister_entity(entity);
-
-   /* Remove all entity_notify callbacks from the media device */
-   list_for_each_entry_safe(notify, nextp, >entity_notify, list)
-   __media_device_unregister_entity_notify(mdev, notify);
-
-   /* Remove all interfaces from the media device */
-   list_for_each_entry_safe(intf, tmp_intf, >interfaces,
-graph_obj.list) {
-   __media_remove_intf_links(intf);
-   media_gobj_destroy(>graph_obj);
-   kfree(intf);
-   }
-
mutex_unlock(>graph_mutex);
 
device_remove_file(>devnode.dev, _attr_model);
-- 
2.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

[RFC v4 01/21] Revert "[media] media: fix media devnode ioctl/syscall and unregister race"

[Sakari Ailus]

This reverts commit 6f0dd24a084a ("[media] media: fix media devnode
ioctl/syscall and unregister race"). The commit was part of an original
patchset to avoid crashes when an unregistering device is in use.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-device.c  | 15 +++
 drivers/media/media-devnode.c |  8 +---
 include/media/media-devnode.h | 16 ++--
 3 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 2783531..f2525eb 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -730,7 +730,6 @@ int __must_check __media_device_register(struct 
media_device *mdev,
if (ret < 0) {
/* devnode free is handled in media_devnode_*() */
mdev->devnode = NULL;
-   media_devnode_unregister_prepare(devnode);
media_devnode_unregister(devnode);
return ret;
}
@@ -787,9 +786,6 @@ void media_device_unregister(struct media_device *mdev)
return;
}
 
-   /* Clear the devnode register bit to avoid races with media dev open */
-   media_devnode_unregister_prepare(mdev->devnode);
-
/* Remove all entities from the media device */
list_for_each_entry_safe(entity, next, >entities, graph_obj.list)
__media_device_unregister_entity(entity);
@@ -810,10 +806,13 @@ void media_device_unregister(struct media_device *mdev)
 
dev_dbg(mdev->dev, "Media device unregistered\n");
 
-   device_remove_file(>devnode->dev, _attr_model);
-   media_devnode_unregister(mdev->devnode);
-   /* devnode free is handled in media_devnode_*() */
-   mdev->devnode = NULL;
+   /* Check if mdev devnode was registered */
+   if (media_devnode_is_registered(mdev->devnode)) {
+   device_remove_file(>devnode->dev, _attr_model);
+   media_devnode_unregister(mdev->devnode);
+   /* devnode free is handled in media_devnode_*() */
+   mdev->devnode = NULL;
+   }
 }
 EXPORT_SYMBOL_GPL(media_device_unregister);
 
diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index f2772ba..5b605ff 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -287,7 +287,7 @@ int __must_check media_devnode_register(struct media_device 
*mdev,
return ret;
 }
 
-void media_devnode_unregister_prepare(struct media_devnode *devnode)
+void media_devnode_unregister(struct media_devnode *devnode)
 {
/* Check if devnode was ever registered at all */
if (!media_devnode_is_registered(devnode))
@@ -295,12 +295,6 @@ void media_devnode_unregister_prepare(struct media_devnode 
*devnode)
 
mutex_lock(_devnode_lock);
clear_bit(MEDIA_FLAG_REGISTERED, >flags);
-   mutex_unlock(_devnode_lock);
-}
-
-void media_devnode_unregister(struct media_devnode *devnode)
-{
-   mutex_lock(_devnode_lock);
/* Delete the cdev on this minor as well */
cdev_del(>cdev);
mutex_unlock(_devnode_lock);
diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
index cd23e91..d55ec2b 100644
--- a/include/media/media-devnode.h
+++ b/include/media/media-devnode.h
@@ -128,26 +128,14 @@ int __must_check media_devnode_register(struct 
media_device *mdev,
struct module *owner);
 
 /**
- * media_devnode_unregister_prepare - clear the media device node register bit
- * @devnode: the device node to prepare for unregister
- *
- * This clears the passed device register bit. Future open calls will be met
- * with errors. Should be called before media_devnode_unregister() to avoid
- * races with unregister and device file open calls.
- *
- * This function can safely be called if the device node has never been
- * registered or has already been unregistered.
- */
-void media_devnode_unregister_prepare(struct media_devnode *devnode);
-
-/**
  * media_devnode_unregister - unregister a media device node
  * @devnode: the device node to unregister
  *
  * This unregisters the passed device. Future open calls will be met with
  * errors.
  *
- * Should be called after media_devnode_unregister_prepare()
+ * This function can safely be called if the device node has never been
+ * registered or has already been unregistered.
  */
 void media_devnode_unregister(struct media_devnode *devnode);
 
-- 
2.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

[RFC v4 02/21] Revert "[media] media: fix use-after-free in cdev_put() when app exits after driver unbind"

[Sakari Ailus]

This reverts commit 5b28dde51d0c ("[media] media: fix use-after-free in
cdev_put() when app exits after driver unbind"). The commit was part of an
original patchset to avoid crashes when an unregistering device is in use.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-device.c  |  6 ++
 drivers/media/media-devnode.c | 48 +--
 2 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index f2525eb..6f5ed09 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -721,16 +721,16 @@ int __must_check __media_device_register(struct 
media_device *mdev,
 
ret = media_devnode_register(mdev, devnode, owner);
if (ret < 0) {
-   /* devnode free is handled in media_devnode_*() */
mdev->devnode = NULL;
+   kfree(devnode);
return ret;
}
 
ret = device_create_file(>dev, _attr_model);
if (ret < 0) {
-   /* devnode free is handled in media_devnode_*() */
mdev->devnode = NULL;
media_devnode_unregister(devnode);
+   kfree(devnode);
return ret;
}
 
@@ -810,8 +810,6 @@ void media_device_unregister(struct media_device *mdev)
if (media_devnode_is_registered(mdev->devnode)) {
device_remove_file(>devnode->dev, _attr_model);
media_devnode_unregister(mdev->devnode);
-   /* devnode free is handled in media_devnode_*() */
-   mdev->devnode = NULL;
}
 }
 EXPORT_SYMBOL_GPL(media_device_unregister);
diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index 5b605ff..ecdc02d 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -63,8 +63,13 @@ static void media_devnode_release(struct device *cd)
struct media_devnode *devnode = to_media_devnode(cd);
 
mutex_lock(_devnode_lock);
+
+   /* Delete the cdev on this minor as well */
+   cdev_del(>cdev);
+
/* Mark device node number as free */
clear_bit(devnode->minor, media_devnode_nums);
+
mutex_unlock(_devnode_lock);
 
/* Release media_devnode and perform other cleanups as needed. */
@@ -72,7 +77,6 @@ static void media_devnode_release(struct device *cd)
devnode->release(devnode);
 
kfree(devnode);
-   pr_debug("%s: Media Devnode Deallocated\n", __func__);
 }
 
 static struct bus_type media_bus_type = {
@@ -201,8 +205,6 @@ static int media_release(struct inode *inode, struct file 
*filp)
/* decrease the refcount unconditionally since the release()
   return value is ignored. */
put_device(>dev);
-
-   pr_debug("%s: Media Release\n", __func__);
return 0;
 }
 
@@ -233,7 +235,6 @@ int __must_check media_devnode_register(struct media_device 
*mdev,
if (minor == MEDIA_NUM_DEVICES) {
mutex_unlock(_devnode_lock);
pr_err("could not get a free minor\n");
-   kfree(devnode);
return -ENFILE;
}
 
@@ -243,31 +244,27 @@ int __must_check media_devnode_register(struct 
media_device *mdev,
devnode->minor = minor;
devnode->media_dev = mdev;
 
-   /* Part 1: Initialize dev now to use dev.kobj for cdev.kobj.parent */
-   devnode->dev.bus = _bus_type;
-   devnode->dev.devt = MKDEV(MAJOR(media_dev_t), devnode->minor);
-   devnode->dev.release = media_devnode_release;
-   if (devnode->parent)
-   devnode->dev.parent = devnode->parent;
-   dev_set_name(>dev, "media%d", devnode->minor);
-   device_initialize(>dev);
-
/* Part 2: Initialize and register the character device */
cdev_init(>cdev, _devnode_fops);
devnode->cdev.owner = owner;
-   devnode->cdev.kobj.parent = >dev.kobj;
 
ret = cdev_add(>cdev, MKDEV(MAJOR(media_dev_t), 
devnode->minor), 1);
if (ret < 0) {
pr_err("%s: cdev_add failed\n", __func__);
-   goto cdev_add_error;
+   goto error;
}
 
-   /* Part 3: Add the media device */
-   ret = device_add(>dev);
+   /* Part 3: Register the media device */
+   devnode->dev.bus = _bus_type;
+   devnode->dev.devt = MKDEV(MAJOR(media_dev_t), devnode->minor);
+   devnode->dev.release = media_devnode_release;
+   if (devnode->parent)
+   devnode->dev.parent = devnode->parent;
+   dev_set_name(>dev, "media%d", devnode->minor);
+   ret = device_register(>dev);
if (ret < 0) {
-   pr_err("%s: device_add failed\n", __func__);
-   goto device_add_error;
+   pr_err("%s: device_register failed\n", __func__);
+   goto error;
}
 
/* Part 4: Activate this minor. The char device can now be used. */
@@ -275,15 

[RFC v4 21/21] omap3isp: Don't rely on devm for memory resource management

[Sakari Ailus]

devm functions are fine for managing resources that are directly related
to the device at hand and that have no other dependencies. However, a
process holding a file handle to a device created by a driver for a device
may result in the file handle left behind after the device is long gone.
This will result in accessing released (and potentially reallocated)
memory.

Instead, rely on the media device which will stick around until all users
are gone.

Signed-off-by: Sakari Ailus 
---
 drivers/media/platform/omap3isp/isp.c | 38 ---
 drivers/media/platform/omap3isp/ispccp2.c |  3 ++-
 drivers/media/platform/omap3isp/isph3a_aewb.c | 20 +-
 drivers/media/platform/omap3isp/isph3a_af.c   | 19 ++
 drivers/media/platform/omap3isp/isphist.c |  5 ++--
 drivers/media/platform/omap3isp/ispstat.c |  2 ++
 6 files changed, 63 insertions(+), 24 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index 54b84a0..5777c55 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -1377,7 +1377,7 @@ static int isp_get_clocks(struct isp_device *isp)
unsigned int i;
 
for (i = 0; i < ARRAY_SIZE(isp_clocks); ++i) {
-   clk = devm_clk_get(isp->dev, isp_clocks[i]);
+   clk = clk_get(isp->dev, isp_clocks[i]);
if (IS_ERR(clk)) {
dev_err(isp->dev, "clk_get %s failed\n", isp_clocks[i]);
return PTR_ERR(clk);
@@ -1389,6 +1389,14 @@ static int isp_get_clocks(struct isp_device *isp)
return 0;
 }
 
+static void isp_put_clocks(struct isp_device *isp)
+{
+   unsigned int i;
+
+   for (i = 0; i < ARRAY_SIZE(isp_clocks); ++i)
+   clk_put(isp->clock[i]);
+}
+
 /*
  * omap3isp_get - Acquire the ISP resource.
  *
@@ -1603,7 +1611,6 @@ static void isp_unregister_entities(struct isp_device 
*isp)
omap3isp_stat_unregister_entities(>isp_af);
omap3isp_stat_unregister_entities(>isp_hist);
 
-   v4l2_device_unregister(>v4l2_dev);
media_device_unregister(isp->media_dev);
media_device_put(isp->media_dev);
 }
@@ -1955,6 +1962,8 @@ static void isp_release(struct media_device *mdev)
 {
struct isp_device *isp = media_device_priv(mdev);
 
+   v4l2_device_unregister(>v4l2_dev);
+
isp_cleanup_modules(isp);
isp_xclk_cleanup(isp);
 
@@ -1963,6 +1972,10 @@ static void isp_release(struct media_device *mdev)
__omap3isp_put(isp, false);
 
media_entity_enum_cleanup(>crashed);
+
+   isp_put_clocks(isp);
+
+   kfree(isp);
 }
 
 static int isp_attach_iommu(struct isp_device *isp)
@@ -2215,7 +2228,7 @@ static int isp_probe(struct platform_device *pdev)
int ret;
int i, m;
 
-   isp = devm_kzalloc(>dev, sizeof(*isp), GFP_KERNEL);
+   isp = kzalloc(sizeof(*isp), GFP_KERNEL);
if (!isp) {
dev_err(>dev, "could not allocate memory\n");
return -ENOMEM;
@@ -2224,21 +2237,23 @@ static int isp_probe(struct platform_device *pdev)
ret = of_property_read_u32(pdev->dev.of_node, "ti,phy-type",
   >phy_type);
if (ret)
-   return ret;
+   goto error_release_isp;
 
isp->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
  "syscon");
-   if (IS_ERR(isp->syscon))
-   return PTR_ERR(isp->syscon);
+   if (IS_ERR(isp->syscon)) {
+   ret = PTR_ERR(isp->syscon);
+   goto error_release_isp;
+   }
 
ret = of_property_read_u32_index(pdev->dev.of_node, "syscon", 1,
 >syscon_offset);
if (ret)
-   return ret;
+   goto error_release_isp;
 
ret = isp_of_parse_nodes(>dev, >notifier);
if (ret < 0)
-   return ret;
+   goto error_release_isp;
 
isp->autoidle = autoidle;
 
@@ -2255,8 +2270,8 @@ static int isp_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, isp);
 
/* Regulators */
-   isp->isp_csiphy1.vdd = devm_regulator_get(>dev, "vdd-csiphy1");
-   isp->isp_csiphy2.vdd = devm_regulator_get(>dev, "vdd-csiphy2");
+   isp->isp_csiphy1.vdd = regulator_get(>dev, "vdd-csiphy1");
+   isp->isp_csiphy2.vdd = regulator_get(>dev, "vdd-csiphy2");
 
/* Clocks
 *
@@ -2388,6 +2403,9 @@ static int isp_probe(struct platform_device *pdev)
__omap3isp_put(isp, false);
 error:
mutex_destroy(>isp_mutex);
+   isp_put_clocks(isp);
+error_release_isp:
+   kfree(isp);
 
return ret;
 }
diff --git a/drivers/media/platform/omap3isp/ispccp2.c 
b/drivers/media/platform/omap3isp/ispccp2.c
index ca09523..d49ce8a 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ 

[RFC v4 00/21] Make use of kref in media device, grab references as needed

[Sakari Ailus]

Hi folks,

This is the third version of the RFC set to fix referencing in media
devices.

The lifetime of the media device (and media devnode) is now bound to that
of struct device embedded in it and its memory is only released once the
last reference is gone: unregistering is simply unregistering, it no
longer should release memory which could be further accessed.

A video node or a sub-device node also gets a reference to the media
device, i.e. the release function of the video device node will release
its reference to the media device. The same goes for file handles to the
media device.

As a side effect of this is that the media device, it is allocate together
with the media devnode. The driver may also rely its own resources to the
media device. Alternatively there's also a priv field to hold drivers
private pointer (for container_of() is an option in this case). We could
drop one of these options but currently both are possible.

I've tested this by manually unbinding the omap3isp platform device while
streaming. Driver changes are required for this to work; by not using
dynamic allocation (i.e. media_device_alloc()) the old behaviour is still
supported. This is still unlikely to be a grave problem as there are not
that many device drivers that support physically removable devices. We've
had this problem for other devices for many years without paying much
notice --- that doesn't mean I don't think at least drivers for removable
devices shouldn't be changed as part of the set later on, I'd just like to
get review comments on the approach first.

The three patches that originally partially resolved some of these issues
are reverted in the beginning of the set. I'm still posting this as an RFC
mainly since the testing is somewhat limited so far.

A sample of what happens if streaming is stopped while the device is being
removed during streaming without this set:

-8<--
[  288.911560] omap3isp 480bc000.isp: media_gobj_destroy id 98: interface link 
id 97 ==> id 8
[  288.920349] omap3isp 480bc000.isp: media_gobj_destroy id 52: data link id 10 
==> id 12
[  288.928741] omap3isp 480bc000.isp: media_gobj_destroy id 51: data link id 10 
==> id 12
[  288.937103] omap3isp 480bc000.isp: media_gobj_destroy id 66: data link id 10 
==> id 16
[  288.945465] omap3isp 480bc000.isp: media_gobj_destroy id 65: data link id 10 
==> id 16
[  288.953796] omap3isp 480bc000.isp: media_gobj_destroy id 9: sink pad 'OMAP3 
ISP CSI2a':0
[  288.962341] omap3isp 480bc000.isp: media_gobj_destroy id 10: source pad 
'OMAP3 ISP CSI2a':1
[  288.971160] omap3isp 480bc000.isp: media_gobj_destroy id 8: entity 'OMAP3 
ISP CSI2a'
[  289.117187] omap3isp 480bc000.isp: media_gobj_destroy id 97: intf_devnode 
v4l-subdev - major: 81, minor: 8
[  289.159820] omap3isp 480bc000.isp: media_gobj_destroy id 96: interface link 
id 95 ==> id 1
[  289.168640] omap3isp 480bc000.isp: media_gobj_destroy id 53: data link id 5 
==> id 2
[  289.176849] omap3isp 480bc000.isp: media_gobj_destroy id 54: data link id 5 
==> id 2
[  289.185058] omap3isp 480bc000.isp: media_gobj_destroy id 68: data link id 3 
==> id 16
[  289.193298] omap3isp 480bc000.isp: media_gobj_destroy id 67: data link id 3 
==> id 16
[  289.201568] omap3isp 480bc000.isp: media_gobj_destroy id 2: sink pad 'OMAP3 
ISP CCP2':0
[  289.210021] omap3isp 480bc000.isp: media_gobj_destroy id 3: source pad 
'OMAP3 ISP CCP2':1
[  289.218658] omap3isp 480bc000.isp: media_gobj_destroy id 1: entity 'OMAP3 
ISP CCP2'
[  289.402587] omap3isp 480bc000.isp: media_gobj_destroy id 95: intf_devnode 
v4l-subdev - major: 81, minor: 7
[  289.448974] omap3isp 480bc000.isp: media_gobj_destroy id 7: interface link 
id 6 ==> id 4
[  289.457641] omap3isp 480bc000.isp: media_gobj_destroy id 6: intf_devnode 
v4l-video - major: 81, minor: 0
[  289.467773] omap3isp 480bc000.isp: media_gobj_destroy id 5: source pad 
'OMAP3 ISP CCP2 input':0
[  289.476959] omap3isp 480bc000.isp: media_gobj_destroy id 4: entity 'OMAP3 
ISP CCP2 input'
[  289.485595] omap3isp 480bc000.isp: media_gobj_destroy id 100: interface link 
id 99 ==> id 15
[  289.494537] omap3isp 480bc000.isp: media_gobj_destroy id 56: data link id 17 
==> id 20
[  289.502868] omap3isp 480bc000.isp: media_gobj_destroy id 55: data link id 17 
==> id 20
[  289.511230] omap3isp 480bc000.isp: media_gobj_destroy id 70: data link id 18 
==> id 24
[  289.519592] omap3isp 480bc000.isp: media_gobj_destroy id 69: data link id 18 
==> id 24
[  289.527954] omap3isp 480bc000.isp: media_gobj_destroy id 72: data link id 17 
==> id 35
[  289.536315] omap3isp 480bc000.isp: media_gobj_destroy id 71: data link id 17 
==> id 35
[  

[RFC v4 09/21] media: Split initialising and adding media devnode

[Sakari Ailus]

As registering a device node of an entity belonging to a media device
will require a reference to the struct device. Taking that reference is
only possible once the device has been initialised, which took place only
when it was registered. Split this in two, and initialise the device when
the media device is allocated.

Don't distribute the effects of these changes yet. Add media_device_get()
and media_device_put() first.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-device.c  | 18 +-
 drivers/media/media-devnode.c | 17 +++--
 include/media/media-devnode.h | 19 ++-
 3 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index 496195e..c79a9f5 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -720,19 +720,26 @@ int __must_check __media_device_register(struct 
media_device *mdev,
/* Set version 0 to indicate user-space that the graph is static */
mdev->topology_version = 0;
 
+   media_devnode_init(>devnode);
+
ret = media_devnode_register(>devnode, owner);
if (ret < 0)
-   return ret;
+   goto out_put;
 
ret = device_create_file(>devnode.dev, _attr_model);
-   if (ret < 0) {
-   media_devnode_unregister(>devnode);
-   return ret;
-   }
+   if (ret < 0)
+   goto out_unregister;
 
dev_dbg(mdev->dev, "Media device registered\n");
 
return 0;
+
+out_unregister:
+   media_devnode_unregister(>devnode);
+out_put:
+   put_device(>devnode.dev);
+
+   return ret;
 }
 EXPORT_SYMBOL_GPL(__media_device_register);
 
@@ -803,6 +810,7 @@ void media_device_unregister(struct media_device *mdev)
device_remove_file(>devnode.dev, _attr_model);
dev_dbg(mdev->dev, "Media device unregistering\n");
media_devnode_unregister(>devnode);
+   put_device(>devnode.dev);
 }
 EXPORT_SYMBOL_GPL(media_device_unregister);
 
diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
index a8302fc..178d692 100644
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -216,6 +216,11 @@ static const struct file_operations media_devnode_fops = {
.llseek = no_llseek,
 };
 
+void media_devnode_init(struct media_devnode *devnode)
+{
+   device_initialize(>dev);
+}
+
 int __must_check media_devnode_register(struct media_devnode *devnode,
struct module *owner)
 {
@@ -254,7 +259,7 @@ int __must_check media_devnode_register(struct 
media_devnode *devnode,
if (devnode->parent)
devnode->dev.parent = devnode->parent;
dev_set_name(>dev, "media%d", devnode->minor);
-   ret = device_register(>dev);
+   ret = device_add(>dev);
if (ret < 0) {
pr_err("%s: device_register failed\n", __func__);
goto error;
@@ -284,13 +289,13 @@ void media_devnode_unregister(struct media_devnode 
*devnode)
clear_bit(MEDIA_FLAG_REGISTERED, >flags);
mutex_unlock(_devnode_lock);
cdev_del(>cdev);
-   device_unregister(>dev);
+   device_del(>dev);
 }
 
 /*
  * Initialise media for linux
  */
-static int __init media_devnode_init(void)
+static int __init media_devnode_module_init(void)
 {
int ret;
 
@@ -312,14 +317,14 @@ static int __init media_devnode_init(void)
return 0;
 }
 
-static void __exit media_devnode_exit(void)
+static void __exit media_devnode_module_exit(void)
 {
bus_unregister(_bus_type);
unregister_chrdev_region(media_dev_t, MEDIA_NUM_DEVICES);
 }
 
-subsys_initcall(media_devnode_init);
-module_exit(media_devnode_exit)
+subsys_initcall(media_devnode_module_init);
+module_exit(media_devnode_module_exit)
 
 MODULE_AUTHOR("Laurent Pinchart ");
 MODULE_DESCRIPTION("Device node registration for media drivers");
diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
index a68b4b6..ffa2d18 100644
--- a/include/media/media-devnode.h
+++ b/include/media/media-devnode.h
@@ -103,6 +103,17 @@ struct media_devnode {
 #define to_media_devnode(cd) container_of(cd, struct media_devnode, dev)
 
 /**
+ * media_devnode_init - initialise a media devnode
+ *
+ * @devnode: struct media_devnode we want to initialise
+ *
+ * Initialise a media devnode. Note that after initialising the media
+ * devnode is refcounted. Releasing references to it may be done using
+ * put_device().
+ */
+void media_devnode_init(struct media_devnode *devnode);
+
+/**
  * media_devnode_register - register a media device node
  *
  * @devnode: struct media_devnode we want to register a device node
@@ -112,11 +123,9 @@ struct media_devnode {
  * with the kernel. An error is returned if no free minor number can be found,
  * or if the registration of the device node fails.
  *
- * Zero is 

[RFC v4 08/21] media: Enable allocating the media device dynamically

[Sakari Ailus]

From: Sakari Ailus 

Allow allocating the media device dynamically. As the struct media_device
embeds struct media_devnode, the lifetime of that object is that same than
that of the media_device.

Signed-off-by: Sakari Ailus 
---
 drivers/media/media-device.c | 15 +++
 include/media/media-device.h | 13 +
 2 files changed, 28 insertions(+)

diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index a31329d..496195e 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -684,6 +684,21 @@ void media_device_init(struct media_device *mdev)
 }
 EXPORT_SYMBOL_GPL(media_device_init);
 
+struct media_device *media_device_alloc(struct device *dev)
+{
+   struct media_device *mdev;
+
+   mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
+   if (!mdev)
+   return NULL;
+
+   mdev->dev = dev;
+   media_device_init(mdev);
+
+   return mdev;
+}
+EXPORT_SYMBOL_GPL(media_device_alloc);
+
 void media_device_cleanup(struct media_device *mdev)
 {
ida_destroy(>entity_internal_idx);
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 96de915..c9b5798 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -207,6 +207,15 @@ static inline __must_check int media_entity_enum_init(
 void media_device_init(struct media_device *mdev);
 
 /**
+ * media_device_alloc() - Allocate and initialise a media device
+ *
+ * @dev:   The associated struct device pointer
+ *
+ * Allocate and initialise a media device. Returns a media device.
+ */
+struct media_device *media_device_alloc(struct device *dev);
+
+/**
  * media_device_cleanup() - Cleanups a media device element
  *
  * @mdev:  pointer to struct _device
@@ -451,6 +460,10 @@ void __media_device_usb_init(struct media_device *mdev,
 const char *driver_name);
 
 #else
+static inline struct media_device *media_device_alloc(struct device *dev)
+{
+   return NULL;
+}
 static inline int media_device_register(struct media_device *mdev)
 {
return 0;
-- 
2.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

[RFC v4 20/21] omap3isp: Release the isp device struct by media device callback

[Sakari Ailus]

Use the media device release callback to release the isp device's data
structure. This approach has the benefit of not releasing memory which may
still be accessed through open file handles whilst the isp driver is being
unbound.

Signed-off-by: Sakari Ailus 
---
 drivers/media/platform/omap3isp/isp.c | 25 +
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index 8bc7a7c..54b84a0 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -657,8 +657,11 @@ static irqreturn_t isp_isr(int irq, void *_isp)
return IRQ_HANDLED;
 }
 
+static void isp_release(struct media_device *mdev);
+
 static const struct media_device_ops isp_media_ops = {
.link_notify = v4l2_pipeline_link_notify,
+   .release = isp_release,
 };
 
 /* 
-
@@ -1948,6 +1951,20 @@ static void isp_detach_iommu(struct isp_device *isp)
iommu_group_remove_device(isp->dev);
 }
 
+static void isp_release(struct media_device *mdev)
+{
+   struct isp_device *isp = media_device_priv(mdev);
+
+   isp_cleanup_modules(isp);
+   isp_xclk_cleanup(isp);
+
+   __omap3isp_get(isp, false);
+   isp_detach_iommu(isp);
+   __omap3isp_put(isp, false);
+
+   media_entity_enum_cleanup(>crashed);
+}
+
 static int isp_attach_iommu(struct isp_device *isp)
 {
struct dma_iommu_mapping *mapping;
@@ -2008,14 +2025,6 @@ static int isp_remove(struct platform_device *pdev)
 
v4l2_async_notifier_unregister(>notifier);
isp_unregister_entities(isp);
-   isp_cleanup_modules(isp);
-   isp_xclk_cleanup(isp);
-
-   __omap3isp_get(isp, false);
-   isp_detach_iommu(isp);
-   __omap3isp_put(isp, false);
-
-   media_entity_enum_cleanup(>crashed);
 
return 0;
 }
-- 
2.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

[PATCH 1/1] media: entity: Add media_entity_has_route() function

[Sakari Ailus]

From: Laurent Pinchart 

This is a wrapper around the media entity has_route operation.

Signed-off-by: Laurent Pinchart 
Signed-off-by: Michal Simek 
Signed-off-by: Sakari Ailus 
---
Hi Niklas,

There was actually another problem with the Kerneldoc comment related to
the mutex. Fixed that one as well.

Kind regards,
Sakari

 drivers/media/media-entity.c | 16 
 include/media/media-entity.h | 17 +
 2 files changed, 33 insertions(+)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 5734bb9..7de08e1 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -242,6 +242,22 @@ EXPORT_SYMBOL_GPL(media_entity_pads_init);
  * Graph traversal
  */
 
+bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
+   unsigned int pad1)
+{
+   if (pad0 >= entity->num_pads || pad1 >= entity->num_pads)
+   return false;
+
+   if (pad0 == pad1)
+   return true;
+
+   if (!entity->ops || !entity->ops->has_route)
+   return true;
+
+   return entity->ops->has_route(entity, pad0, pad1);
+}
+EXPORT_SYMBOL_GPL(media_entity_has_route);
+
 static struct media_entity *
 media_entity_other(struct media_entity *entity, struct media_link *link)
 {
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 2060e48..aa8d3c5 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -834,6 +834,23 @@ __must_check int media_entity_graph_walk_init(
struct media_entity_graph *graph, struct media_device *mdev);
 
 /**
+ * media_entity_has_route - Check if two entity pads are connected internally
+ *
+ * @entity: The entity
+ * @pad0: The first pad index
+ * @pad1: The second pad index
+ *
+ * This function can be used to check whether two pads of an entity are
+ * connected internally in the entity.
+ *
+ * The caller must hold entity->graph_obj.mdev->mutex.
+ *
+ * Return: true if the pads are connected internally and false otherwise.
+ */
+bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
+   unsigned int pad1);
+
+/**
  * media_entity_graph_walk_cleanup - Release resources used by graph walk.
  *
  * @graph: Media graph structure that will be used to walk the graph
-- 
2.7.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 01/32] media: entity: Add has_route entity operation

[Sakari Ailus]

On Wed, Nov 02, 2016 at 02:22:58PM +0100, Niklas Söderlund wrote:
> From: Laurent Pinchart 
> 
> The optional operation can be used by entities to report whether two
> pads are internally connected.
> 
> Signed-off-by: Laurent Pinchart 
> Signed-off-by: Michal Simek 
> Signed-off-by: Niklas Söderlund 

Thanks!

Acked-by: Sakari Ailus 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/32] media: entity: Add media_entity_has_route() function

[Sakari Ailus]

Hi Niklas,

On Wed, Nov 02, 2016 at 02:22:59PM +0100, Niklas Söderlund wrote:
> From: Laurent Pinchart 
> 
> This is a wrapper around the media entity has_route operation.
> 
> Signed-off-by: Laurent Pinchart 
> Signed-off-by: Michal Simek 
> Signed-off-by: Niklas Söderlund 
> ---
>  drivers/media/media-entity.c | 29 +
>  include/media/media-entity.h |  3 +++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index c68239e..4d03ea7 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -242,6 +242,35 @@ EXPORT_SYMBOL_GPL(media_entity_pads_init);
>   * Graph traversal
>   */
>  
> +/**
> + * media_entity_has_route - Check if two entity pads are connected internally
> + * @entity: The entity
> + * @pad0: The first pad index
> + * @pad1: The second pad index
> + *
> + * This function can be used to check whether two pads of an entity are
> + * connected internally in the entity.
> + *
> + * The caller must hold entity->source->parent->mutex.
> + *
> + * Return: true if the pads are connected internally and false otherwise.
> + */
> +bool media_entity_has_route(struct media_entity *entity, unsigned int pad0,
> + unsigned int pad1)
> +{
> + if (pad0 >= entity->num_pads || pad1 >= entity->num_pads)
> + return false;
> +
> + if (pad0 == pad1)
> + return true;
> +
> + if (!entity->ops || !entity->ops->has_route)
> + return true;
> +
> + return entity->ops->has_route(entity, pad0, pad1);
> +}
> +EXPORT_SYMBOL_GPL(media_entity_has_route);
> +
>  static struct media_entity *
>  media_entity_other(struct media_entity *entity, struct media_link *link)
>  {
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 8f9fc85..5fb3f06 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -851,6 +851,9 @@ void media_entity_graph_walk_cleanup(struct 
> media_entity_graph *graph);
>   */
>  void media_entity_put(struct media_entity *entity);
>  
> +bool media_entity_has_route(struct media_entity *entity, unsigned int sink,
> + unsigned int source);

The Kerneldoc documentation should be found here, not in the .c file.

Also the arguments are different from the actual implementation.

That's the diff to what I happen to have here, feel free to use instead:



> +
>  /**
>   * media_entity_graph_walk_start - Start walking the media graph at a
>   *   given entity

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: fix uninitialized variable warning in dib0700_rc_urb_completion()

[Sean Young]

On Mon, Nov 07, 2016 at 08:41:12AM -0700, Shuah Khan wrote:
> Fix the following uninitialized variable compiler warning:
> 
> drivers/media/usb/dvb-usb/dib0700_core.c: In function 
> ‘dib0700_rc_urb_completion’:
>  drivers/media/usb/dvb-usb/dib0700_core.c:763:2: warning: ‘protocol’ may be 
> used uninitialized in this function [-Wmaybe-uninitialized]
>rc_keydown(d->rc_dev, protocol, keycode, toggle);
>^~~~
> 
> Signed-off-by: Shuah Khan 
> ---
>  drivers/media/usb/dvb-usb/dib0700_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c 
> b/drivers/media/usb/dvb-usb/dib0700_core.c
> index f319665..cfe28ec 100644
> --- a/drivers/media/usb/dvb-usb/dib0700_core.c
> +++ b/drivers/media/usb/dvb-usb/dib0700_core.c
> @@ -676,7 +676,7 @@ static void dib0700_rc_urb_completion(struct urb *purb)
>  {
>   struct dvb_usb_device *d = purb->context;
>   struct dib0700_rc_response *poll_reply;
> - enum rc_type protocol;
> + enum rc_type protocol = RC_TYPE_UNKNOWN;
>   u32 uninitialized_var(keycode);
>   u8 toggle;
>  

There is another (better) fix for it here:

https://patchwork.linuxtv.org/patch/37516/


Thanks
Sean
--
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: [Ksummit-discuss] Including images on Sphinx documents

[Mauro Carvalho Chehab]

Em Mon, 7 Nov 2016 09:05:05 -0800
Josh Triplett  escreveu:

> On Mon, Nov 07, 2016 at 09:46:48AM -0200, Mauro Carvalho Chehab wrote:
> > That's said, PNG also doesn't seem to work fine on Sphinx 1.4.x.
> > 
> > On my tests, I installed *all* texlive extensions on Fedora 24, to
> > be sure that the issue is not the lack of some extension[1], with:
> > 
> > # dnf install $(sudo dnf search texlive |grep all|cut -d. -f 1|grep 
> > texlive-)
> > 
> > When running LaTeX in interactive mode, building just the media
> > PDF file with:
> > 
> > $ cls;make cleandocs; make SPHINXOPTS="-j5" DOCBOOKS="" 
> > SPHINXDIRS=media latexdocs 
> > $ PDFLATEX=xelatex LATEXOPTS="-interaction=interactive" -C 
> > Documentation/output/media/latex
> > 
> > I get this:
> > 
> > LaTeX Warning: Hyper reference `uapi/v4l/subdev-formats:bayer-patterns' 
> > on page
> >  153 undefined on input line 21373.
> > 
> >  [153]
> > ! Extra alignment tab has been changed to \cr.
> >  \endtemplate 
> > 
> > l.21429 \unskip}\relax \unskip}
> >\relax \\
> > ? 
> > 
> > This patch fixes the issue:
> > 
> > https://git.linuxtv.org/mchehab/experimental.git/commit/?h=dirty-pdf=b709de415f34d77cc121cad95bece9c7ef4d12fd
> > 
> > That means that Sphinx is not generating the right LaTeX output even for
> > (some?) PNG images.  
> 
> \includegraphics normally works just fine for PNG images in PDF
> documents.

I didn't try to fix the Sphinx output in LaTeX format when a PNG
image is used, but, from the above log, clearly it did something
wrong. Perhaps there's something bad defined at the sphinx.sty file,
or it simply generates the wrong LaTeX code when the image
is not on PDF format.

> [...]
> > And it may even require "--shell-escape" to be passed at the xelatex
> > call if inkscape is not in the path, with seems to be a strong
> > indication that SVG support is not native to texlive, but, instead,
> > just a way to make LaTeX to call inkscape to do the image conversion.  
> 
> Please don't require --shell-escape as part of the TeX workflow.  If
> LaTeX can't handle the desired image format natively, it needs
> conversion in advance.

Agreed. I sent a patch series to linux-doc, doing the conversion in
advance:
https://marc.info/?l=linux-doc=147859902804144=2

Not sure why, but the archives don't have all patches yet.
Anyway, the relevant one is this:

https://git.linuxtv.org/mchehab/experimental.git/commit/?h=pdf-fixes=5d41c452c787f6a6c755a3855312435bc439acb8

It basically calls ImageMagick "convert" tool for all png and
pdf files currently at the documentation (they're all at media,
ATM).


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

[PATCH 1/1] media: entity: Swap pads if route is checked from source to sink

[Sakari Ailus]

This way the pads are always passed to the has_route() op sink pad first.
Makes sense.

Signed-off-by: Sakari Ailus 
---
Hi Niklas,

This should make it easier to implement the has_route() op in drivers.

Feel free to merge this to "[PATCH 02/32] media: entity: Add
media_entity_has_route() function" if you like, or add separately after
the second patch.

 drivers/media/media-entity.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
index 747adcb..520f3f6 100644
--- a/drivers/media/media-entity.c
+++ b/drivers/media/media-entity.c
@@ -254,6 +254,10 @@ bool media_entity_has_route(struct media_entity *entity, 
unsigned int pad0,
if (!entity->ops || !entity->ops->has_route)
return true;
 
+   if (entity->pads[pad0].flags & MEDIA_PAD_FL_SOURCE
+   && entity->pads[pad1].flags & MEDIA_PAD_FL_SINK)
+   swap(pad0, pad1);
+
return entity->ops->has_route(entity, pad0, pad1);
 }
 EXPORT_SYMBOL_GPL(media_entity_has_route);
-- 
2.7.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

Problem retrieving zl10353 information: Resource temporarily unavailable (but signal =71% ?)

[mjs]

Hello,

I'm trying to get a dvb-t usb-stick to work with debian.

 Components: em2882 - xc3028l (uses XC3028L-V36.fw) - ce6353 (zl10353) - 
tvp5150 - emp202
 Kernel: 4.7.0-0.bpo.1-686-pae - debian 8
 i2c device: eeprom @ 0xa0 - tvp5150 @ 0xb8 - tuner(analog) @0xc2 (from 
dmesg 2.6 kernel)

I got to this point:

 femon -H:
 FE: Zarlink ZL10353 DVB-T (DVBT)
 Problem retrieving frontend information: Resource temporarily unavailable
 status  C| signal  71% | snr  74% | ber -1080313980 | unc -1218616323 
| 


Using next (G)PIO settings, enable more did not improve anything:

static struct em28xx_reg_seq zolid_tuner[] = {
//  {EM2820_R08_GPIO_CTRL,  EM_GPIO_4,  EM_GPIO_4,   10},
//  {EM2820_R08_GPIO_CTRL,   0, EM_GPIO_4,   10},
//  {EM2820_R08_GPIO_CTRL,  EM_GPIO_4,  EM_GPIO_4,   10},
{   -1, -1, -1,  -1},
};
static struct em28xx_reg_seq zolid_digital[] = {
//  {EM2820_R08_GPIO_CTRL,  0x6e,   ~EM_GPIO_4, 100},
//  {EM2880_R04_GPO,0x04,   0xff,   100},   
/* zl10353 reset ? */
{EM2880_R04_GPO,0x08,   0xff,10},   
/* zl10353 to connect tuner (dmesg) */
//  {EM2880_R04_GPO,0x0c,   0xff,10},
{   -1, -1, -1,  -1},
};
static struct em28xx_reg_seq zolid_analog[] = {
{EM2820_R08_GPIO_CTRL,  0x6d,   ~EM_GPIO_4,  10},   
/* em202 (dmesg) */
//  {EM2880_R04_GPO,0x04,   0xff,   100},
//  {EM2880_R04_GPO,0x08,   0xff,10},
//  {EM2880_R04_GPO,0x0c,   0xff,10},
{   -1, -1, -1,  -1},

Two years ago I used snoop and perl tools on the ms-windows-driver and got next 
result:
40 00 00 00 04 00 01 00 >>> 04, 08 or 0c
19 times alternating 08 and 0c, and the last one was 04 followed by 0c
Also 40 00 00 00 08 00 01 00 >>>  6a, 6b, 6f, 7a, 7f, fd, fe or ff
I tried all of them in the second Coulomb zolid_digital as 
EM2820_R09_GPIO_CTRL, no improvement.

I do have a data-sheet em2882, did search trough linux-media and used 
duck-duck-go trying to get relevant info, no luck at this point.

Question:
Where to find knowledge about the em2882 GPIO and GPO ?
And naturally, any tips or advice is appreciated.

Thanks in advance.

Marcel Stork (Netherlands)





lsusb:
Bus 005 Device 002: ID eb1a:2883 eMPIA Technology, Inc. 
--
/dev/dvb/adapter0 with demux0, drv0, frontend0 and net0 is created.
--
dmesg:
[ 1897.124737] em28xx: New device  USB 2883 Device @ 480 Mbps (eb1a:2883, 
interface 0, class 0)
[ 1897.124745] em28xx: Audio interface 0 found (Vendor Class)
[ 1897.124750] em28xx: Video interface 0 found: isoc
[ 1897.124754] em28xx: DVB interface 0 found: isoc
[ 1897.124896] em28xx: chip ID is em2882/3
[ 1897.230009] em2882/3 #0: EEPROM ID = 1a eb 67 95, EEPROM hash = 0x85dd871e
[ 1897.230017] em2882/3 #0: EEPROM info:
[ 1897.230020] em2882/3 #0: AC97 audio (5 sample rates)
[ 1897.230024] em2882/3 #0: 500mA max power
[ 1897.230029] em2882/3 #0: Table at offset 0x24, strings=0x226a, 0x108c, 
0x
[ 1897.230035] em2882/3 #0: Identified as :ZOLID HYBRID TV STICK (card=100)
[ 1897.230040] em2882/3 #0: analog set to isoc mode.
[ 1897.230044] em2882/3 #0: dvb set to isoc mode.
[ 1897.230280] usbcore: registered new interface driver em28xx
[ 1897.291557] em2882/3 #0: Registering V4L2 extension
[ 1897.313877] tvp5150 7-005c: tvp5150 (4.0) chip found @ 0xb8 (em2882/3 #0)
[ 1897.313885] tvp5150 7-005c: tvp5150am1 detected.
[ 1897.327869] tuner 7-0061: Tuner -1 found with type(s) Radio TV.
[ 1897.376267] xc2028 7-0061: creating new instance
[ 1897.376277] xc2028 7-0061: type set to XCeive xc2028/xc3028 tuner
[ 1897.376495] em2882/3 #0: Config register raw data: 0xd0
  [ 1897.377246] em2882/3 #0: AC97 vendor ID = 0x
  [ 1897.377621] em2882/3 #0: AC97 features = 0x6a90
[ 1897.377626] em2882/3 #0: Empia 202 AC97 audio processor detected
[ 1897.399113] usb 5-3: firmware: direct-loading firmware xc3028L-v36.fw
[ 1897.399129] xc2028 7-0061: Loading 81 firmware images from xc3028L-v36.fw, 
type: xc2028 firmware, ver 3.6
[ 1897.524132] xc2028 7-0061: Loading firmware for type=BASE F8MHZ MTS (7), id 
.
  [ 1898.511621] MTS (4), id 00ff:
[ 1898.511630] xc2028 7-0061: Loading firmware for type=MTS (4), id 
0007.
[ 1898.809002] em2882/3 #0: V4L2 video device registered as video0
[ 1898.809009] em2882/3 #0: V4L2 VBI device registered as vbi0
[ 1898.809994] em2882/3 #0: V4L2 extension successfully initialized
[ 1898.81] em28xx: Registered (Em28xx v4l2 Extension) extension
[ 1898.971663] 

Re: [RFC v3 00/21] Make use of kref in media device, grab references as needed

[Sakari Ailus]

Hi Shuah,

On Mon, Nov 07, 2016 at 01:16:45PM -0700, Shuah Khan wrote:
> Hi Sakari,
> 
> On 08/26/2016 05:43 PM, Sakari Ailus wrote:
> > Hi folks,
> > 
> > This is the third version of the RFC set to fix referencing in media
> > devices.
> > 
> > The lifetime of the media device (and media devnode) is now bound to that
> > of struct device embedded in it and its memory is only released once the
> > last reference is gone: unregistering is simply unregistering, it no
> > longer should release memory which could be further accessed.
> > 
> > 
> > A video node or a sub-device node also gets a reference to the media
> > device, i.e. the release function of the video device node will release
> > its reference to the media device. The same goes for file handles to the
> > media device.
> > 
> > 
> > As a side effect of this is that the media device, it is allocate together
> > with the media devnode. The driver may also rely its own resources to the
> > media device. Alternatively there's also a priv field to hold drivers
> > private pointer (for container_of() is an option in this case). We could
> > drop one of these options but currently both are possible.
> > 
> > 
> > I've tested this by manually unbinding the omap3isp platform device while
> > streaming. Driver changes are required for this to work; by not using
> > dynamic allocation (i.e. media_device_alloc()) the old behaviour is still
> > supported. This is still unlikely to be a grave problem as there are not
> > that many device drivers that support physically removable devices. We've
> > had this problem for other devices for many years without paying much
> > notice --- that doesn't mean I don't think at least drivers for removable
> > devices shouldn't be changed as part of the set later on, I'd just like to
> > get review comments on the approach first.
> > 
> > 
> > The three patches that originally partially resolved some of these issues
> > are reverted in the beginning of the set. I'm still posting this as an RFC
> > mainly since the testing is somewhat limited so far.
> 
> The main difference between the approach taken in these 3 reverted fixes and
> this RFC series is as follows:
> 
> Reverted fixes:
> - Fix the lifetime problem with the media devnode by dynamically allocating
>   devnode instead of media_device. One of the main considerations to this
>   approach is to isolate the changes in media core and avoid changes to
>   drivers.
> - I tested these fixes extensively and added selftests and README file for
>   the regression tests. I haven't seen any problems after these fixes went
>   in while physically removing au0828 device, em028xx, and uvcvideo

I'd rather call them workarounds, as they do work around the issues rather
than properly fixing them. This approach isn't really extensible to fix the
remaining problems either. It is true that *some* of the issues that were
present before these patches do not show up anymore with them, but we really
do need to fix all of these bugs.

The underlying problem is that there may be opened file handles, references
from elsewhere in the kernel or such to in-memory objects that are not
refcounted properly: referencing released memory is a no-go in kernel.

> 
> This RFC series:
> - Dynamically allocates media_device
> - This approach requires changes to drivers. It would be wise to not require
>   churn to driver code and fix the problem in media-core.
> 
> Do you have information on the problems that still remain with the above fixes
> in place? These fixes went into 4.8 is I recall correctly. Could you please
> send us the list of problems and dmesg for the problems you found with the
> above fixes and how this RFC series addresses them.

Just try removing a device when it's streaming. No more than that is needed.

This is one of the bugs fixed by the patchset, albeit drivers do need to be
changed as well to benefit from the changes. 

> 
> Can these problems be fixed without needing to change the approach in the
> reverted patches?

I don't think it's feasible, really. Besides, the workaround were rather
ugly and were merged only since there was a said urgency to have a partial
fix early. See above as well.

> 
> I have a patch series on top of the fixes this RFC series is reverting
> to allocate media_device only in the cases where sharing media device
> is necessary. e.g: au0828 and snd-usb-audio.
> 
> Media Device Allocator API
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg98793.html
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg97779.html
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg97704.html
> 
> This series has been