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:   Fri Nov  3 05:00:21 CET 2017
media-tree git hash:9917fbcfa20ab987d6381fd0365665e5c1402d75
media_build git hash:   c93534951f5d66bef7f17f16293acf2be346b726
v4l-utils git hash: e656ea2b16cbb83ca5e37d9df7af0c2cc34379f4
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.12.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: 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.12.67-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: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9.26-i686: ERRORS
linux-4.10.14-i686: ERRORS
linux-4.11-i686: ERRORS
linux-4.12.1-i686: ERRORS
linux-4.13-i686: ERRORS
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.12.67-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: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.26-x86_64: ERRORS
linux-4.10.14-x86_64: ERRORS
linux-4.11-x86_64: ERRORS
linux-4.12.1-x86_64: ERRORS
linux-4.13-x86_64: ERRORS
apps: OK
spec-git: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

RE: [PATCH v9 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor

[Vishal Sagar]

Hi Leon,

I understand this fixes correctly freeing the v4l control handlers in probe().

But if there is a scenario where the sensor is mounted on a removable daughter 
card,
shouldn't the probe fail if the daughter card is not connected? 
A sample read/write to an IMX274 register should be sufficient to confirm this 
in the probe() and fail.

Does it make sense to add this in the probe()?

Regards
Vishal Sagar 

> -Original Message-
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Leon Luo
> Sent: Thursday, October 26, 2017 12:21 PM
> To: mche...@kernel.org; robh...@kernel.org; mark.rutl...@arm.com;
> hans.verk...@cisco.com; sakari.ai...@linux.intel.com
> Cc: linux-media@vger.kernel.org; devicet...@vger.kernel.org; linux-
> ker...@vger.kernel.org; le...@leopardimaging.com; Soren Brinkmann
> 
> Subject: [PATCH v9 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor
> 
> The imx274 is a Sony CMOS image sensor that has 1/2.5 image size.
> It supports up to 3840x2160 (4K) 60fps, 1080p 120fps. The interface
> is 4-lane MIPI CSI-2 running at 1.44Gbps each.
> 
> This driver has been tested on Xilinx ZCU102 platform with a Leopard
> LI-IMX274MIPI-FMC camera board.
> 
> Support for the following features:
> -Resolutions: 3840x2160, 1920x1080, 1280x720
> -Frame rate: 3840x2160 : 5 – 60fps
> 1920x1080 : 5 – 120fps
> 1280x720 : 5 – 120fps
> -Exposure time: 16 – (frame interval) micro-seconds
> -Gain: 1x - 180x
> -VFLIP: enable/disabledrivers/media/i2c/imx274.c
> -Test pattern: 12 test patterns
> 
> Signed-off-by: Leon Luo 
> Tested-by: Sören Brinkmann 
> Acked-by: Sakari Ailus 
> Acked-by: Chris Kohn 
> ---
> v9:
>  - remove v4l2_async_unregister_subdev from probe error
> v8:
>  - check division by zero error
> v7:
>  - use __v4l2_ctrl_s_ctrl instead of v4l2_ctrl_s_ctrl to have
>clean mutex_lock/mutex_unlock in imx274_s_stream()
>  - define imx274_tp_regs[] as static, move the test pattern reg
>out of imx274_tp_regs[], and define it as a macro
>IMX274_TEST_PATTERN_REG
> v6:
>  - remove media/v4l2-image-sizes.h from include header
>  - make the header file alphabetical order
>  - remove fmt->pad check in imx274_get_fmt,
>the V4L2 subdev framework does it already
>  - change 'struct reg_8 *regs' to 'struct reg_8 regs[n]',
>where n is the exact numbers needed for this function
>  - move MODULE_DEVICE_TABLE(of, imx274_of_id_table); closer
>to imx274_of_id_table definition
>  - remove return check of imx274_write_table in imx274_remove,
>because it should remove all resources even i2c fails here
>  - move imx274_load_default before v4l2_async_register_subdev
> v5:
>  - no changes
> v4:
>  - use 32-bit data type to avoid __divdi3 compile error for i386
>  - clean up OR together error codesdrivers/media/i2c/imx274.c
> v3:
>  - clean up header files
>  - use struct directly instead of alias #define
>  - use v4l2_ctrl_s_ctrl instead of v4l2_s_ctrl
>  - revise debug output
>  - move static helpers closer to their call site
>  - don't OR toegether error codes
>  - use closest valid gain value instead of erroring out
>  - assigne lock to the control handler and omit explicit locking
>  - acquire mutex lock for imx274_get_fmt
>  - remove format->pad check in imx274_set_fmt since the pad is always 0
>  - pass struct v4l2_ctrl pointer in gain/exposure/vlip/test pattern controls
>  - remove priv->ctrls.vflip->val = val in imx274_set_vflip()
>  - remove priv->ctrls.test_pattern->val = val in imx274_set_test_pattern()
>  - remove empty open/close callbacks
>  - remove empty core ops
>  - add more error labels in probe
>  - use v4l2_async_unregister_subdev instead of v4l2_device_unregister_subdev
>  - use dynamic debug
>  - split start_stream to two steps: imx274_mode_regs() and
> imx274_start_stream()
>frame rate & exposure can be updated
>between imx274_mode_regs() & imx274_start_stream()
> 
> v2:
>  - Fix Kconfig to not remove existing options
> ---
>  drivers/media/i2c/Kconfig  |7 +
>  drivers/media/i2c/Makefile |1 +
>  drivers/media/i2c/imx274.c | 1810
> 
>  3 files changed, 1818 insertions(+)
>  create mode 100644 drivers/media/i2c/imx274.c
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 47113774a297..9659849e33a0 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -555,6 +555,13 @@ config VIDEO_APTINA_PLL
>  config VIDEO_SMIAPP_PLL
>   tristate
> 
> +config VIDEO_IMX274
> + tristate "Sony IMX274 sensor support"
> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + ---help---
> +   This is a V4L2 sensor-level driver for the Sony IMX274
> +   CMOS image sensor.
> +
>  config VIDEO_OV2640
>   tristate "OmniVision OV2640 sensor support"
>   

Re: [PATCH 2/2] media: s5p-mfc: fix lock confection - request_firmware() once and keep state

[Marian Mihailescu]

I can confirm, with this patch, there is always error loading MFC in
boot log, since FS is not mounted.

-Marian

On Fri, Nov 3, 2017 at 10:57 AM, Shuah Khan  wrote:
> On 11/02/2017 02:31 AM, Andrzej Hajda wrote:
>> On 06.10.2017 23:30, Shuah Khan wrote:
>>> Driver calls request_firmware() whenever the device is opened for the
>>> first time. As the device gets opened and closed, dev->num_inst == 1
>>> is true several times. This is not necessary since the firmware is saved
>>> in the fw_buf. s5p_mfc_load_firmware() copies the buffer returned by
>>> the request_firmware() to dev->fw_buf.
>>>
>>> fw_buf sticks around until it gets released from s5p_mfc_remove(), hence
>>> there is no need to keep requesting firmware and copying it to fw_buf.
>>>
>>> This might have been overlooked when changes are made to free fw_buf from
>>> the device release interface s5p_mfc_release().
>>>
>>> Fix s5p_mfc_load_firmware() to call request_firmware() once and keep state.
>>> Change _probe() to load firmware once fw_buf has been allocated.
>>>
>>> s5p_mfc_open() and it continues to call s5p_mfc_load_firmware() and init
>>> hardware which is the step where firmware is written to the device.
>>>
>>> This addresses the mfc_mutex contention due to repeated request_firmware()
>>> calls from open() in the following circular locking warning:
>>>
>>> [  552.194115] qtdemux0:sink/2710 is trying to acquire lock:
>>> [  552.199488]  (>mfc_mutex){+.+.}, at: [] 
>>> s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
>>> [  552.207459]
>>>but task is already holding lock:
>>> [  552.213264]  (>mmap_sem){}, at: [] 
>>> vm_mmap_pgoff+0x44/0xb8
>>> [  552.220284]
>>>which lock already depends on the new lock.
>>>
>>> [  552.228429]
>>>the existing dependency chain (in reverse order) is:
>>> [  552.235881]
>>>-> #2 (>mmap_sem){}:
>>> [  552.241259]__might_fault+0x80/0xb0
>>> [  552.245331]filldir64+0xc0/0x2f8
>>> [  552.249144]call_filldir+0xb0/0x14c
>>> [  552.253214]ext4_readdir+0x768/0x90c
>>> [  552.257374]iterate_dir+0x74/0x168
>>> [  552.261360]SyS_getdents64+0x7c/0x1a0
>>> [  552.265608]ret_fast_syscall+0x0/0x28
>>> [  552.269850]
>>>-> #1 (>i_mutex_dir_key#2){}:
>>> [  552.276180]down_read+0x48/0x90
>>> [  552.279904]lookup_slow+0x74/0x178
>>> [  552.283889]walk_component+0x1a4/0x2e4
>>> [  552.288222]link_path_walk+0x174/0x4a0
>>> [  552.292555]path_openat+0x68/0x944
>>> [  552.296541]do_filp_open+0x60/0xc4
>>> [  552.300528]file_open_name+0xe4/0x114
>>> [  552.304772]filp_open+0x28/0x48
>>> [  552.308499]kernel_read_file_from_path+0x30/0x78
>>> [  552.313700]_request_firmware+0x3ec/0x78c
>>> [  552.318291]request_firmware+0x3c/0x54
>>> [  552.322642]s5p_mfc_load_firmware+0x54/0x150 [s5p_mfc]
>>> [  552.328358]s5p_mfc_open+0x4e4/0x550 [s5p_mfc]
>>> [  552.94]v4l2_open+0xa0/0x104 [videodev]
>>> [  552.338137]chrdev_open+0xa4/0x18c
>>> [  552.342121]do_dentry_open+0x208/0x310
>>> [  552.346454]path_openat+0x28c/0x944
>>> [  552.350526]do_filp_open+0x60/0xc4
>>> [  552.354512]do_sys_open+0x118/0x1c8
>>> [  552.358586]ret_fast_syscall+0x0/0x28
>>> [  552.362830]
>>>-> #0 (>mfc_mutex){+.+.}:
>>>-> #0 (>mfc_mutex){+.+.}:
>>> [  552.368379]lock_acquire+0x6c/0x88
>>> [  552.372364]__mutex_lock+0x68/0xa34
>>> [  552.376437]mutex_lock_interruptible_nested+0x1c/0x24
>>> [  552.382086]s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
>>> [  552.386939]v4l2_mmap+0x54/0x88 [videodev]
>>> [  552.391601]mmap_region+0x3a8/0x638
>>> [  552.395673]do_mmap+0x330/0x3a4
>>> [  552.399400]vm_mmap_pgoff+0x90/0xb8
>>> [  552.403472]SyS_mmap_pgoff+0x90/0xc0
>>> [  552.407632]ret_fast_syscall+0x0/0x28
>>> [  552.411876]
>>>other info that might help us debug this:
>>>
>>> [  552.419848] Chain exists of:
>>>  >mfc_mutex --> >i_mutex_dir_key#2 --> 
>>> >mmap_sem
>>>
>>> [  552.431200]  Possible unsafe locking scenario:
>>>
>>> [  552.437092]CPU0CPU1
>>> [  552.441598]
>>> [  552.446104]   lock(>mmap_sem);
>>> [  552.449484]
>>> lock(>i_mutex_dir_key#2);
>>> [  552.456329]lock(>mmap_sem);
>>> [  552.46]   lock(>mfc_mutex);
>>> [  552.465775]
>>> *** DEADLOCK ***
>>
>> I am not 100% but it looks like false positive. Could you describe
>> scenario when it deadlocks?
>>
>>>
>>> Signed-off-by: Shuah Khan 
>>> ---
>>>  drivers/media/platform/s5p-mfc/s5p_mfc.c| 4 
>>>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 3 +++
>>>  

Re: [PATCH 2/2] media: s5p-mfc: fix lock confection - request_firmware() once and keep state

[Shuah Khan]

On 11/02/2017 02:31 AM, Andrzej Hajda wrote:
> On 06.10.2017 23:30, Shuah Khan wrote:
>> Driver calls request_firmware() whenever the device is opened for the
>> first time. As the device gets opened and closed, dev->num_inst == 1
>> is true several times. This is not necessary since the firmware is saved
>> in the fw_buf. s5p_mfc_load_firmware() copies the buffer returned by
>> the request_firmware() to dev->fw_buf.
>>
>> fw_buf sticks around until it gets released from s5p_mfc_remove(), hence
>> there is no need to keep requesting firmware and copying it to fw_buf.
>>
>> This might have been overlooked when changes are made to free fw_buf from
>> the device release interface s5p_mfc_release().
>>
>> Fix s5p_mfc_load_firmware() to call request_firmware() once and keep state.
>> Change _probe() to load firmware once fw_buf has been allocated.
>>
>> s5p_mfc_open() and it continues to call s5p_mfc_load_firmware() and init
>> hardware which is the step where firmware is written to the device.
>>
>> This addresses the mfc_mutex contention due to repeated request_firmware()
>> calls from open() in the following circular locking warning:
>>
>> [  552.194115] qtdemux0:sink/2710 is trying to acquire lock:
>> [  552.199488]  (>mfc_mutex){+.+.}, at: [] 
>> s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
>> [  552.207459]
>>but task is already holding lock:
>> [  552.213264]  (>mmap_sem){}, at: [] 
>> vm_mmap_pgoff+0x44/0xb8
>> [  552.220284]
>>which lock already depends on the new lock.
>>
>> [  552.228429]
>>the existing dependency chain (in reverse order) is:
>> [  552.235881]
>>-> #2 (>mmap_sem){}:
>> [  552.241259]__might_fault+0x80/0xb0
>> [  552.245331]filldir64+0xc0/0x2f8
>> [  552.249144]call_filldir+0xb0/0x14c
>> [  552.253214]ext4_readdir+0x768/0x90c
>> [  552.257374]iterate_dir+0x74/0x168
>> [  552.261360]SyS_getdents64+0x7c/0x1a0
>> [  552.265608]ret_fast_syscall+0x0/0x28
>> [  552.269850]
>>-> #1 (>i_mutex_dir_key#2){}:
>> [  552.276180]down_read+0x48/0x90
>> [  552.279904]lookup_slow+0x74/0x178
>> [  552.283889]walk_component+0x1a4/0x2e4
>> [  552.288222]link_path_walk+0x174/0x4a0
>> [  552.292555]path_openat+0x68/0x944
>> [  552.296541]do_filp_open+0x60/0xc4
>> [  552.300528]file_open_name+0xe4/0x114
>> [  552.304772]filp_open+0x28/0x48
>> [  552.308499]kernel_read_file_from_path+0x30/0x78
>> [  552.313700]_request_firmware+0x3ec/0x78c
>> [  552.318291]request_firmware+0x3c/0x54
>> [  552.322642]s5p_mfc_load_firmware+0x54/0x150 [s5p_mfc]
>> [  552.328358]s5p_mfc_open+0x4e4/0x550 [s5p_mfc]
>> [  552.94]v4l2_open+0xa0/0x104 [videodev]
>> [  552.338137]chrdev_open+0xa4/0x18c
>> [  552.342121]do_dentry_open+0x208/0x310
>> [  552.346454]path_openat+0x28c/0x944
>> [  552.350526]do_filp_open+0x60/0xc4
>> [  552.354512]do_sys_open+0x118/0x1c8
>> [  552.358586]ret_fast_syscall+0x0/0x28
>> [  552.362830]
>>-> #0 (>mfc_mutex){+.+.}:
>>-> #0 (>mfc_mutex){+.+.}:
>> [  552.368379]lock_acquire+0x6c/0x88
>> [  552.372364]__mutex_lock+0x68/0xa34
>> [  552.376437]mutex_lock_interruptible_nested+0x1c/0x24
>> [  552.382086]s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
>> [  552.386939]v4l2_mmap+0x54/0x88 [videodev]
>> [  552.391601]mmap_region+0x3a8/0x638
>> [  552.395673]do_mmap+0x330/0x3a4
>> [  552.399400]vm_mmap_pgoff+0x90/0xb8
>> [  552.403472]SyS_mmap_pgoff+0x90/0xc0
>> [  552.407632]ret_fast_syscall+0x0/0x28
>> [  552.411876]
>>other info that might help us debug this:
>>
>> [  552.419848] Chain exists of:
>>  >mfc_mutex --> >i_mutex_dir_key#2 --> 
>> >mmap_sem
>>
>> [  552.431200]  Possible unsafe locking scenario:
>>
>> [  552.437092]CPU0CPU1
>> [  552.441598]
>> [  552.446104]   lock(>mmap_sem);
>> [  552.449484]lock(>i_mutex_dir_key#2);
>> [  552.456329]lock(>mmap_sem);
>> [  552.46]   lock(>mfc_mutex);
>> [  552.465775]
>> *** DEADLOCK ***
> 
> I am not 100% but it looks like false positive. Could you describe
> scenario when it deadlocks?
> 
>>
>> Signed-off-by: Shuah Khan 
>> ---
>>  drivers/media/platform/s5p-mfc/s5p_mfc.c| 4 
>>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 3 +++
>>  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c   | 5 +
>>  3 files changed, 12 insertions(+)
>>
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
>> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
>> index 1afde50..4c253fb 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
>> +++ 

Re: [PATCH 1/2] media: s5p-mfc: check for firmware allocation before requesting firmware

[Shuah Khan]

On 11/02/2017 02:12 AM, Andrzej Hajda wrote:
> Hi Shuah,
> 
> On 06.10.2017 23:30, Shuah Khan wrote:
>> Check if firmware is allocated before requesting firmware instead of
>> requesting firmware only to release it if firmware is not allocated.
>>
>> Signed-off-by: Shuah Khan 
>> ---
>>  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 10 +-
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c 
>> b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
>> index 69ef9c2..f064a0d1 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
>> @@ -55,6 +55,11 @@ int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev)
>>   * into kernel. */
>>  mfc_debug_enter();
>>  
>> +if (!dev->fw_buf.virt) {
>> +mfc_err("MFC firmware is not allocated\n");
>> +return -EINVAL;
>> +}
>> +
>>  for (i = MFC_FW_MAX_VERSIONS - 1; i >= 0; i--) {
>>  if (!dev->variant->fw_name[i])
>>  continue;
>> @@ -75,11 +80,6 @@ int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev)
>>  release_firmware(fw_blob);
>>  return -ENOMEM;
>>  }
>> -if (!dev->fw_buf.virt) {
>> -mfc_err("MFC firmware is not allocated\n");
>> -release_firmware(fw_blob);
>> -return -EINVAL;
>> -}
> 
> Is there any scenario in which dev->fw_buf.virt is null and
> s5p_mfc_load_firmware is called?
> I suspect this check is not necessary at all.
> 

I don't believe so. Allocation is done in s5p_mfc_configure_dma_memory()
code path and if that fails it bails out of _probe. I can remove the check
all together.

thanks,
-- Shuah

Re: [PATCH v2] media: ttpci: remove autorepeat handling and use timer_setup

[Mauro Carvalho Chehab]

Em Thu, 2 Nov 2017 16:50:37 -0700
Dmitry Torokhov  escreveu:

> On Thu, Nov 02, 2017 at 04:24:27PM -0700, Kees Cook wrote:
> > On Tue, Oct 31, 2017 at 1:11 PM, Sean Young  wrote:  
> > > Leave the autorepeat handling up to the input layer, and move
> > > to the new timer API.
> > >
> > > Compile tested only.
> > >
> > > Signed-off-by: Sean Young   
> > 
> > Hi! Just checking up on this... the input timer conversion is blocked
> > by getting this sorted out, so I'd love to have something either
> > media, input, or timer tree can carry. :)  
> 
> Acked-by: Dmitry Torokhov 
> 
> From my POV the patch is good. Mauro, do you want me to take it through
> my tree, or maybe you could create an immutable branch off 4.14-rc5 (or
> 6) with this commit and I will pull it in and then can apply Kees input
> core conversion patch?

Feel free to apply it into your tree with my ack:

Acked-by: Mauro Carvalho Chehab 

Regards,
Mauro

> 
> Thanks!
> 
> > 
> > Thanks!
> > 
> > -Kees
> >   
> > > ---
> > > v2:
> > >  - fixes and improvements from Dmitry Torokhov
> > >
> > >  drivers/media/pci/ttpci/av7110.h|  2 +-
> > >  drivers/media/pci/ttpci/av7110_ir.c | 56 
> > > +
> > >  2 files changed, 21 insertions(+), 37 deletions(-)
> > >
> > > diff --git a/drivers/media/pci/ttpci/av7110.h 
> > > b/drivers/media/pci/ttpci/av7110.h
> > > index 347827925c14..bcb72ecbedc0 100644
> > > --- a/drivers/media/pci/ttpci/av7110.h
> > > +++ b/drivers/media/pci/ttpci/av7110.h
> > > @@ -93,7 +93,7 @@ struct infrared {
> > > u8  inversion;
> > > u16 last_key;
> > > u16 last_toggle;
> > > -   u8  delay_timer_finished;
> > > +   boolkeypressed;
> > >  };
> > >
> > >
> > > diff --git a/drivers/media/pci/ttpci/av7110_ir.c 
> > > b/drivers/media/pci/ttpci/av7110_ir.c
> > > index ca05198de2c2..ee414803e6b5 100644
> > > --- a/drivers/media/pci/ttpci/av7110_ir.c
> > > +++ b/drivers/media/pci/ttpci/av7110_ir.c
> > > @@ -84,15 +84,16 @@ static u16 default_key_map [256] = {
> > >
> > >
> > >  /* key-up timer */
> > > -static void av7110_emit_keyup(unsigned long parm)
> > > +static void av7110_emit_keyup(struct timer_list *t)
> > >  {
> > > -   struct infrared *ir = (struct infrared *) parm;
> > > +   struct infrared *ir = from_timer(ir, t, keyup_timer);
> > >
> > > -   if (!ir || !test_bit(ir->last_key, ir->input_dev->key))
> > > +   if (!ir || !ir->keypressed)
> > > return;
> > >
> > > input_report_key(ir->input_dev, ir->last_key, 0);
> > > input_sync(ir->input_dev);
> > > +   ir->keypressed = false;
> > >  }
> > >
> > >
> > > @@ -152,29 +153,18 @@ static void av7110_emit_key(unsigned long parm)
> > > return;
> > > }
> > >
> > > -   if (timer_pending(>keyup_timer)) {
> > > -   del_timer(>keyup_timer);
> > > -   if (ir->last_key != keycode || toggle != ir->last_toggle) 
> > > {
> > > -   ir->delay_timer_finished = 0;
> > > -   input_event(ir->input_dev, EV_KEY, ir->last_key, 
> > > 0);
> > > -   input_event(ir->input_dev, EV_KEY, keycode, 1);
> > > -   input_sync(ir->input_dev);
> > > -   } else if (ir->delay_timer_finished) {
> > > -   input_event(ir->input_dev, EV_KEY, keycode, 2);
> > > -   input_sync(ir->input_dev);
> > > -   }
> > > -   } else {
> > > -   ir->delay_timer_finished = 0;
> > > -   input_event(ir->input_dev, EV_KEY, keycode, 1);
> > > -   input_sync(ir->input_dev);
> > > -   }
> > > +   if (ir->keypressed &&
> > > +   (ir->last_key != keycode || toggle != ir->last_toggle))
> > > +   input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
> > >
> > > +   input_event(ir->input_dev, EV_KEY, keycode, 1);
> > > +   input_sync(ir->input_dev);
> > > +
> > > +   ir->keypressed = true;
> > > ir->last_key = keycode;
> > > ir->last_toggle = toggle;
> > >
> > > -   ir->keyup_timer.expires = jiffies + UP_TIMEOUT;
> > > -   add_timer(>keyup_timer);
> > > -
> > > +   mod_timer(>keyup_timer, jiffies + UP_TIMEOUT);
> > >  }
> > >
> > >
> > > @@ -204,16 +194,6 @@ static void input_register_keys(struct infrared *ir)
> > > ir->input_dev->keycodemax = ARRAY_SIZE(ir->key_map);
> > >  }
> > >
> > > -
> > > -/* called by the input driver after rep[REP_DELAY] ms */
> > > -static void input_repeat_key(unsigned long parm)
> > > -{
> > > -   struct infrared *ir = (struct infrared *) parm;
> > > -
> > > -   ir->delay_timer_finished = 1;
> > > -}
> > > -
> > > -
> > >  /* check for configuration changes */
> > >  int 

Re: [RFC v4 16/17] [media] vb2: add out-fence support to QBUF

[Gustavo Padovan]

Hi Brian,

2017-10-27 Brian Starkey :

> Hi Gustavo,
> 
> On Fri, Oct 20, 2017 at 07:50:11PM -0200, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
> > an out_fence and send to userspace on the V4L2_EVENT_OUT_FENCE when
> > the buffer is queued to the driver, or right away if the queue is ordered
> > both in VB2 and in the driver.
> > 
> > The fence is signaled on buffer_done(), when the job on the buffer is
> > finished.
> > 
> > v5:
> > - delay fd_install to DQ_EVENT (Hans)
> > - if queue is fully ordered send OUT_FENCE event right away
> > (Brian)
> > - rename 'q->ordered' to 'q->ordered_in_driver'
> > - merge change to implement OUT_FENCE event here
> > 
> > v4:
> > - return the out_fence_fd in the BUF_QUEUED event(Hans)
> > 
> > v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans)
> > - set the OUT_FENCE flag if there is a fence pending (Hans)
> > - call fd_install() after vb2_core_qbuf() (Hans)
> > - clean up fence if vb2_core_qbuf() fails (Hans)
> > - add list to store sync_file and fence for the next queued buffer
> > 
> > v2: check if the queue is ordered.
> > 
> > Signed-off-by: Gustavo Padovan 
> > ---
> > drivers/media/v4l2-core/v4l2-event.c |  2 ++
> > drivers/media/v4l2-core/videobuf2-core.c | 25 +++
> > drivers/media/v4l2-core/videobuf2-v4l2.c | 55 
> > 
> > 3 files changed, 82 insertions(+)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-event.c 
> > b/drivers/media/v4l2-core/v4l2-event.c
> > index 6274e3e174e0..275da224ace4 100644
> > --- a/drivers/media/v4l2-core/v4l2-event.c
> > +++ b/drivers/media/v4l2-core/v4l2-event.c
> > @@ -385,6 +385,8 @@ int v4l2_subscribe_event_v4l2(struct v4l2_fh *fh,
> > switch (sub->type) {
> > case V4L2_EVENT_CTRL:
> > return v4l2_ctrl_subscribe_event(fh, sub);
> > +   case V4L2_EVENT_OUT_FENCE:
> > +   return v4l2_event_subscribe(fh, sub, VIDEO_MAX_FRAME, NULL);
> > }
> > return -EINVAL;
> > }
> > diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
> > b/drivers/media/v4l2-core/videobuf2-core.c
> > index c7ba67bda5ac..21e2052776c1 100644
> > --- a/drivers/media/v4l2-core/videobuf2-core.c
> > +++ b/drivers/media/v4l2-core/videobuf2-core.c
> > @@ -354,6 +354,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
> > vb2_memory memory,
> > vb->planes[plane].length = plane_sizes[plane];
> > vb->planes[plane].min_length = plane_sizes[plane];
> > }
> > +   vb->out_fence_fd = -1;
> > q->bufs[vb->index] = vb;
> > 
> > /* Allocate video buffer memory for the MMAP type */
> > @@ -934,10 +935,24 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
> > vb2_buffer_state state)
> > case VB2_BUF_STATE_QUEUED:
> > return;
> > case VB2_BUF_STATE_REQUEUEING:
> > +   /*
> > +* Explicit synchronization requires ordered queues for now,
> > +* so WARN_ON if we are requeuing on an ordered queue.
> > +*/
> > +   if (vb->out_fence)
> > +   WARN_ON_ONCE(q->ordered_in_driver);
> > +
> > if (q->start_streaming_called)
> > __enqueue_in_driver(vb);
> > return;
> > default:
> > +   if (state == VB2_BUF_STATE_ERROR)
> > +   dma_fence_set_error(vb->out_fence, -ENOENT);
> > +   dma_fence_signal(vb->out_fence);
> > +   dma_fence_put(vb->out_fence);
> > +   vb->out_fence = NULL;
> > +   vb->out_fence_fd = -1;
> > +
> > /* Inform any processes that may be waiting for buffers */
> > wake_up(>done_wq);
> > break;
> > @@ -1235,6 +1250,9 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
> > trace_vb2_buf_queue(q, vb);
> > 
> > call_void_vb_qop(vb, buf_queue, vb);
> > +
> > +   if (!(q->is_output || q->ordered_in_vb2))
> > +   call_void_bufop(q, send_out_fence, vb);
> > }
> > 
> > static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
> > @@ -1451,6 +1469,7 @@ static struct dma_fence *__set_in_fence(struct 
> > vb2_queue *q,
> > }
> > 
> > q->last_fence = dma_fence_get(fence);
> > +   call_void_bufop(q, send_out_fence, vb);
> > }
> > 
> > return fence;
> > @@ -1840,6 +1859,11 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
> > }
> > 
> > /*
> > +* Renew out-fence context.
> > +*/
> 
> Why is that? I don't think I understand the nuances of fence contexts.

Because inside each context we should maintain ordering of the fences.
If we cancel the stream and restart it with we need a new context. Look
at ordering between two different streams doesn't make sense.

> 
> > +   q->out_fence_context = 

Re: [PATCH v2] media: ttpci: remove autorepeat handling and use timer_setup

[Dmitry Torokhov]

On Thu, Nov 02, 2017 at 04:24:27PM -0700, Kees Cook wrote:
> On Tue, Oct 31, 2017 at 1:11 PM, Sean Young  wrote:
> > Leave the autorepeat handling up to the input layer, and move
> > to the new timer API.
> >
> > Compile tested only.
> >
> > Signed-off-by: Sean Young 
> 
> Hi! Just checking up on this... the input timer conversion is blocked
> by getting this sorted out, so I'd love to have something either
> media, input, or timer tree can carry. :)

Acked-by: Dmitry Torokhov 

>From my POV the patch is good. Mauro, do you want me to take it through
my tree, or maybe you could create an immutable branch off 4.14-rc5 (or
6) with this commit and I will pull it in and then can apply Kees input
core conversion patch?

Thanks!

> 
> Thanks!
> 
> -Kees
> 
> > ---
> > v2:
> >  - fixes and improvements from Dmitry Torokhov
> >
> >  drivers/media/pci/ttpci/av7110.h|  2 +-
> >  drivers/media/pci/ttpci/av7110_ir.c | 56 
> > +
> >  2 files changed, 21 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/media/pci/ttpci/av7110.h 
> > b/drivers/media/pci/ttpci/av7110.h
> > index 347827925c14..bcb72ecbedc0 100644
> > --- a/drivers/media/pci/ttpci/av7110.h
> > +++ b/drivers/media/pci/ttpci/av7110.h
> > @@ -93,7 +93,7 @@ struct infrared {
> > u8  inversion;
> > u16 last_key;
> > u16 last_toggle;
> > -   u8  delay_timer_finished;
> > +   boolkeypressed;
> >  };
> >
> >
> > diff --git a/drivers/media/pci/ttpci/av7110_ir.c 
> > b/drivers/media/pci/ttpci/av7110_ir.c
> > index ca05198de2c2..ee414803e6b5 100644
> > --- a/drivers/media/pci/ttpci/av7110_ir.c
> > +++ b/drivers/media/pci/ttpci/av7110_ir.c
> > @@ -84,15 +84,16 @@ static u16 default_key_map [256] = {
> >
> >
> >  /* key-up timer */
> > -static void av7110_emit_keyup(unsigned long parm)
> > +static void av7110_emit_keyup(struct timer_list *t)
> >  {
> > -   struct infrared *ir = (struct infrared *) parm;
> > +   struct infrared *ir = from_timer(ir, t, keyup_timer);
> >
> > -   if (!ir || !test_bit(ir->last_key, ir->input_dev->key))
> > +   if (!ir || !ir->keypressed)
> > return;
> >
> > input_report_key(ir->input_dev, ir->last_key, 0);
> > input_sync(ir->input_dev);
> > +   ir->keypressed = false;
> >  }
> >
> >
> > @@ -152,29 +153,18 @@ static void av7110_emit_key(unsigned long parm)
> > return;
> > }
> >
> > -   if (timer_pending(>keyup_timer)) {
> > -   del_timer(>keyup_timer);
> > -   if (ir->last_key != keycode || toggle != ir->last_toggle) {
> > -   ir->delay_timer_finished = 0;
> > -   input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
> > -   input_event(ir->input_dev, EV_KEY, keycode, 1);
> > -   input_sync(ir->input_dev);
> > -   } else if (ir->delay_timer_finished) {
> > -   input_event(ir->input_dev, EV_KEY, keycode, 2);
> > -   input_sync(ir->input_dev);
> > -   }
> > -   } else {
> > -   ir->delay_timer_finished = 0;
> > -   input_event(ir->input_dev, EV_KEY, keycode, 1);
> > -   input_sync(ir->input_dev);
> > -   }
> > +   if (ir->keypressed &&
> > +   (ir->last_key != keycode || toggle != ir->last_toggle))
> > +   input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
> >
> > +   input_event(ir->input_dev, EV_KEY, keycode, 1);
> > +   input_sync(ir->input_dev);
> > +
> > +   ir->keypressed = true;
> > ir->last_key = keycode;
> > ir->last_toggle = toggle;
> >
> > -   ir->keyup_timer.expires = jiffies + UP_TIMEOUT;
> > -   add_timer(>keyup_timer);
> > -
> > +   mod_timer(>keyup_timer, jiffies + UP_TIMEOUT);
> >  }
> >
> >
> > @@ -204,16 +194,6 @@ static void input_register_keys(struct infrared *ir)
> > ir->input_dev->keycodemax = ARRAY_SIZE(ir->key_map);
> >  }
> >
> > -
> > -/* called by the input driver after rep[REP_DELAY] ms */
> > -static void input_repeat_key(unsigned long parm)
> > -{
> > -   struct infrared *ir = (struct infrared *) parm;
> > -
> > -   ir->delay_timer_finished = 1;
> > -}
> > -
> > -
> >  /* check for configuration changes */
> >  int av7110_check_ir_config(struct av7110 *av7110, int force)
> >  {
> > @@ -333,8 +313,7 @@ int av7110_ir_init(struct av7110 *av7110)
> > av_list[av_cnt++] = av7110;
> > av7110_check_ir_config(av7110, true);
> >
> > -   setup_timer(>ir.keyup_timer, av7110_emit_keyup,
> > -   (unsigned long)>ir);
> > +   timer_setup(>ir.keyup_timer, av7110_emit_keyup, 0);
> >
> > input_dev = input_allocate_device();
> > if (!input_dev)
> > @@ -365,8 +344,13 @@ int 

Re: [PATCH v2] media: ttpci: remove autorepeat handling and use timer_setup

[Kees Cook]

On Tue, Oct 31, 2017 at 1:11 PM, Sean Young  wrote:
> Leave the autorepeat handling up to the input layer, and move
> to the new timer API.
>
> Compile tested only.
>
> Signed-off-by: Sean Young 

Hi! Just checking up on this... the input timer conversion is blocked
by getting this sorted out, so I'd love to have something either
media, input, or timer tree can carry. :)

Thanks!

-Kees

> ---
> v2:
>  - fixes and improvements from Dmitry Torokhov
>
>  drivers/media/pci/ttpci/av7110.h|  2 +-
>  drivers/media/pci/ttpci/av7110_ir.c | 56 
> +
>  2 files changed, 21 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/media/pci/ttpci/av7110.h 
> b/drivers/media/pci/ttpci/av7110.h
> index 347827925c14..bcb72ecbedc0 100644
> --- a/drivers/media/pci/ttpci/av7110.h
> +++ b/drivers/media/pci/ttpci/av7110.h
> @@ -93,7 +93,7 @@ struct infrared {
> u8  inversion;
> u16 last_key;
> u16 last_toggle;
> -   u8  delay_timer_finished;
> +   boolkeypressed;
>  };
>
>
> diff --git a/drivers/media/pci/ttpci/av7110_ir.c 
> b/drivers/media/pci/ttpci/av7110_ir.c
> index ca05198de2c2..ee414803e6b5 100644
> --- a/drivers/media/pci/ttpci/av7110_ir.c
> +++ b/drivers/media/pci/ttpci/av7110_ir.c
> @@ -84,15 +84,16 @@ static u16 default_key_map [256] = {
>
>
>  /* key-up timer */
> -static void av7110_emit_keyup(unsigned long parm)
> +static void av7110_emit_keyup(struct timer_list *t)
>  {
> -   struct infrared *ir = (struct infrared *) parm;
> +   struct infrared *ir = from_timer(ir, t, keyup_timer);
>
> -   if (!ir || !test_bit(ir->last_key, ir->input_dev->key))
> +   if (!ir || !ir->keypressed)
> return;
>
> input_report_key(ir->input_dev, ir->last_key, 0);
> input_sync(ir->input_dev);
> +   ir->keypressed = false;
>  }
>
>
> @@ -152,29 +153,18 @@ static void av7110_emit_key(unsigned long parm)
> return;
> }
>
> -   if (timer_pending(>keyup_timer)) {
> -   del_timer(>keyup_timer);
> -   if (ir->last_key != keycode || toggle != ir->last_toggle) {
> -   ir->delay_timer_finished = 0;
> -   input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
> -   input_event(ir->input_dev, EV_KEY, keycode, 1);
> -   input_sync(ir->input_dev);
> -   } else if (ir->delay_timer_finished) {
> -   input_event(ir->input_dev, EV_KEY, keycode, 2);
> -   input_sync(ir->input_dev);
> -   }
> -   } else {
> -   ir->delay_timer_finished = 0;
> -   input_event(ir->input_dev, EV_KEY, keycode, 1);
> -   input_sync(ir->input_dev);
> -   }
> +   if (ir->keypressed &&
> +   (ir->last_key != keycode || toggle != ir->last_toggle))
> +   input_event(ir->input_dev, EV_KEY, ir->last_key, 0);
>
> +   input_event(ir->input_dev, EV_KEY, keycode, 1);
> +   input_sync(ir->input_dev);
> +
> +   ir->keypressed = true;
> ir->last_key = keycode;
> ir->last_toggle = toggle;
>
> -   ir->keyup_timer.expires = jiffies + UP_TIMEOUT;
> -   add_timer(>keyup_timer);
> -
> +   mod_timer(>keyup_timer, jiffies + UP_TIMEOUT);
>  }
>
>
> @@ -204,16 +194,6 @@ static void input_register_keys(struct infrared *ir)
> ir->input_dev->keycodemax = ARRAY_SIZE(ir->key_map);
>  }
>
> -
> -/* called by the input driver after rep[REP_DELAY] ms */
> -static void input_repeat_key(unsigned long parm)
> -{
> -   struct infrared *ir = (struct infrared *) parm;
> -
> -   ir->delay_timer_finished = 1;
> -}
> -
> -
>  /* check for configuration changes */
>  int av7110_check_ir_config(struct av7110 *av7110, int force)
>  {
> @@ -333,8 +313,7 @@ int av7110_ir_init(struct av7110 *av7110)
> av_list[av_cnt++] = av7110;
> av7110_check_ir_config(av7110, true);
>
> -   setup_timer(>ir.keyup_timer, av7110_emit_keyup,
> -   (unsigned long)>ir);
> +   timer_setup(>ir.keyup_timer, av7110_emit_keyup, 0);
>
> input_dev = input_allocate_device();
> if (!input_dev)
> @@ -365,8 +344,13 @@ int av7110_ir_init(struct av7110 *av7110)
> input_free_device(input_dev);
> return err;
> }
> -   input_dev->timer.function = input_repeat_key;
> -   input_dev->timer.data = (unsigned long) >ir;
> +
> +   /*
> +* Input core's default autorepeat is 33 cps with 250 msec
> +* delay, let's adjust to numbers more suitable for remote
> +* control.
> +*/
> +   input_enable_softrepeat(input_dev, 250, 125);
>
> if (av_cnt == 1) {
> e = proc_create("av7110_ir", S_IWUSR, NULL, 
> _ir_proc_fops);
> --
> 2.13.6
>



-- 
Kees 

[PATCH] libv4lconvert: We support more then 32 bit src fmts now, so use 64 bit bitmasks

[Hans de Goede]

We support more then 32 bit src fmts now, so we can no longer re-use
struct v4l2_frmsizeenum.pixel_format to store a bitmask of all the
supported src-formats for a given frame-size.

This fixes a subtile bug where we would try to use SE401 as src fmt
instead of YUYV under certain circumstances.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1508706
Signed-off-by: Hans de Goede 
---
 lib/libv4lconvert/libv4lconvert-priv.h | 2 ++
 lib/libv4lconvert/libv4lconvert.c  | 9 -
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/lib/libv4lconvert/libv4lconvert-priv.h 
b/lib/libv4lconvert/libv4lconvert-priv.h
index e2389347..9a467e10 100644
--- a/lib/libv4lconvert/libv4lconvert-priv.h
+++ b/lib/libv4lconvert/libv4lconvert-priv.h
@@ -66,6 +66,8 @@ struct v4lconvert_data {
int cinfo_initialized;
 #endif // HAVE_JPEG
struct v4l2_frmsizeenum framesizes[V4LCONVERT_MAX_FRAMESIZES];
+   /* Bitmask of all supported src_formats which can do for a size */
+   int64_t framesize_supported_src_formats[V4LCONVERT_MAX_FRAMESIZES];
unsigned int no_framesizes;
int bandwidth;
int fps;
diff --git a/lib/libv4lconvert/libv4lconvert.c 
b/lib/libv4lconvert/libv4lconvert.c
index 1a5ccec2..d666bd97 100644
--- a/lib/libv4lconvert/libv4lconvert.c
+++ b/lib/libv4lconvert/libv4lconvert.c
@@ -434,7 +434,8 @@ static int v4lconvert_do_try_format_uvc(struct 
v4lconvert_data *data,
 
for (i = 0; i < ARRAY_SIZE(supported_src_pixfmts); i++) {
/* is this format supported? */
-   if (!(data->framesizes[best_framesize].pixel_format & (1 << i)))
+   if (!(data->framesize_supported_src_formats[best_framesize] &
+ (1ULL << i)))
continue;
 
/* Note the hardcoded use of discrete is based on this function
@@ -1647,9 +1648,7 @@ static void v4lconvert_get_framesizes(struct 
v4lconvert_data *data,
return;
}
data->framesizes[data->no_framesizes].type = 
frmsize.type;
-   /* We use the pixel_format member to store a bitmask of 
all
-  supported src_formats which can do this size */
-   data->framesizes[data->no_framesizes].pixel_format = 1 
<< index;
+   
data->framesize_supported_src_formats[data->no_framesizes] = 1ULL << index;
 
switch (frmsize.type) {
case V4L2_FRMSIZE_TYPE_DISCRETE:
@@ -1662,7 +1661,7 @@ static void v4lconvert_get_framesizes(struct 
v4lconvert_data *data,
}
data->no_framesizes++;
} else {
-   data->framesizes[j].pixel_format |= 1 << index;
+   data->framesize_supported_src_formats[j] |= 1ULL << 
index;
}
}
 }
-- 
2.14.3

[PATCH 4/4] dma-buf: Use rcu_assign_pointer() to set rcu protected pointers

[Ville Syrjala]

From: Ville Syrjälä 

Use rcu_assign_pointer() when setting an rcu protected pointer.
This gets rid of another sparse warning.

Cc: Dave Airlie 
Cc: Jason Ekstrand 
Cc: linaro-mm-...@lists.linaro.org
Cc: linux-media@vger.kernel.org
Cc: Alex Deucher 
Cc: Christian König 
Cc: Sumit Semwal 
Cc: Chris Wilson 
Signed-off-by: Ville Syrjälä 
---
 drivers/dma-buf/reservation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index b44d9d7db347..d90333e0b6d5 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -318,7 +318,7 @@ int reservation_object_copy_fences(struct 
reservation_object *dst,
continue;
}
 
-   dst_list->shared[dst_list->shared_count++] = fence;
+   
rcu_assign_pointer(dst_list->shared[dst_list->shared_count++], fence);
}
} else {
dst_list = NULL;
-- 
2.13.6

[PATCH 2/4] dma-buf/fence: Sparse wants __rcu on the object itself

[Ville Syrjala]

From: Chris Wilson 

In order to silent sparse in dma_fence_get_rcu_safe(), we need to mark
the incoming fence object as being RCU protected and not the pointer to
the object.

Cc: Dave Airlie 
Cc: Jason Ekstrand 
Cc: linaro-mm-...@lists.linaro.org
Cc: linux-media@vger.kernel.org
Cc: Alex Deucher 
Cc: Christian König 
Cc: Sumit Semwal 
Signed-off-by: Chris Wilson 
---
 include/linux/dma-fence.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index efdabbb64e3c..4c008170fe65 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -242,7 +242,7 @@ static inline struct dma_fence *dma_fence_get_rcu(struct 
dma_fence *fence)
  * The caller is required to hold the RCU read lock.
  */
 static inline struct dma_fence *
-dma_fence_get_rcu_safe(struct dma_fence * __rcu *fencep)
+dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep)
 {
do {
struct dma_fence *fence;
-- 
2.13.6

[PATCH 3/4] drm/syncobj: Use proper methods for accessing rcu protected pointers

[Ville Syrjala]

From: Ville Syrjälä 

Use rcu_dereference_protected() and rcu_assign_pointer() for accessing
the rcu protected syncobj->fence pointer. This eliminates several sparse
warnings.

Cc: Dave Airlie 
Cc: Jason Ekstrand 
Cc: linaro-mm-...@lists.linaro.org
Cc: linux-media@vger.kernel.org
Cc: Alex Deucher 
Cc: Christian König 
Cc: Sumit Semwal 
Cc: Chris Wilson 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_syncobj.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index f776fc1cc543..9b733c510cbf 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -106,7 +106,8 @@ static int drm_syncobj_fence_get_or_add_callback(struct 
drm_syncobj *syncobj,
 * callback when a fence has already been set.
 */
if (syncobj->fence) {
-   *fence = dma_fence_get(syncobj->fence);
+   *fence = dma_fence_get(rcu_dereference_protected(syncobj->fence,
+
lockdep_is_held(>lock)));
ret = 1;
} else {
*fence = NULL;
@@ -168,8 +169,9 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
 
spin_lock(>lock);
 
-   old_fence = syncobj->fence;
-   syncobj->fence = fence;
+   old_fence = rcu_dereference_protected(syncobj->fence,
+ lockdep_is_held(>lock));
+   rcu_assign_pointer(syncobj->fence, fence);
 
if (fence != old_fence) {
list_for_each_entry_safe(cur, tmp, >cb_list, node) {
@@ -659,7 +661,8 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj 
*syncobj,
container_of(cb, struct syncobj_wait_entry, syncobj_cb);
 
/* This happens inside the syncobj lock */
-   wait->fence = dma_fence_get(syncobj->fence);
+   wait->fence = dma_fence_get(rcu_dereference_protected(syncobj->fence,
+ 
lockdep_is_held(>lock)));
wake_up_process(wait->task);
 }
 
-- 
2.13.6

[PATCH 1/4] drm/syncobj: Mark up the fence as an RCU protected pointer

[Ville Syrjala]

From: Chris Wilson 

We take advantage of that syncobj->fence is an RCU-protected pointer, and
so sparse complains that it is lacking annotation.

Cc: Dave Airlie 
Cc: Jason Ekstrand 
Cc: linaro-mm-...@lists.linaro.org
Cc: linux-media@vger.kernel.org
Cc: Alex Deucher 
Cc: Christian König 
Cc: Sumit Semwal 
Signed-off-by: Chris Wilson 
---
 include/drm/drm_syncobj.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index 43e2f382d2f0..9e8ba90c6784 100644
--- a/include/drm/drm_syncobj.h
+++ b/include/drm/drm_syncobj.h
@@ -49,7 +49,7 @@ struct drm_syncobj {
 * This field should not be used directly.  Use drm_syncobj_fence_get
 * and drm_syncobj_replace_fence instead.
 */
-   struct dma_fence *fence;
+   struct dma_fence __rcu *fence;
/**
 * @cb_list:
 * List of callbacks to call when the fence gets replaced
-- 
2.13.6

[PATCH 0/4] dma-buf: Silence dma_fence __rcu sparse warnings

[Ville Syrjala]

From: Ville Syrjälä 

When building drm+i915 I get around 150 lines of sparse noise from 
dma_fence __rcu warnings. This series eliminates all of that.

The first two patches were already posted by Chris, but there wasn't
any real reaction, so I figured I'd repost with a wider Cc list.

As for the other two patches, I'm no expert on dma_fence and I didn't
spend a lot of time looking at it so I can't be sure I annotated all
the accesses correctly. But I figured someone will scream at me if
I got it wrong ;)

Cc: Dave Airlie 
Cc: Jason Ekstrand 
Cc: linaro-mm-...@lists.linaro.org
Cc: linux-media@vger.kernel.org
Cc: Alex Deucher 
Cc: Christian König 
Cc: Sumit Semwal 
Cc: Chris Wilson 

Chris Wilson (2):
  drm/syncobj: Mark up the fence as an RCU protected pointer
  dma-buf/fence: Sparse wants __rcu on the object itself

Ville Syrjälä (2):
  drm/syncobj: Use proper methods for accessing rcu protected pointers
  dma-buf: Use rcu_assign_pointer() to set rcu protected pointers

 drivers/dma-buf/reservation.c |  2 +-
 drivers/gpu/drm/drm_syncobj.c | 11 +++
 include/drm/drm_syncobj.h |  2 +-
 include/linux/dma-fence.h |  2 +-
 4 files changed, 10 insertions(+), 7 deletions(-)

-- 
2.13.6

[PATCH v7 3/3] intel-ipu3: cio2: Add new MIPI-CSI2 driver

[Yong Zhi]

This patch adds CIO2 CSI-2 device driver for
Intel's IPU3 camera sub-system support.

Signed-off-by: Yong Zhi 
Signed-off-by: Hyungwoo Yang 
Signed-off-by: Rajmohan Mani 
Signed-off-by: Vijaykumar Ramya 
---
 drivers/media/pci/Kconfig|2 +
 drivers/media/pci/Makefile   |3 +-
 drivers/media/pci/intel/Makefile |5 +
 drivers/media/pci/intel/ipu3/Kconfig |   19 +
 drivers/media/pci/intel/ipu3/Makefile|1 +
 drivers/media/pci/intel/ipu3/ipu3-cio2.c | 2053 ++
 drivers/media/pci/intel/ipu3/ipu3-cio2.h |  449 +++
 7 files changed, 2531 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/pci/intel/Makefile
 create mode 100644 drivers/media/pci/intel/ipu3/Kconfig
 create mode 100644 drivers/media/pci/intel/ipu3/Makefile
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-cio2.c
 create mode 100644 drivers/media/pci/intel/ipu3/ipu3-cio2.h

diff --git a/drivers/media/pci/Kconfig b/drivers/media/pci/Kconfig
index da28e68c87d8..5932e225f9c0 100644
--- a/drivers/media/pci/Kconfig
+++ b/drivers/media/pci/Kconfig
@@ -54,5 +54,7 @@ source "drivers/media/pci/smipcie/Kconfig"
 source "drivers/media/pci/netup_unidvb/Kconfig"
 endif
 
+source "drivers/media/pci/intel/ipu3/Kconfig"
+
 endif #MEDIA_PCI_SUPPORT
 endif #PCI
diff --git a/drivers/media/pci/Makefile b/drivers/media/pci/Makefile
index a7e8af0f64a7..d8f98434fb8c 100644
--- a/drivers/media/pci/Makefile
+++ b/drivers/media/pci/Makefile
@@ -13,7 +13,8 @@ obj-y+=   ttpci/  \
ddbridge/   \
saa7146/\
smipcie/\
-   netup_unidvb/
+   netup_unidvb/   \
+   intel/
 
 obj-$(CONFIG_VIDEO_IVTV) += ivtv/
 obj-$(CONFIG_VIDEO_ZORAN) += zoran/
diff --git a/drivers/media/pci/intel/Makefile b/drivers/media/pci/intel/Makefile
new file mode 100644
index ..745c8b2a7819
--- /dev/null
+++ b/drivers/media/pci/intel/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the IPU3 cio2 and ImGU drivers
+#
+
+obj-y  += ipu3/
diff --git a/drivers/media/pci/intel/ipu3/Kconfig 
b/drivers/media/pci/intel/ipu3/Kconfig
new file mode 100644
index ..4bb8e0e97e49
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/Kconfig
@@ -0,0 +1,19 @@
+config VIDEO_IPU3_CIO2
+   tristate "Intel ipu3-cio2 driver"
+   depends on VIDEO_V4L2 && PCI
+   depends on VIDEO_V4L2_SUBDEV_API
+   depends on X86 || COMPILE_TEST
+   depends on MEDIA_CONTROLLER
+   depends on HAS_DMA
+   depends on ACPI
+   select V4L2_FWNODE
+   select VIDEOBUF2_DMA_SG
+
+   ---help---
+   This is the Intel IPU3 CIO2 CSI-2 receiver unit, found in Intel
+   Skylake and Kaby Lake SoCs and used for capturing images and
+   video from a camera sensor.
+
+   Say Y or M here if you have a Skylake/Kaby Lake SoC with MIPI CSI-2
+   connected camera.
+   The module will be called ipu3-cio2.
diff --git a/drivers/media/pci/intel/ipu3/Makefile 
b/drivers/media/pci/intel/ipu3/Makefile
new file mode 100644
index ..20186e3ff2ae
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c 
b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
new file mode 100644
index ..f750ae6f746c
--- /dev/null
+++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c
@@ -0,0 +1,2053 @@
+/*
+ * Copyright (c) 2017 Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * 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.
+ *
+ * Based partially on Intel IPU4 driver written by
+ *  Sakari Ailus 
+ *  Samu Onkalo 
+ *  Jouni Högander 
+ *  Jouni Ukkonen 
+ *  Antti Laakso 
+ * et al.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ipu3-cio2.h"
+
+struct ipu3_cio2_fmt {
+   u32 mbus_code;
+   u32 fourcc;
+   u8 mipicode;
+};
+
+/*
+ * These are raw formats used in Intel's third generation of
+ * Image Processing Unit known as IPU3.
+ * 10bit raw bayer packed, 32 bytes for every 25 pixels,
+ * last LSB 6 bits unused.
+ */
+static const struct ipu3_cio2_fmt formats[] = {
+   {   /* put default entry at beginning */
+   .mbus_code  = 

[PATCH v7 2/3] doc-rst: add IPU3 raw10 bayer pixel format definitions

[Yong Zhi]

The formats added by this patch are:

V4L2_PIX_FMT_IPU3_SBGGR10
V4L2_PIX_FMT_IPU3_SGBRG10
V4L2_PIX_FMT_IPU3_SGRBG10
V4L2_PIX_FMT_IPU3_SRGGB10

Signed-off-by: Hyungwoo Yang 
Signed-off-by: Yong Zhi 
---
 Documentation/media/uapi/v4l/pixfmt-rgb.rst|   1 +
 .../media/uapi/v4l/pixfmt-srggb10-ipu3.rst | 335 +
 2 files changed, 336 insertions(+)
 create mode 100644 Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst

diff --git a/Documentation/media/uapi/v4l/pixfmt-rgb.rst 
b/Documentation/media/uapi/v4l/pixfmt-rgb.rst
index 4cc27195dc79..cf2ef7df9616 100644
--- a/Documentation/media/uapi/v4l/pixfmt-rgb.rst
+++ b/Documentation/media/uapi/v4l/pixfmt-rgb.rst
@@ -16,6 +16,7 @@ RGB Formats
 pixfmt-srggb10p
 pixfmt-srggb10alaw8
 pixfmt-srggb10dpcm8
+pixfmt-srggb10-ipu3
 pixfmt-srggb12
 pixfmt-srggb12p
 pixfmt-srggb16
diff --git a/Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst 
b/Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst
new file mode 100644
index ..72fbd8f96381
--- /dev/null
+++ b/Documentation/media/uapi/v4l/pixfmt-srggb10-ipu3.rst
@@ -0,0 +1,335 @@
+.. -*- coding: utf-8; mode: rst -*-
+
+.. _V4L2_PIX_FMT_IPU3_SBGGR10:
+.. _V4L2_PIX_FMT_IPU3_SGBRG10:
+.. _V4L2_PIX_FMT_IPU3_SGRBG10:
+.. _V4L2_PIX_FMT_IPU3_SRGGB10:
+
+**
+V4L2_PIX_FMT_IPU3_SBGGR10 ('ip3b'), V4L2_PIX_FMT_IPU3_SGBRG10 ('ip3g'), 
V4L2_PIX_FMT_IPU3_SGRBG10 ('ip3G'), V4L2_PIX_FMT_IPU3_SRGGB10 ('ip3r')
+**
+
+10-bit Bayer formats
+
+Description
+===
+
+These four pixel formats are used by Intel IPU3 driver, they are raw
+sRGB / Bayer formats with 10 bits per sample with every 25 pixels packed
+to 32 bytes leaving 6 most significant bits padding in the last byte.
+The format is little endian.
+
+In other respects this format is similar to :ref:`V4L2-PIX-FMT-SRGGB10`.
+Below is an example of a small image in V4L2_PIX_FMT_IPU3_SBGGR10 format.
+
+**Byte Order.**
+Each cell is one byte.
+
+.. tabularcolumns:: |p{0.8cm}|p{4.0cm}|p{4.0cm}|p{4.0cm}|p{4.0cm}|
+
+.. flat-table::
+
+* - start + 0:
+  - B\ :sub:`low`
+  - G\ :sub:`0001low`\ (bits 7--2)
+
+B\ :sub:`high`\ (bits 1--0)
+  - B\ :sub:`0002low`\ (bits 7--4)
+
+G\ :sub:`0001high`\ (bits 3--0)
+  - G\ :sub:`0003low`\ (bits 7--6)
+
+B\ :sub:`0002high`\ (bits 5--0)
+* - start + 4:
+  - G\ :sub:`0003high`
+  - B\ :sub:`0004low`
+  - G\ :sub:`0005low`\ (bits 7--2)
+
+B\ :sub:`0004high`\ (bits 1--0)
+  - B\ :sub:`0006low`\ (bits 7--4)
+
+G\ :sub:`0005high`\ (bits 3--0)
+* - start + 8:
+  - G\ :sub:`0007low`\ (bits 7--6)
+
+B\ :sub:`0006high`\ (bits 5--0)
+  - G\ :sub:`0007high`
+  - B\ :sub:`0008low`
+  - G\ :sub:`0009low`\ (bits 7--2)
+
+B\ :sub:`0008high`\ (bits 1--0)
+* - start + 12:
+  - B\ :sub:`0010low`\ (bits 7--4)
+
+G\ :sub:`0009high`\ (bits 3--0)
+  - G\ :sub:`0011low`\ (bits 7--6)
+
+B\ :sub:`0010high`\ (bits 5--0)
+  - G\ :sub:`0011high`
+  - B\ :sub:`0012low`
+* - start + 16:
+  - G\ :sub:`0013low`\ (bits 7--2)
+
+B\ :sub:`0012high`\ (bits 1--0)
+  - B\ :sub:`0014low`\ (bits 7--4)
+
+G\ :sub:`0013high`\ (bits 3--0)
+  - G\ :sub:`0015low`\ (bits 7--6)
+
+B\ :sub:`0014high`\ (bits 5--0)
+  - G\ :sub:`0015high`
+* - start + 20
+  - B\ :sub:`0016low`
+  - G\ :sub:`0017low`\ (bits 7--2)
+
+B\ :sub:`0016high`\ (bits 1--0)
+  - B\ :sub:`0018low`\ (bits 7--4)
+
+G\ :sub:`0017high`\ (bits 3--0)
+  - G\ :sub:`0019low`\ (bits 7--6)
+
+B\ :sub:`0018high`\ (bits 5--0)
+* - start + 24:
+  - G\ :sub:`0019high`
+  - B\ :sub:`0020low`
+  - G\ :sub:`0021low`\ (bits 7--2)
+
+B\ :sub:`0020high`\ (bits 1--0)
+  - B\ :sub:`0022low`\ (bits 7--4)
+
+G\ :sub:`0021high`\ (bits 3--0)
+* - start + 28:
+  - G\ :sub:`0023low`\ (bits 7--6)
+
+B\ :sub:`0022high`\ (bits 5--0)
+  - G\ :sub:`0023high`
+  - B\ :sub:`0024low`
+  - B\ :sub:`0024high`\ (bits 1--0)
+* - start + 32:
+  - G\ :sub:`0100low`
+  - R\ :sub:`0101low`\ (bits 7--2)
+
+G\ :sub:`0100high`\ (bits 1--0)
+  - G\ :sub:`0102low`\ (bits 7--4)
+
+R\ :sub:`0101high`\ (bits 3--0)
+  - R\ :sub:`0103low`\ (bits 7--6)
+
+G\ :sub:`0102high`\ (bits 5--0)
+* - start + 36:
+  - R\ :sub:`0103high`
+  - G\ :sub:`0104low`
+  - R\ :sub:`0105low`\ (bits 7--2)
+
+G\ :sub:`0104high`\ (bits 1--0)
+  - G\ :sub:`0106low`\ (bits 7--4)
+
+R\ :sub:`0105high`\ (bits 3--0)
+* 

[PATCH v7 1/3] videodev2.h, v4l2-ioctl: add IPU3 raw10 color format

[Yong Zhi]

Add IPU3 specific formats:

V4L2_PIX_FMT_IPU3_SBGGR10
V4L2_PIX_FMT_IPU3_SGBRG10
V4L2_PIX_FMT_IPU3_SGRBG10
V4L2_PIX_FMT_IPU3_SRGGB10

Signed-off-by: Yong Zhi 
---
 drivers/media/v4l2-core/v4l2-ioctl.c | 4 
 include/uapi/linux/videodev2.h   | 6 ++
 2 files changed, 10 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
b/drivers/media/v4l2-core/v4l2-ioctl.c
index 79614992ee21..3937945b12dc 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1202,6 +1202,10 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
case V4L2_PIX_FMT_SGBRG10P: descr = "10-bit Bayer GBGB/RGRG 
Packed"; break;
case V4L2_PIX_FMT_SGRBG10P: descr = "10-bit Bayer GRGR/BGBG 
Packed"; break;
case V4L2_PIX_FMT_SRGGB10P: descr = "10-bit Bayer RGRG/GBGB 
Packed"; break;
+   case V4L2_PIX_FMT_IPU3_SBGGR10: descr = "10-bit bayer BGGR IPU3 
Packed"; break;
+   case V4L2_PIX_FMT_IPU3_SGBRG10: descr = "10-bit bayer GBRG IPU3 
Packed"; break;
+   case V4L2_PIX_FMT_IPU3_SGRBG10: descr = "10-bit bayer GRBG IPU3 
Packed"; break;
+   case V4L2_PIX_FMT_IPU3_SRGGB10: descr = "10-bit bayer RGGB IPU3 
Packed"; break;
case V4L2_PIX_FMT_SBGGR10ALAW8: descr = "8-bit Bayer BGBG/GRGR 
(A-law)"; break;
case V4L2_PIX_FMT_SGBRG10ALAW8: descr = "8-bit Bayer GBGB/RGRG 
(A-law)"; break;
case V4L2_PIX_FMT_SGRBG10ALAW8: descr = "8-bit Bayer GRGR/BGBG 
(A-law)"; break;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 185d6a0acc06..bcf6a50f6aac 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -668,6 +668,12 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_MT21Cv4l2_fourcc('M', 'T', '2', '1') /* Mediatek 
compressed block mode  */
 #define V4L2_PIX_FMT_INZI v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Planar 
Greyscale 10-bit and Depth 16-bit */
 
+/* 10bit raw bayer packed, 32 bytes for every 25 pixels, last LSB 6 bits 
unused */
+#define V4L2_PIX_FMT_IPU3_SBGGR10  v4l2_fourcc('i', 'p', '3', 'b') /* IPU3 
packed 10-bit BGGR bayer */
+#define V4L2_PIX_FMT_IPU3_SGBRG10  v4l2_fourcc('i', 'p', '3', 'g') /* IPU3 
packed 10-bit GBRG bayer */
+#define V4L2_PIX_FMT_IPU3_SGRBG10  v4l2_fourcc('i', 'p', '3', 'G') /* IPU3 
packed 10-bit GRBG bayer */
+#define V4L2_PIX_FMT_IPU3_SRGGB10  v4l2_fourcc('i', 'p', '3', 'r') /* IPU3 
packed 10-bit RGGB bayer */
+
 /* SDR formats - used only for Software Defined Radio devices */
 #define V4L2_SDR_FMT_CU8  v4l2_fourcc('C', 'U', '0', '8') /* IQ u8 */
 #define V4L2_SDR_FMT_CU16LE   v4l2_fourcc('C', 'U', '1', '6') /* IQ u16le 
*/
-- 
2.7.4

[PATCH v7 0/3] add IPU3 CIO2 CSI2 driver

[Yong Zhi]

Hi,

This is patch series(version 7) of Intel IPU3 CIO2 driver, the driver exposes
V4L2, V4L2 sub-device and Media controller interfaces to the user space.

This series was tested on Kaby Lake based platform with 2 sensor configurations,
media topology was pasted at end for reference.

Link to user space implementation:



===
= history =
===

version 7:
- cio2_subdev_open(): Assign variable at init time. (Sakari)
- cio2_subdev_get_fmt(): Retrieve subdev fmt from subdev not sensor. (Sakari)
- cio2_fbpt_rearrange(): Use in-place swapping of FBPT entries and queue 
buffers.
  (implemented by Sakari).

version 6:
- Replaced NUM_FORMATS with ARRAY_SIZE(formats). (Sakari)
- cio2_fbpt_rearrange(): skip fbpt re-arrange when not needed.
- cio2_resume(): move cio2_fbpt_rearrange() to cio2_suspend() to avoid mem alloc
  at resume time.(Sakari)
- cio2_pci_probe(): call cio2_fbpt_init_dummy() before 
v4l2_device_register().(Sakari)
- Fixed checkpatch.pl reported checks with --strict option.
- Added static keyword to cio2_queue_init().
- cio2_buffer_done(): assign ns in variable initialization.(Sakari)
- cio2_notifier_bound(): create hard link between cio2 queues and CSI-2 
port.(Sakari)
- cio2_notifier_unbount(): remove obsolete comments.(Sakari)
- cio2_notifier_init(): return error when no subdevs found.(Sakari)
- cio2_find_queue_by_sensor_node(): remove the function (Tomasz Figa)
- ipu3-cio2.h:
  Change CIO2_QUEUES 4 to match IPU3 capability.(Sakari)
  Remove un-used "other_entries" in struct cio2_fbpt_entry.(Sakari)
- pixfmt-srggb10-ipu3.rst: Changed tabularcolumns to 4.(Sakari)
- Enable x86 32bit build (Sakari)

version 5:
- cio2_vb2_start_streaming():
- cio2_vb2_stop_streaming(): removed redundant call of csi2 sub-dev for 
s_stream.
- cio2_vb2_buf_queue(): disabled interrupts for the duration of the buf queue,
  to prevent this code from being pre-empted, as suggested by Tomasz Figa,
  to mitigate the effects of race conditions around vb2 buf queuing code.
  Switched to a finite loop to check for the first free buffer and errored
  out, when there are no buffers available. Removed calls to vb2_plane_vaddr()
  Maintain correct buf queued count, in error cases.
- Implemented system sleep pm ops to support cio2 driver suspend/resume.
- Made the v4l2 buffer and SOF event use sequence from same source.
- cio2_vb2_queue_setup(): remove validating pixelformat suggested by Tomasz
  Figa.
- cio2_v4l2_g_fmt()/cio2_v4l2_s_fmt(): seperated formats on sub-dev and video
  device suggested by Sakari Ailus.
- cio2_v4l2_try_fmt(): seperated video node and subdev format in the get_fmt,
  try_fmt and set_fmt callbacks.
- cio2_queue_event_sof(): added comments suggested by Hans Verkuil
- cio2_queue_init(): re-ordered q->subdev_pads settings. remove 4 lines for
  quantization init.
- cio2_subdev_get_fmt(): get colorspace/xfer_func/ycbcr_enc/quantization
  from sensor suggested by Hans Verkuil.
- cio2_fbpt_entry_init_buf(): stored offset of the first sg_list entry to
  remove calls to vb2_plane_vaddr().
- cio2_subdev_open(): added new callback to intialize the try format.
- cio2_subdev_video_ops(): removed empty implementation suggested by Sakari 
Ailus.
- cio2_notifier_init(): added fwnode binding support for subdevices using
  v4l2_async_notifier_parse_fwnode_endpoints()
  Patch series v15 Unified fwnode endpoint parser, async sub-device notifier
  support, N9 flash DTS is needed for the fwnode binding code to compile.
  https://www.mail-archive.com/linux-media@vger.kernel.org/msg120239.html
  This also requires the following patch (v1) for the fwnode binding to work
  https://patchwork.kernel.org/patch/9986445/
- cio2_notifier_complete(): removed redundant call of
  fwnode_graph_get_remote_endpoint() and fwnode_graph_parse_endpoint().
- Switched to Multi Plane APIs suggested by Tomasz Figa.
  User space changes supporting multi plane APIs can be found here
  
https://chromium-review.googlesource.com/c/chromiumos/platform/arc-camera/+/683802
- ipu3-cio.h: moved macros out of struct cio2_fbpt_entry suggested by Hans 
Verkuil.
- cio2_hw_mbus_to_mipicode(): replaced with cio2_find_format().
- cio2_pci_probe(): cleaned up goto logic on error conditions suggested by 
Tomasz Figa.
- Fixed v4l2_compliance test failures
  added 3 dummy function to pass v4l2_compliance test.
- Extended format example in pixfmt-srggb10-ipu3.rst to show DMA word boundary.

version 4:
- add cio2_video_link_validate() for video entity suggested by Sakari Ailus
- cio2_notifier_complete(): fix comments suggested by Sakari Ailus
- cio2_vb2_buf_queue(): fix the forever loop suggested by Tomasz Figa
- cio2_v4l2_querycap(): use vdev device_caps commented by Hans Verkuil
- cio2_vb2_buf_init(): allocate LOP table per page suggested by Tomasz Figa
- cio2_hw_init(): call cio2_csi2_calc_timing() earlier suggested by Tomasz Figa
- cio2_csi2_calc_timing(): add defalt settings for 

Re: 32bit userland cannot work with DVB drivers in 64bit kernel, design issue

[Alan Cox]

On Thu, 2 Nov 2017 12:16:39 +0100
Menion  wrote:

> Hi all
> I am investigating for Armbian, the feasability of running 32bit
> userland on single board computers based on arm64 SoC, where only 64
> bit kernel is available, for reducing the memory footprint.
> I have discovered that there is a flaw in the DVB frontend ioctl (at
> least) that prevents to do so.
> in frontend.h the biggest problem is:
> 
> struct dtv_properties {
> __u32 num;
> struct dtv_property *props;
> };
> 
> The master userland-kernel ioctl structure is based on an array set by
> pointer, so the 32bit userland will allocate 32bit pointer (and the
> resulting structure size) while the 64bit kernel will expect the 64bit
> pointers

And in some cases the pointer might end up aligned to the next 64bit
boundary.

> The void *reserved2 field will also give problem when crossing the
> 32-64bits boundaries
> As today the endire dvb linux infrastructure can only work in 32-32
> and 64-64 bit mode

If this isn't handled by the existing media compat_ioctl support then you
can send patches to use compat_ioctl handlers to fix this.

Alan

Re: [PATCH 1/1] media: i2c: as3645a: Remove driver

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Friday, 8 September 2017 16:51:40 EET Sakari Ailus wrote:
> Remove the V4L2 AS3645A sub-device driver in favour of the LED flash class
> driver for the same hardware, drivers/leds/leds-as3645a.c. The latter uses
> the V4L2 flash LED class framework to provide V4L2 sub-device interface.
> 
> Signed-off-by: Sakari Ailus 

Acked-by: Laurent Pinchart 

> ---
>  MAINTAINERS |   8 -
>  drivers/media/i2c/Kconfig   |   8 -
>  drivers/media/i2c/Makefile  |   1 -
>  drivers/media/i2c/as3645a.c | 880
>  include/media/i2c/as3645a.h | 
> 66 
>  5 files changed, 963 deletions(-)
>  delete mode 100644 drivers/media/i2c/as3645a.c
>  delete mode 100644 include/media/i2c/as3645a.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index eb930ebecfcb..40964ab8cbbd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2129,14 +2129,6 @@ L: linux-l...@vger.kernel.org
>  S:   Maintained
>  F:   drivers/leds/leds-as3645a.c
> 
> -AS3645A LED FLASH CONTROLLER DRIVER
> -M:   Laurent Pinchart 
> -L:   linux-media@vger.kernel.org
> -T:   git git://linuxtv.org/media_tree.git
> -S:   Maintained
> -F:   drivers/media/i2c/as3645a.c
> -F:   include/media/i2c/as3645a.h
> -
>  ASAHI KASEI AK8974 DRIVER
>  M:   Linus Walleij 
>  L:   linux-...@vger.kernel.org
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 94153895fcd4..559ee95a47cf 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -796,14 +796,6 @@ config VIDEO_ADP1653
> This is a driver for the ADP1653 flash controller. It is used for
> example in Nokia N900.
> 
> -config VIDEO_AS3645A
> - tristate "AS3645A flash driver support"
> - depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
> - depends on MEDIA_CAMERA_SUPPORT
> - ---help---
> -   This is a driver for the AS3645A and LM3555 flash controllers. It has
> -   build in control for flash, torch and indicator LEDs.
> -
>  config VIDEO_LM3560
>   tristate "LM3560 dual flash driver support"
>   depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index c843c181dfb9..e88cdb20eb72 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -83,7 +83,6 @@ obj-$(CONFIG_VIDEO_S5K4ECGX)+= s5k4ecgx.o
>  obj-$(CONFIG_VIDEO_S5K5BAF)  += s5k5baf.o
>  obj-$(CONFIG_VIDEO_S5C73M3)  += s5c73m3/
>  obj-$(CONFIG_VIDEO_ADP1653)  += adp1653.o
> -obj-$(CONFIG_VIDEO_AS3645A)  += as3645a.o
>  obj-$(CONFIG_VIDEO_LM3560)   += lm3560.o
>  obj-$(CONFIG_VIDEO_LM3646)   += lm3646.o
>  obj-$(CONFIG_VIDEO_SMIAPP_PLL)   += smiapp-pll.o
> diff --git a/drivers/media/i2c/as3645a.c b/drivers/media/i2c/as3645a.c
> deleted file mode 100644
> index af5db71a0888..
> --- a/drivers/media/i2c/as3645a.c
> +++ /dev/null
> @@ -1,880 +0,0 @@
> -/*
> - * drivers/media/i2c/as3645a.c - AS3645A and LM3555 flash controllers
> driver - *
> - * Copyright (C) 2008-2011 Nokia Corporation
> - * Copyright (c) 2011, Intel Corporation.
> - *
> - * Contact: Laurent Pinchart 
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License
> - * version 2 as published by the Free Software Foundation.
> - *
> - * 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.
> - *
> - * TODO:
> - * - Check hardware FSTROBE control when sensor driver add support for this
> - *
> - */
> -
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#include 
> -#include 
> -#include 
> -
> -#define AS_TIMER_MS_TO_CODE(t)   (((t) - 100) / 50)
> -#define AS_TIMER_CODE_TO_MS(c)   (50 * (c) + 100)
> -
> -/* Register definitions */
> -
> -/* Read-only Design info register: Reset state:  0001 */
> -#define AS_DESIGN_INFO_REG   0x00
> -#define AS_DESIGN_INFO_FACTORY(x)(((x) >> 4))
> -#define AS_DESIGN_INFO_MODEL(x)  ((x) & 0x0f)
> -
> -/* Read-only Version control register: Reset state:  
> - * for first engineering samples
> - */
> -#define AS_VERSION_CONTROL_REG   0x01
> -#define AS_VERSION_CONTROL_RFU(x)(((x) >> 4))
> -#define AS_VERSION_CONTROL_VERSION(x)((x) & 0x0f)
> -
> -/* Read / Write  (Indicator and timer register): Reset state:   
> */
> -#define AS_INDICATOR_AND_TIMER_REG   0x02
> -#define AS_INDICATOR_AND_TIMER_TIMEOUT_SHIFT 0
> -#define AS_INDICATOR_AND_TIMER_VREF_SHIFT4
> -#define 

Re: [RESEND PATCH 1/1] of: Make return types of to_of_node and of_fwnode_handle macros consistent

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Thursday, 2 November 2017 11:59:18 EET Sakari Ailus wrote:
> (Fixed Mauro's e-mail.)
> 
> to_of_node() macro checks whether the fwnode_handle passed to it is not an
> OF node, and if so, returns NULL in order to be NULL-safe. Otherwise it
> returns the pointer to the OF node which the fwnode_handle contains.
> 
> The problem with returning NULL is that its type was void *, which
> sometimes matters. Explicitly return struct device_node * instead.
> 
> Make a similar change to of_fwnode_handle() as well.
> 
> Fixes: d20dc1493db4 ("of: Support const and non-const use for to_of_node()")
> Fixes: debd3a3b27c7 ("of: Make of_fwnode_handle() safer")
> Signed-off-by: Sakari Ailus 

Reviewed-by: Laurent Pinchart 

> ---
> Hi Mauro,
> 
> Could you check whether this addresses the smatch issue with the xilinx
> driver?
> 
> Thanks.
> 
>  include/linux/of.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/of.h b/include/linux/of.h
> index b240ed69dc96..0651231c115e 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -161,7 +161,7 @@ static inline bool is_of_node(const struct fwnode_handle
> *fwnode) is_of_node(__to_of_node_fwnode) ?\
>   container_of(__to_of_node_fwnode,   \
>struct device_node, fwnode) :  \
> - NULL;   \
> + (struct device_node *)NULL; \
>   })
> 
>  #define of_fwnode_handle(node)   
> \
> @@ -169,7 +169,8 @@ static inline bool is_of_node(const struct fwnode_handle
> *fwnode) typeof(node) __of_fwnode_handle_node = (node);   \
>   \
>   __of_fwnode_handle_node ?   \
> - &__of_fwnode_handle_node->fwnode : NULL;\
> + &__of_fwnode_handle_node->fwnode :  \
> + (struct fwnode_handle *)NULL;   \
>   })
> 
>  static inline bool of_have_populated_dt(void)


-- 
Regards,

Laurent Pinchart

[PATCH] [media] au0828: make const array addr_list static

[Colin King]

From: Colin Ian King 

Don't populate array addr_list on the stack but instead make it
static. Makes the object code smaller by over 360 bytes:

Before:
   textdata bss dec hex filename
   80361488 192971625f4 au0828-input.o

After:
   textdata bss dec hex filename
   76961488 192937624a0 au0828-input.o

(gcc version 7.2.0 x86_64)

Signed-off-by: Colin Ian King 
---
 drivers/media/usb/au0828/au0828-input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/au0828/au0828-input.c 
b/drivers/media/usb/au0828/au0828-input.c
index 7996eb83a54e..af68afe085b5 100644
--- a/drivers/media/usb/au0828/au0828-input.c
+++ b/drivers/media/usb/au0828/au0828-input.c
@@ -269,7 +269,7 @@ static void au0828_rc_stop(struct rc_dev *rc)
 static int au0828_probe_i2c_ir(struct au0828_dev *dev)
 {
int i = 0;
-   const unsigned short addr_list[] = {
+   static const unsigned short addr_list[] = {
 0x47, I2C_CLIENT_END
};
 
-- 
2.14.1

[PATCH] [media] cx88: make const arrays default_addr_list and pvr2000_addr_list static

[Colin King]

From: Colin Ian King 

Don't populate arrays default_addr_list and pvr2000_addr_list on the
stack but instead make them static. Makes the object code smaller by
over 340 bytes:

Before:
   textdata bss dec hex filename
  125202800  64   153843c18 drivers/media/pci/cx88/cx88-input.o

After:
   textdata bss dec hex filename
  121422832  64   150383abe drivers/media/pci/cx88/cx88-input.o

(gcc version 7.2.0 x86_64)

Signed-off-by: Colin Ian King 
---
 drivers/media/pci/cx88/cx88-input.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/cx88/cx88-input.c 
b/drivers/media/pci/cx88/cx88-input.c
index e02449bf2041..4e9953e61a12 100644
--- a/drivers/media/pci/cx88/cx88-input.c
+++ b/drivers/media/pci/cx88/cx88-input.c
@@ -593,11 +593,11 @@ static int get_key_pvr2000(struct IR_i2c *ir, enum 
rc_proto *protocol,
 void cx88_i2c_init_ir(struct cx88_core *core)
 {
struct i2c_board_info info;
-   const unsigned short default_addr_list[] = {
+   static const unsigned short default_addr_list[] = {
0x18, 0x6b, 0x71,
I2C_CLIENT_END
};
-   const unsigned short pvr2000_addr_list[] = {
+   static const unsigned short pvr2000_addr_list[] = {
0x18, 0x1a,
I2C_CLIENT_END
};
-- 
2.14.1

[PATCH] media: drxd: make const array fastIncrDecLUT static

[Colin King]

From: Colin Ian King 

Don't populate array fastIncrDecLUT on the stack but instead make it
static. Makes the object code smaller by over 360 bytes:

   textdata bss dec hex filename
  32680 944  64   336888398 drxd_hard.o

   textdata bss dec hex filename
  322231040  64   33327822f drxd_hard.o

(gcc version 7.2.0 x86_64)

Signed-off-by: Colin Ian King 
---
 drivers/media/dvb-frontends/drxd_hard.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/drxd_hard.c 
b/drivers/media/dvb-frontends/drxd_hard.c
index 3bdf9b1f4e7c..0696bc62dcc9 100644
--- a/drivers/media/dvb-frontends/drxd_hard.c
+++ b/drivers/media/dvb-frontends/drxd_hard.c
@@ -640,7 +640,7 @@ static int SetCfgIfAgc(struct drxd_state *state, struct 
SCfgAgc *cfg)
const u16 maxRur = 8;
static const u16 slowIncrDecLUT[] = {
3, 4, 4, 5, 6 };
-   const u16 fastIncrDecLUT[] = {
+   static const u16 fastIncrDecLUT[] = {
14, 15, 15, 16,
17, 18, 18, 19,
20, 21, 22, 23,
-- 
2.14.1

[PATCH] media: dib0700: fix invalid dvb_detach argument

[Andrey Konovalov]

dvb_detach(arg) calls symbol_put_addr(arg), where arg should be a pointer
to a function. Right now a pointer to state->dib7000p_ops is passed to
dvb_detach(), which causes a BUG() in symbol_put_addr() as discovered by
syzkaller. Pass state->dib7000p_ops.set_wbd_ref instead.

[ cut here ]
kernel BUG at kernel/module.c:1081!
invalid opcode:  [#1] PREEMPT SMP KASAN
Modules linked in:
CPU: 1 PID: 1151 Comm: kworker/1:1 Tainted: GW
4.14.0-rc1-42251-gebb2c2437d80 #224
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
task: 88006a336300 task.stack: 88006a7c8000
RIP: 0010:symbol_put_addr+0x54/0x60 kernel/module.c:1083
RSP: 0018:88006a7ce210 EFLAGS: 00010246
RAX:  RBX: 880062a8d190 RCX: 
RDX: dc20 RSI: 85876d60 RDI: 880062a8d190
RBP: 88006a7ce218 R08: 11000d4f9c12 R09: 11000d4f9ae4
R10: 11000d4f9bed R11:  R12: 880062a8d180
R13: ffed R14: 880062a8d190 R15: 88006947c000
FS:  () GS:88006c90() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 7f6416532000 CR3: 632f5000 CR4: 06e0
Call Trace:
 stk7070p_frontend_attach+0x515/0x610
drivers/media/usb/dvb-usb/dib0700_devices.c:1013
 dvb_usb_adapter_frontend_init+0x32b/0x660
drivers/media/usb/dvb-usb/dvb-usb-dvb.c:286
 dvb_usb_adapter_init drivers/media/usb/dvb-usb/dvb-usb-init.c:86
 dvb_usb_init drivers/media/usb/dvb-usb/dvb-usb-init.c:162
 dvb_usb_device_init+0xf70/0x17f0 drivers/media/usb/dvb-usb/dvb-usb-init.c:277
 dib0700_probe+0x171/0x5a0 drivers/media/usb/dvb-usb/dib0700_core.c:886
 usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
 __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
 bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
 __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
 device_add+0xd0b/0x1660 drivers/base/core.c:1835
 usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
 generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
 usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
 __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
 bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
 __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
 device_add+0xd0b/0x1660 drivers/base/core.c:1835
 usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
 hub_port_connect drivers/usb/core/hub.c:4903
 hub_port_connect_change drivers/usb/core/hub.c:5009
 port_event drivers/usb/core/hub.c:5115
 hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
 process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
 worker_thread+0x221/0x1850 kernel/workqueue.c:2253
 kthread+0x3a1/0x470 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
Code: ff ff 48 85 c0 74 24 48 89 c7 e8 48 ea ff ff bf 01 00 00 00 e8
de 20 e3 ff 65 8b 05 b7 2f c2 7e 85 c0 75 c9 e8 f9 0b c1 ff eb c2 <0f>
0b 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 b8 00 00
RIP: symbol_put_addr+0x54/0x60 RSP: 88006a7ce210
---[ end trace b75b357739e7e116 ]---

Signed-off-by: Andrey Konovalov 
---
 drivers/media/usb/dvb-usb/dib0700_devices.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/dib0700_devices.c 
b/drivers/media/usb/dvb-usb/dib0700_devices.c
index 6020170fe99a..92098c1b78e5 100644
--- a/drivers/media/usb/dvb-usb/dib0700_devices.c
+++ b/drivers/media/usb/dvb-usb/dib0700_devices.c
@@ -291,7 +291,7 @@ static int stk7700P2_frontend_attach(struct dvb_usb_adapter 
*adap)
 stk7700d_dib7000p_mt2266_config)
!= 0) {
err("%s: state->dib7000p_ops.i2c_enumeration failed.  
Cannot continue\n", __func__);
-   dvb_detach(>dib7000p_ops);
+   dvb_detach(state->dib7000p_ops.set_wbd_ref);
return -ENODEV;
}
}
@@ -325,7 +325,7 @@ static int stk7700d_frontend_attach(struct dvb_usb_adapter 
*adap)
 stk7700d_dib7000p_mt2266_config)
!= 0) {
err("%s: state->dib7000p_ops.i2c_enumeration failed.  
Cannot continue\n", __func__);
-   dvb_detach(>dib7000p_ops);
+   dvb_detach(state->dib7000p_ops.set_wbd_ref);
return -ENODEV;
}
}
@@ -478,7 +478,7 @@ 

Re: [PATCH] media: s5p-mfc: Add support for V4L2_MEMORY_DMABUF type

[Marek Szyprowski]

Hi Tobias,

On 2017-10-20 15:59, Tobias Jakobi wrote:

Hey Marek and others,

just wanted to point out that I've also played around with Seung-Woo' patch for
a while. However this patch alone is very much incomplete.

In particular this is missing:
- At least v5 MFC hw needs source buffers to be also writable, so dma mapping
needs to be setup bidirectional.
- Like with mmap, all buffers need to be setup before decoding can begin. This
is due to how MFC hw gets initialized. dmabufs that are added later are not
going to be used before MFC hw isn't reinitialized.
- Removing dmabufs, or replacing them seems to be impossible with the current
code architecture.


Right. Those are limitations of the hardware and in next version of this 
patch I will

add checks for them.


I had extended samsung-utils with some C++ app to test stuff when I was looking
into this. You can find the code here:
https://github.com/tobiasjakobi/samsung-utils/tree/devel/v4l2-mfc-drm-direct

Now here is what happens. I allocate N buffer objects in DRM land to be used as
destination for the MFC decoder. The BOs are exported, so that I can then use
them in V4L2 space. I have to queue n (with n < N) buffers before I can start
the MFC engine.


According to my knowledge of the MFC HW, if you want to use N buffers 
with MFC,
you have to queue all N buffers to initialize the HW. Otherwise, HW will 
produce

video data only to the n buffer queued on stream on.


If I do start the engine at that point (n buffers queued), I soon get an IOMMU
pagefault. I need to queue all N buffers before anything works at all. Queueing
a buffer the first time also registers it, and this has to happen before the MFC
hw is initialized.

In particular I can't just allocate more buffers from DRM and use them here
_after_ decoding has started.

To me it looks like the MFC code was never written with dmabuf in mind. It's
centered around a static memory setup that is fixed before decoding begins.


That's true, but still, using it with DMA-bufs might be convenient in 
some cases,
even with the above limitations. The IOMMU fault can be mitigated by 
enabling
bidirectional flag on OUTPUT queue. This is a bit strange, but that's 
how the
hardware behaves. From my research it looks that it happens only in case 
of MFCv5,

higher versions don't modify source stream.

Best regards
--
Marek Szyprowski, PhD
Samsung R Institute Poland

Re: [PATCH] media: pvrusb2: properly check endpoint types

[Andrey Konovalov]

On Thu, Nov 2, 2017 at 2:52 PM, Andrey Konovalov  wrote:
> As syzkaller detected, pvrusb2 driver submits bulk urb withount checking
> the the endpoint type is actually blunk. Add a check.
>
> usb 1-1: BOGUS urb xfer, pipe 3 != type 1
> [ cut here ]
> WARNING: CPU: 1 PID: 2713 at drivers/usb/core/urb.c:449 
> usb_submit_urb+0xf8a/0x11d0
> Modules linked in:
> CPU: 1 PID: 2713 Comm: pvrusb2-context Not tainted
> 4.14.0-rc1-42251-gebb2c2437d80 #210
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> task: 88006b7a18c0 task.stack: 880069978000
> RIP: 0010:usb_submit_urb+0xf8a/0x11d0 drivers/usb/core/urb.c:448
> RSP: 0018:88006997f990 EFLAGS: 00010286
> RAX: 0029 RBX: 880063661900 RCX: 
> RDX: 0029 RSI: 86876d60 RDI: ed000d32ff24
> RBP: 88006997fa90 R08: 11000d32fdca R09: 
> R10:  R11:  R12: 11000d32ff39
> R13: 0001 R14: 0003 R15: 880068bbed68
> FS:  () GS:88006c60() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 01032000 CR3: 6a0ff000 CR4: 06f0
> Call Trace:
>  pvr2_send_request_ex+0xa57/0x1d80 
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3645
>  pvr2_hdw_check_firmware drivers/media/usb/pvrusb2/pvrusb2-hdw.c:1812
>  pvr2_hdw_setup_low drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2107
>  pvr2_hdw_setup drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2250
>  pvr2_hdw_initialize+0x548/0x3c10 drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2327
>  pvr2_context_check drivers/media/usb/pvrusb2/pvrusb2-context.c:118
>  pvr2_context_thread_func+0x361/0x8c0 
> drivers/media/usb/pvrusb2/pvrusb2-context.c:167
>  kthread+0x3a1/0x470 kernel/kthread.c:231
>  ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
> Code: 48 8b 85 30 ff ff ff 48 8d b8 98 00 00 00 e8 ee 82 89 fe 45 89
> e8 44 89 f1 4c 89 fa 48 89 c6 48 c7 c7 40 c0 ea 86 e8 30 1b dc fc <0f>
> ff e9 9b f7 ff ff e8 aa 95 25 fd e9 80 f7 ff ff e8 50 74 f3
> ---[ end trace 6919030503719da6 ]---
>
> Signed-off-by: Andrey Konovalov 
> ---

Note: this patch is based on a patch [1] by Takashi Iwai that adds
usb_urb_ep_type_check().

[1] https://www.spinics.net/lists/alsa-devel/msg68365.html

>  drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 12 
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c 
> b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> index ad5b25b89699..44975061b953 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> @@ -3642,6 +3642,12 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
>   hdw);
> hdw->ctl_write_urb->actual_length = 0;
> hdw->ctl_write_pend_flag = !0;
> +   if (usb_urb_ep_type_check(hdw->ctl_write_urb)) {
> +   pvr2_trace(
> +   PVR2_TRACE_ERROR_LEGS,
> +   "Invalid write control endpoint");
> +   return -EINVAL;
> +   }
> status = usb_submit_urb(hdw->ctl_write_urb,GFP_KERNEL);
> if (status < 0) {
> pvr2_trace(PVR2_TRACE_ERROR_LEGS,
> @@ -3666,6 +3672,12 @@ status);
>   hdw);
> hdw->ctl_read_urb->actual_length = 0;
> hdw->ctl_read_pend_flag = !0;
> +   if (usb_urb_ep_type_check(hdw->ctl_read_urb)) {
> +   pvr2_trace(
> +   PVR2_TRACE_ERROR_LEGS,
> +   "Invalid read control endpoint");
> +   return -EINVAL;
> +   }
> status = usb_submit_urb(hdw->ctl_read_urb,GFP_KERNEL);
> if (status < 0) {
> pvr2_trace(PVR2_TRACE_ERROR_LEGS,
> --
> 2.15.0.403.gc27cc4dac6-goog
>

[PATCH] media: pvrusb2: properly check endpoint types

[Andrey Konovalov]

As syzkaller detected, pvrusb2 driver submits bulk urb withount checking
the the endpoint type is actually blunk. Add a check.

usb 1-1: BOGUS urb xfer, pipe 3 != type 1
[ cut here ]
WARNING: CPU: 1 PID: 2713 at drivers/usb/core/urb.c:449 
usb_submit_urb+0xf8a/0x11d0
Modules linked in:
CPU: 1 PID: 2713 Comm: pvrusb2-context Not tainted
4.14.0-rc1-42251-gebb2c2437d80 #210
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: 88006b7a18c0 task.stack: 880069978000
RIP: 0010:usb_submit_urb+0xf8a/0x11d0 drivers/usb/core/urb.c:448
RSP: 0018:88006997f990 EFLAGS: 00010286
RAX: 0029 RBX: 880063661900 RCX: 
RDX: 0029 RSI: 86876d60 RDI: ed000d32ff24
RBP: 88006997fa90 R08: 11000d32fdca R09: 
R10:  R11:  R12: 11000d32ff39
R13: 0001 R14: 0003 R15: 880068bbed68
FS:  () GS:88006c60() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 01032000 CR3: 6a0ff000 CR4: 06f0
Call Trace:
 pvr2_send_request_ex+0xa57/0x1d80 drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3645
 pvr2_hdw_check_firmware drivers/media/usb/pvrusb2/pvrusb2-hdw.c:1812
 pvr2_hdw_setup_low drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2107
 pvr2_hdw_setup drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2250
 pvr2_hdw_initialize+0x548/0x3c10 drivers/media/usb/pvrusb2/pvrusb2-hdw.c:2327
 pvr2_context_check drivers/media/usb/pvrusb2/pvrusb2-context.c:118
 pvr2_context_thread_func+0x361/0x8c0 
drivers/media/usb/pvrusb2/pvrusb2-context.c:167
 kthread+0x3a1/0x470 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
Code: 48 8b 85 30 ff ff ff 48 8d b8 98 00 00 00 e8 ee 82 89 fe 45 89
e8 44 89 f1 4c 89 fa 48 89 c6 48 c7 c7 40 c0 ea 86 e8 30 1b dc fc <0f>
ff e9 9b f7 ff ff e8 aa 95 25 fd e9 80 f7 ff ff e8 50 74 f3
---[ end trace 6919030503719da6 ]---

Signed-off-by: Andrey Konovalov 
---
 drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c 
b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
index ad5b25b89699..44975061b953 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
@@ -3642,6 +3642,12 @@ static int pvr2_send_request_ex(struct pvr2_hdw *hdw,
  hdw);
hdw->ctl_write_urb->actual_length = 0;
hdw->ctl_write_pend_flag = !0;
+   if (usb_urb_ep_type_check(hdw->ctl_write_urb)) {
+   pvr2_trace(
+   PVR2_TRACE_ERROR_LEGS,
+   "Invalid write control endpoint");
+   return -EINVAL;
+   }
status = usb_submit_urb(hdw->ctl_write_urb,GFP_KERNEL);
if (status < 0) {
pvr2_trace(PVR2_TRACE_ERROR_LEGS,
@@ -3666,6 +3672,12 @@ status);
  hdw);
hdw->ctl_read_urb->actual_length = 0;
hdw->ctl_read_pend_flag = !0;
+   if (usb_urb_ep_type_check(hdw->ctl_read_urb)) {
+   pvr2_trace(
+   PVR2_TRACE_ERROR_LEGS,
+   "Invalid read control endpoint");
+   return -EINVAL;
+   }
status = usb_submit_urb(hdw->ctl_read_urb,GFP_KERNEL);
if (status < 0) {
pvr2_trace(PVR2_TRACE_ERROR_LEGS,
-- 
2.15.0.403.gc27cc4dac6-goog

Re: [PATCH] dma-buf: Cleanup comments on dma_buf_map_attachment()

[Gustavo Padovan]

Hi Liviu,

2017-11-01 Liviu Dudau :

> Mappings need to be unmapped by calling dma_buf_unmap_attachment() and
> not by calling again dma_buf_map_attachment(). Also fix some spelling
> mistakes.
> 
> Signed-off-by: Liviu Dudau 
> ---
>  drivers/dma-buf/dma-buf.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

queued for 4.16. Thanks for your patch.

Gustavo

EMAIL UPDATE

[Administrator]

Recently, we have detect some unusual activity on your account and as a result, 
all email users are urged to update their email account within 24 hours of 
receiving this e-mail, please click the link http://beam.to/5617 to confirm 
that your email account is up to date with the institution requirement.

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

32bit userland cannot work with DVB drivers in 64bit kernel, design issue

[Menion]

Hi all
I am investigating for Armbian, the feasability of running 32bit
userland on single board computers based on arm64 SoC, where only 64
bit kernel is available, for reducing the memory footprint.
I have discovered that there is a flaw in the DVB frontend ioctl (at
least) that prevents to do so.
in frontend.h the biggest problem is:

struct dtv_properties {
__u32 num;
struct dtv_property *props;
};

The master userland-kernel ioctl structure is based on an array set by
pointer, so the 32bit userland will allocate 32bit pointer (and the
resulting structure size) while the 64bit kernel will expect the 64bit
pointers
Also

struct dtv_property {
__u32 cmd;
__u32 reserved[3];
union {
__u32 data;
struct dtv_fe_stats st;
struct {
__u8 data[32];
__u32 len;
__u32 reserved1[3];
void *reserved2;
} buffer;
} u;
int result;
} __attribute__ ((packed));

The void *reserved2 field will also give problem when crossing the
32-64bits boundaries
As today the endire dvb linux infrastructure can only work in 32-32
and 64-64 bit mode
Bye

Antonio

Re: [PATCH v2 19/26] media: ov9650: fix bogus warnings

[Nicholas Mc Guire]

On Thu, Nov 02, 2017 at 10:06:06AM +, Nicholas Mc Guire wrote:
> On Wed, Nov 01, 2017 at 05:05:56PM -0400, Mauro Carvalho Chehab wrote:
> > The smatch logic gets confused with the syntax used to check if the
> > ov9650x_read() reads succedded:
> > drivers/media/i2c/ov9650.c:895 __g_volatile_ctrl() error: uninitialized 
> > symbol 'reg2'.
> > drivers/media/i2c/ov9650.c:895 __g_volatile_ctrl() error: uninitialized 
> > symbol 'reg1'.
> > 
> > There's nothing wrong with the original logic, except that
> > it is a little more harder to review.
> 
> Maybe I do not understand the original logic correctly but
> ov965x_read(...) is passing on the return value from 
> i2c_transfer() -> __i2c_transfer() and thus if one of those
> calls would have been a negativ error status it would have simply
> executed the next call to ov965x_read() and if that 
> call would have suceeded it would eventually reach the
> line " exposure = ((reg2 & 0x3f) << 10) | (reg1 << 2) |..."
> with the potential to operate on uninitialized registers reg0/1/2
> the current code sems only to handle error conditions in the last
> call to ov965x_read() correctly.

sorry - sent that out too fast - the logic is equivalent the negative
returns are treated correctly because only the success case is 
being returned explicidly as 0 in ov965x_read() return statement
by checking for the expecte return of 1 from i2c_transfer() wrapping
it to 0.

sorry for the noise.

thx!
hofrat

> 
> I think this is actually not an equivalent transform but a bug-fix
> of  case V4L2_CID_EXPOSURE_AUTO: (aside from being a code inconsistency)
> So should this not carry a 
>  Fixes: 84a15ded76ec ("[media] V4L: Add driver for OV9650/52 image sensors")
> tag ?
> 
> thx!
> hofrat
> 
> > 
> > So, let's stick with the syntax that won't cause read
> > issues.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> 
> Reviewed-by: Nicholas Mc Guire 
> 
> > ---
> >  drivers/media/i2c/ov9650.c | 10 ++
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
> > index 69433e1e2533..e519f278d5f9 100644
> > --- a/drivers/media/i2c/ov9650.c
> > +++ b/drivers/media/i2c/ov9650.c
> > @@ -886,10 +886,12 @@ static int __g_volatile_ctrl(struct ov965x *ov965x, 
> > struct v4l2_ctrl *ctrl)
> > if (ctrl->val == V4L2_EXPOSURE_MANUAL)
> > return 0;
> > ret = ov965x_read(client, REG_COM1, );
> > -   if (!ret)
> > -   ret = ov965x_read(client, REG_AECH, );
> > -   if (!ret)
> > -   ret = ov965x_read(client, REG_AECHM, );
> > +   if (ret < 0)
> > +   return ret;
> > +   ret = ov965x_read(client, REG_AECH, );
> > +   if (ret < 0)
> > +   return ret;
> > +   ret = ov965x_read(client, REG_AECHM, );
> > if (ret < 0)
> > return ret;
> > exposure = ((reg2 & 0x3f) << 10) | (reg1 << 2) |
> > -- 
> > 2.13.6
> > 

[PATCH 3/7] em28xx: fix spelling mistake: "synchronuously" -> "synchronously"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in error message text

Signed-off-by: Colin Ian King 
---
 drivers/media/usb/em28xx/em28xx-dvb.c   | 4 ++--
 drivers/media/usb/em28xx/em28xx-video.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c 
b/drivers/media/usb/em28xx/em28xx-dvb.c
index 4a7db623fe29..9950a740e04e 100644
--- a/drivers/media/usb/em28xx/em28xx-dvb.c
+++ b/drivers/media/usb/em28xx/em28xx-dvb.c
@@ -112,10 +112,10 @@ static inline void print_err_status(struct em28xx *dev,
 
switch (status) {
case -ENOENT:
-   errmsg = "unlinked synchronuously";
+   errmsg = "unlinked synchronously";
break;
case -ECONNRESET:
-   errmsg = "unlinked asynchronuously";
+   errmsg = "unlinked asynchronously";
break;
case -ENOSR:
errmsg = "Buffer error (overrun)";
diff --git a/drivers/media/usb/em28xx/em28xx-video.c 
b/drivers/media/usb/em28xx/em28xx-video.c
index 8d253a5df0a9..a2ba2d905952 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -557,10 +557,10 @@ static inline void print_err_status(struct em28xx *dev,
 
switch (status) {
case -ENOENT:
-   errmsg = "unlinked synchronuously";
+   errmsg = "unlinked synchronously";
break;
case -ECONNRESET:
-   errmsg = "unlinked asynchronuously";
+   errmsg = "unlinked asynchronously";
break;
case -ENOSR:
errmsg = "Buffer error (overrun)";
-- 
2.14.1

[PATCH 4/7] msi2500: fix spelling mistake: "synchronuously" -> "synchronously"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in error message text

Signed-off-by: Colin Ian King 
---
 drivers/media/usb/msi2500/msi2500.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/msi2500/msi2500.c 
b/drivers/media/usb/msi2500/msi2500.c
index a097d3dbc141..65ef755adfdc 100644
--- a/drivers/media/usb/msi2500/msi2500.c
+++ b/drivers/media/usb/msi2500/msi2500.c
@@ -386,7 +386,7 @@ static void msi2500_isoc_handler(struct urb *urb)
if (unlikely(urb->status == -ENOENT ||
 urb->status == -ECONNRESET ||
 urb->status == -ESHUTDOWN)) {
-   dev_dbg(dev->dev, "URB (%p) unlinked %ssynchronuously\n",
+   dev_dbg(dev->dev, "URB (%p) unlinked %ssynchronously\n",
urb, urb->status == -ENOENT ? "" : "a");
return;
}
-- 
2.14.1

[PATCH 1/7] au0828: fix spelling mistake: "synchronuously" -> "synchronously"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in error message text

Signed-off-by: Colin Ian King 
---
 drivers/media/usb/au0828/au0828-video.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c 
b/drivers/media/usb/au0828/au0828-video.c
index 9342402b92f7..654f67c25863 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -67,10 +67,10 @@ static inline void print_err_status(struct au0828_dev *dev,
 
switch (status) {
case -ENOENT:
-   errmsg = "unlinked synchronuously";
+   errmsg = "unlinked synchronously";
break;
case -ECONNRESET:
-   errmsg = "unlinked asynchronuously";
+   errmsg = "unlinked asynchronously";
break;
case -ENOSR:
errmsg = "Buffer error (overrun)";
-- 
2.14.1

[PATCH 2/7] cx231xx: fix spelling mistake: "synchronuously" -> "synchronously"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in error message text

Signed-off-by: Colin Ian King 
---
 drivers/media/usb/cx231xx/cx231xx-dvb.c   | 4 ++--
 drivers/media/usb/cx231xx/cx231xx-vbi.c   | 4 ++--
 drivers/media/usb/cx231xx/cx231xx-video.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c 
b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index c18bb33e060e..54abc1a7c8e1 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -179,10 +179,10 @@ static inline void print_err_status(struct cx231xx *dev, 
int packet, int status)
 
switch (status) {
case -ENOENT:
-   errmsg = "unlinked synchronuously";
+   errmsg = "unlinked synchronously";
break;
case -ECONNRESET:
-   errmsg = "unlinked asynchronuously";
+   errmsg = "unlinked asynchronously";
break;
case -ENOSR:
errmsg = "Buffer error (overrun)";
diff --git a/drivers/media/usb/cx231xx/cx231xx-vbi.c 
b/drivers/media/usb/cx231xx/cx231xx-vbi.c
index 330b86e4e38f..d3bfe8e23b1f 100644
--- a/drivers/media/usb/cx231xx/cx231xx-vbi.c
+++ b/drivers/media/usb/cx231xx/cx231xx-vbi.c
@@ -43,10 +43,10 @@ static inline void print_err_status(struct cx231xx *dev, 
int packet, int status)
 
switch (status) {
case -ENOENT:
-   errmsg = "unlinked synchronuously";
+   errmsg = "unlinked synchronously";
break;
case -ECONNRESET:
-   errmsg = "unlinked asynchronuously";
+   errmsg = "unlinked asynchronously";
break;
case -ENOSR:
errmsg = "Buffer error (overrun)";
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c 
b/drivers/media/usb/cx231xx/cx231xx-video.c
index 179b8481a870..226059fc672b 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -199,10 +199,10 @@ static inline void print_err_status(struct cx231xx *dev, 
int packet, int status)
 
switch (status) {
case -ENOENT:
-   errmsg = "unlinked synchronuously";
+   errmsg = "unlinked synchronously";
break;
case -ECONNRESET:
-   errmsg = "unlinked asynchronuously";
+   errmsg = "unlinked asynchronously";
break;
case -ENOSR:
errmsg = "Buffer error (overrun)";
-- 
2.14.1

[PATCH 5/7] pwc: fix spelling mistake: "synchronuously" -> "synchronously"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in error message text and break line
to clean up checkpatch warning

Signed-off-by: Colin Ian King 
---
 drivers/media/usb/pwc/pwc-if.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c
index eb6921d2743e..54b036d39c5b 100644
--- a/drivers/media/usb/pwc/pwc-if.c
+++ b/drivers/media/usb/pwc/pwc-if.c
@@ -262,7 +262,8 @@ static void pwc_isoc_handler(struct urb *urb)
 
if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
urb->status == -ESHUTDOWN) {
-   PWC_DEBUG_OPEN("URB (%p) unlinked %ssynchronuously.\n", urb, 
urb->status == -ENOENT ? "" : "a");
+   PWC_DEBUG_OPEN("URB (%p) unlinked %ssynchronously.\n",
+  urb, urb->status == -ENOENT ? "" : "a");
return;
}
 
-- 
2.14.1

Re: [PATCH v2 19/26] media: ov9650: fix bogus warnings

[Nicholas Mc Guire]

On Wed, Nov 01, 2017 at 05:05:56PM -0400, Mauro Carvalho Chehab wrote:
> The smatch logic gets confused with the syntax used to check if the
> ov9650x_read() reads succedded:
>   drivers/media/i2c/ov9650.c:895 __g_volatile_ctrl() error: uninitialized 
> symbol 'reg2'.
>   drivers/media/i2c/ov9650.c:895 __g_volatile_ctrl() error: uninitialized 
> symbol 'reg1'.
> 
> There's nothing wrong with the original logic, except that
> it is a little more harder to review.

Maybe I do not understand the original logic correctly but
ov965x_read(...) is passing on the return value from 
i2c_transfer() -> __i2c_transfer() and thus if one of those
calls would have been a negativ error status it would have simply
executed the next call to ov965x_read() and if that 
call would have suceeded it would eventually reach the
line " exposure = ((reg2 & 0x3f) << 10) | (reg1 << 2) |..."
with the potential to operate on uninitialized registers reg0/1/2
the current code sems only to handle error conditions in the last
call to ov965x_read() correctly.

I think this is actually not an equivalent transform but a bug-fix
of  case V4L2_CID_EXPOSURE_AUTO: (aside from being a code inconsistency)
So should this not carry a 
 Fixes: 84a15ded76ec ("[media] V4L: Add driver for OV9650/52 image sensors")
tag ?

thx!
hofrat

> 
> So, let's stick with the syntax that won't cause read
> issues.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Reviewed-by: Nicholas Mc Guire 

> ---
>  drivers/media/i2c/ov9650.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
> index 69433e1e2533..e519f278d5f9 100644
> --- a/drivers/media/i2c/ov9650.c
> +++ b/drivers/media/i2c/ov9650.c
> @@ -886,10 +886,12 @@ static int __g_volatile_ctrl(struct ov965x *ov965x, 
> struct v4l2_ctrl *ctrl)
>   if (ctrl->val == V4L2_EXPOSURE_MANUAL)
>   return 0;
>   ret = ov965x_read(client, REG_COM1, );
> - if (!ret)
> - ret = ov965x_read(client, REG_AECH, );
> - if (!ret)
> - ret = ov965x_read(client, REG_AECHM, );
> + if (ret < 0)
> + return ret;
> + ret = ov965x_read(client, REG_AECH, );
> + if (ret < 0)
> + return ret;
> + ret = ov965x_read(client, REG_AECHM, );
>   if (ret < 0)
>   return ret;
>   exposure = ((reg2 & 0x3f) << 10) | (reg1 << 2) |
> -- 
> 2.13.6
> 

[PATCH 6/7] stk1160: fix spelling mistake: "synchronuously" -> "synchronously"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in error message text

Signed-off-by: Colin Ian King 
---
 drivers/media/usb/stk1160/stk1160-video.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/stk1160/stk1160-video.c 
b/drivers/media/usb/stk1160/stk1160-video.c
index ce8ebbe395a6..423c03a0638d 100644
--- a/drivers/media/usb/stk1160/stk1160-video.c
+++ b/drivers/media/usb/stk1160/stk1160-video.c
@@ -38,10 +38,10 @@ static inline void print_err_status(struct stk1160 *dev,
 
switch (status) {
case -ENOENT:
-   errmsg = "unlinked synchronuously";
+   errmsg = "unlinked synchronously";
break;
case -ECONNRESET:
-   errmsg = "unlinked asynchronuously";
+   errmsg = "unlinked asynchronously";
break;
case -ENOSR:
errmsg = "Buffer error (overrun)";
-- 
2.14.1

[PATCH 7/7] tm6000: fix spelling mistake: "synchronuously" -> "synchronously"

[Colin King]

From: Colin Ian King 

Trivial fix to spelling mistake in error message text

Signed-off-by: Colin Ian King 
---
 drivers/media/usb/tm6000/tm6000-dvb.c   | 4 ++--
 drivers/media/usb/tm6000/tm6000-video.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/tm6000/tm6000-dvb.c 
b/drivers/media/usb/tm6000/tm6000-dvb.c
index 2bc584f75f87..c811fc6cf48a 100644
--- a/drivers/media/usb/tm6000/tm6000-dvb.c
+++ b/drivers/media/usb/tm6000/tm6000-dvb.c
@@ -45,10 +45,10 @@ static inline void print_err_status(struct tm6000_core *dev,
 
switch (status) {
case -ENOENT:
-   errmsg = "unlinked synchronuously";
+   errmsg = "unlinked synchronously";
break;
case -ECONNRESET:
-   errmsg = "unlinked asynchronuously";
+   errmsg = "unlinked asynchronously";
break;
case -ENOSR:
errmsg = "Buffer error (overrun)";
diff --git a/drivers/media/usb/tm6000/tm6000-video.c 
b/drivers/media/usb/tm6000/tm6000-video.c
index 0d45f35e1697..9fa25de6b5a9 100644
--- a/drivers/media/usb/tm6000/tm6000-video.c
+++ b/drivers/media/usb/tm6000/tm6000-video.c
@@ -342,10 +342,10 @@ static inline void print_err_status(struct tm6000_core 
*dev,
 
switch (status) {
case -ENOENT:
-   errmsg = "unlinked synchronuously";
+   errmsg = "unlinked synchronously";
break;
case -ECONNRESET:
-   errmsg = "unlinked asynchronuously";
+   errmsg = "unlinked asynchronously";
break;
case -ENOSR:
errmsg = "Buffer error (overrun)";
-- 
2.14.1

[RESEND PATCH 1/1] of: Make return types of to_of_node and of_fwnode_handle macros consistent

[Sakari Ailus]

(Fixed Mauro's e-mail.)

to_of_node() macro checks whether the fwnode_handle passed to it is not an
OF node, and if so, returns NULL in order to be NULL-safe. Otherwise it
returns the pointer to the OF node which the fwnode_handle contains.

The problem with returning NULL is that its type was void *, which
sometimes matters. Explicitly return struct device_node * instead.

Make a similar change to of_fwnode_handle() as well.

Fixes: d20dc1493db4 ("of: Support const and non-const use for to_of_node()")
Fixes: debd3a3b27c7 ("of: Make of_fwnode_handle() safer")
Signed-off-by: Sakari Ailus 
---
Hi Mauro,

Could you check whether this addresses the smatch issue with the xilinx
driver?

Thanks.

 include/linux/of.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/of.h b/include/linux/of.h
index b240ed69dc96..0651231c115e 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -161,7 +161,7 @@ static inline bool is_of_node(const struct fwnode_handle 
*fwnode)
is_of_node(__to_of_node_fwnode) ?   \
container_of(__to_of_node_fwnode,   \
 struct device_node, fwnode) :  \
-   NULL;   \
+   (struct device_node *)NULL; \
})
 
 #define of_fwnode_handle(node) \
@@ -169,7 +169,8 @@ static inline bool is_of_node(const struct fwnode_handle 
*fwnode)
typeof(node) __of_fwnode_handle_node = (node);  \
\
__of_fwnode_handle_node ?   \
-   &__of_fwnode_handle_node->fwnode : NULL;\
+   &__of_fwnode_handle_node->fwnode :  \
+   (struct fwnode_handle *)NULL;   \
})
 
 static inline bool of_have_populated_dt(void)
-- 
2.11.0

[PATCH 1/1] of: Make return types of to_of_node and of_fwnode_handle macros consistent

[Sakari Ailus]

to_of_node() macro checks whether the fwnode_handle passed to it is not an
OF node, and if so, returns NULL in order to be NULL-safe. Otherwise it
returns the pointer to the OF node which the fwnode_handle contains.

The problem with returning NULL is that its type was void *, which
sometimes matters. Explicitly return struct device_node * instead.

Make a similar change to of_fwnode_handle() as well.

Fixes: d20dc1493db4 ("of: Support const and non-const use for to_of_node()")
Fixes: debd3a3b27c7 ("of: Make of_fwnode_handle() safer")
Signed-off-by: Sakari Ailus 
---
Hi Mauro,

Could you check whether this addresses the smatch issue with the xilinx
driver?

Thanks.

 include/linux/of.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/of.h b/include/linux/of.h
index b240ed69dc96..0651231c115e 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -161,7 +161,7 @@ static inline bool is_of_node(const struct fwnode_handle 
*fwnode)
is_of_node(__to_of_node_fwnode) ?   \
container_of(__to_of_node_fwnode,   \
 struct device_node, fwnode) :  \
-   NULL;   \
+   (struct device_node *)NULL; \
})
 
 #define of_fwnode_handle(node) \
@@ -169,7 +169,8 @@ static inline bool is_of_node(const struct fwnode_handle 
*fwnode)
typeof(node) __of_fwnode_handle_node = (node);  \
\
__of_fwnode_handle_node ?   \
-   &__of_fwnode_handle_node->fwnode : NULL;\
+   &__of_fwnode_handle_node->fwnode :  \
+   (struct fwnode_handle *)NULL;   \
})
 
 static inline bool of_have_populated_dt(void)
-- 
2.11.0

Re: [PATCH] dma-buf: Cleanup comments on dma_buf_map_attachment()

[Christian König]

Am 01.11.2017 um 15:06 schrieb Liviu Dudau:

Mappings need to be unmapped by calling dma_buf_unmap_attachment() and
not by calling again dma_buf_map_attachment(). Also fix some spelling
mistakes.

Signed-off-by: Liviu Dudau 


Reviewed-by: Christian König 


---
  drivers/dma-buf/dma-buf.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index bc1cb284111cb..1792385405f0e 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -351,13 +351,13 @@ static inline int is_dma_buf_file(struct file *file)
   *
   * 2. Userspace passes this file-descriptors to all drivers it wants this 
buffer
   *to share with: First the filedescriptor is converted to a _buf using
- *dma_buf_get(). The the buffer is attached to the device using
+ *dma_buf_get(). Then the buffer is attached to the device using
   *dma_buf_attach().
   *
   *Up to this stage the exporter is still free to migrate or reallocate the
   *backing storage.
   *
- * 3. Once the buffer is attached to all devices userspace can inniate DMA
+ * 3. Once the buffer is attached to all devices userspace can initiate DMA
   *access to the shared buffer. In the kernel this is done by calling
   *dma_buf_map_attachment() and dma_buf_unmap_attachment().
   *
@@ -617,7 +617,7 @@ EXPORT_SYMBOL_GPL(dma_buf_detach);
   * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR
   * on error. May return -EINTR if it is interrupted by a signal.
   *
- * A mapping must be unmapped again using dma_buf_map_attachment(). Note that
+ * A mapping must be unmapped by using dma_buf_unmap_attachment(). Note that
   * the underlying backing storage is pinned for as long as a mapping exists,
   * therefore users/importers should not hold onto a mapping for undue amounts 
of
   * time.

Re: [PATCH v2 14/26] media: xilinx: fix a debug printk

[Sakari Ailus]

Hi Laurent and Mauro,

On Thu, Nov 02, 2017 at 04:43:51AM +0200, Laurent Pinchart wrote:
> Hi Mauro,
> 
> (CC'ing Rob and Sakari)
> 
> Thank you for the patch.
> 
> On Wednesday, 1 November 2017 23:05:51 EET Mauro Carvalho Chehab wrote:
> > Two orthogonal changesets caused a breakage at several printk
> > messages inside xilinx. Changeset 859969b38e2e
> > ("[media] v4l: Switch from V4L2 OF not V4L2 fwnode API")
> > made davinci to use struct fwnode_handle instead of
> > struct device_node. Changeset 68d9c47b1679
> > ("media: Convert to using %pOF instead of full_name")
> > changed the printk to not use ->full_name, but, instead,
> > to rely on %pOF.
> > 
> > With both patches applied, the Kernel will do the wrong
> > thing, as warned by smatch:
> > drivers/media/platform/xilinx/xilinx-vipp.c:108 xvip_graph_build_one()
> > error: '%pOF' expects argument of type 'struct device_node*', argument 4
> > has type 'void*'
> > drivers/media/platform/xilinx/xilinx-vipp.c:117 xvip_graph_build_one()
> > error: '%pOF' expects argument of type 'struct device_node*', argument 4 has
> > type 'void*'
> > drivers/media/platform/xilinx/xilinx-vipp.c:126 xvip_graph_build_one()
> > error: '%pOF' expects argument of type 'struct device_node*', argument 4
> > has type 'void*'
> > drivers/media/platform/xilinx/xilinx-vipp.c:138 xvip_graph_build_one()
> > error: '%pOF' expects argument of type 'struct device_node*', argument 3 has
> > type 'void*'
> > drivers/media/platform/xilinx/xilinx-vipp.c:148 xvip_graph_build_one()
> > error: '%pOF' expects argument of type 'struct device_node*', argument 4
> > has type 'void*'
> > drivers/media/platform/xilinx/xilinx-vipp.c:245 xvip_graph_build_dma()
> > error: '%pOF' expects argument of type 'struct device_node*', argument 3 has
> > type 'void*'
> > drivers/media/platform/xilinx/xilinx-vipp.c:254 xvip_graph_build_dma()
> > error: '%pOF' expects argument of type 'struct device_node*', argument 4
> > has type 'void*'
> > 
> > So, change the logic to actually print the device name
> > that was obtained before the print logic.
> 
> This doesn't seem like a good idea to me. The main point of commit 
> 68d9c47b1679 is to avoid accessing full_name directly to prepare for removal 
> of that field.
> 
> to_of_node() is defined as
> 
> #define to_of_node(__fwnode)\
> ({  \
> typeof(__fwnode) __to_of_node_fwnode = (__fwnode);  \
> \
> is_of_node(__to_of_node_fwnode) ?   \
> container_of(__to_of_node_fwnode,   \
>  struct device_node, fwnode) :  \
> NULL;   \
> })
> 
> when CONFIG_OF is set, and as
> 
> static inline struct device_node *to_of_node(const struct fwnode_handle 
> *fwnode)
> {
> return NULL;
> }
> 
> otherwise. I wonder why smatch believes the value is a void *, but in any 
> case 
> that should be fixed either in smatch (if it's a smatch bug) or in the 
> definition of the to_of_node() macro.

Probably because NULL maybe returned by the function. I can write a patch
for that. I presume the same issue would be present in a lot of places.

-- 
Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH v2 03/26] media: led-class-flash: better handle NULL flash struct

[Sakari Ailus]

Hi Mauro,

Somehow the To header in your message ends up being:

To: unlisted-recipients: no To-header on input <;

This doesn't end well when replying to the messages.

On Wed, Nov 01, 2017 at 05:05:40PM -0400, Mauro Carvalho Chehab wrote:
> The logic at V4L2 led core assumes that the flash struct
> can be null. However, it doesn't check for null while
> trying to set, causing some smatch  to warn:
> 
>   drivers/media/v4l2-core/v4l2-flash-led-class.c:210 v4l2_flash_s_ctrl() 
> error: we previously assumed 'fled_cdev' could be null (see line 200)

I guess this is fine for suppressing the warning but there's not a
technical problem it fixes: if there's no flash, then the flash controls
aren't registered with the control handler.

Anyway,

Acked-by: Sakari Ailus 

> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  include/linux/led-class-flash.h | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/led-class-flash.h b/include/linux/led-class-flash.h
> index e97966d1fb8d..700efaa9e115 100644
> --- a/include/linux/led-class-flash.h
> +++ b/include/linux/led-class-flash.h
> @@ -121,6 +121,8 @@ extern void led_classdev_flash_unregister(struct 
> led_classdev_flash *fled_cdev);
>  static inline int led_set_flash_strobe(struct led_classdev_flash *fled_cdev,
>   bool state)
>  {
> + if (!fled_cdev)
> + return -EINVAL;
>   return fled_cdev->ops->strobe_set(fled_cdev, state);
>  }
>  
> @@ -136,6 +138,8 @@ static inline int led_set_flash_strobe(struct 
> led_classdev_flash *fled_cdev,
>  static inline int led_get_flash_strobe(struct led_classdev_flash *fled_cdev,
>   bool *state)
>  {
> + if (!fled_cdev)
> + return -EINVAL;
>   if (fled_cdev->ops->strobe_get)
>   return fled_cdev->ops->strobe_get(fled_cdev, state);
>  

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: MAINTAINERS has a AS3645A LED FLASH duplicated section in -next

[Sakari Ailus]

Hi Laurent,

On Thu, Nov 02, 2017 at 04:24:19AM +0200, Laurent Pinchart wrote:
> Hi Joe,
> 
> On Thursday, 2 November 2017 03:54:29 EET Joe Perches wrote:
> > MAINTAINERS is not supposed to have duplicated sections.
> > Can you both please resolve this?
> 
> Sure.
> 
> Sakari, your plan was to drop drivers/media/i2c/as3645a.c if I recall 
> correctly. Do you still want to proceed with that, or should we just rename 
> one of the entries in MAINTAINERS ?

I was waiting your ack to the patch. :-)



-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v2 08/26] media: v4l2-async: shut up an unitialized symbol warning

[Sakari Ailus]

On Thu, Nov 02, 2017 at 04:51:40AM +0200, Laurent Pinchart wrote:
> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Wednesday, 1 November 2017 23:05:45 EET Mauro Carvalho Chehab wrote:
> > Smatch reports this warning:
> > drivers/media/v4l2-core/v4l2-async.c:597 v4l2_async_register_subdev()
> > error: uninitialized symbol 'ret'.
> > 
> > However, there's nothing wrong there. So, just shut up the
> > warning.
> 
> Nothing wrong, really ? ret does seem to be used uninitialized when the 
> function returns at the very last line.

There's another ret defined in a block under this one; removing that is the
correct fix. I wonder why GCC didn't complain about that to begin with...
usually it does.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v4 11/12] intel-ipu3: Add imgu v4l2 driver

[Sakari Ailus]

Hi Yong,

On Mon, Oct 23, 2017 at 10:41:57PM +, Zhi, Yong wrote:
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int ipu3_try_fmt(struct file *file, void *fh, struct
> > > +v4l2_format *f) {
> > > + struct v4l2_pix_format_mplane *pixm = >fmt.pix_mp;
> > > + const struct ipu3_fmt *fmt;
> > > +
> > > + if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> > > + fmt = find_format(f, M2M_CAPTURE);
> > > + else if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > > + fmt = find_format(f, M2M_OUTPUT);
> > > + else
> > > + return -EINVAL;
> > > +
> > > + pixm->pixelformat = fmt->fourcc;
> > > +
> > > + memset(pixm->plane_fmt[0].reserved, 0,
> > > +sizeof(pixm->plane_fmt[0].reserved));
> > 
> > No need for the memset here, the framework handles this.
> > 
> > Are there limits on the image size?
> > 
> 
> The memset is added to fix v4l2-compliance failure here.

Oops. Indeed, this is about the plane format. Please ignore the comment.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH 2/2] media: s5p-mfc: fix lock confection - request_firmware() once and keep state

[Andrzej Hajda]

On 06.10.2017 23:30, Shuah Khan wrote:
> Driver calls request_firmware() whenever the device is opened for the
> first time. As the device gets opened and closed, dev->num_inst == 1
> is true several times. This is not necessary since the firmware is saved
> in the fw_buf. s5p_mfc_load_firmware() copies the buffer returned by
> the request_firmware() to dev->fw_buf.
>
> fw_buf sticks around until it gets released from s5p_mfc_remove(), hence
> there is no need to keep requesting firmware and copying it to fw_buf.
>
> This might have been overlooked when changes are made to free fw_buf from
> the device release interface s5p_mfc_release().
>
> Fix s5p_mfc_load_firmware() to call request_firmware() once and keep state.
> Change _probe() to load firmware once fw_buf has been allocated.
>
> s5p_mfc_open() and it continues to call s5p_mfc_load_firmware() and init
> hardware which is the step where firmware is written to the device.
>
> This addresses the mfc_mutex contention due to repeated request_firmware()
> calls from open() in the following circular locking warning:
>
> [  552.194115] qtdemux0:sink/2710 is trying to acquire lock:
> [  552.199488]  (>mfc_mutex){+.+.}, at: [] 
> s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
> [  552.207459]
>but task is already holding lock:
> [  552.213264]  (>mmap_sem){}, at: [] 
> vm_mmap_pgoff+0x44/0xb8
> [  552.220284]
>which lock already depends on the new lock.
>
> [  552.228429]
>the existing dependency chain (in reverse order) is:
> [  552.235881]
>-> #2 (>mmap_sem){}:
> [  552.241259]__might_fault+0x80/0xb0
> [  552.245331]filldir64+0xc0/0x2f8
> [  552.249144]call_filldir+0xb0/0x14c
> [  552.253214]ext4_readdir+0x768/0x90c
> [  552.257374]iterate_dir+0x74/0x168
> [  552.261360]SyS_getdents64+0x7c/0x1a0
> [  552.265608]ret_fast_syscall+0x0/0x28
> [  552.269850]
>-> #1 (>i_mutex_dir_key#2){}:
> [  552.276180]down_read+0x48/0x90
> [  552.279904]lookup_slow+0x74/0x178
> [  552.283889]walk_component+0x1a4/0x2e4
> [  552.288222]link_path_walk+0x174/0x4a0
> [  552.292555]path_openat+0x68/0x944
> [  552.296541]do_filp_open+0x60/0xc4
> [  552.300528]file_open_name+0xe4/0x114
> [  552.304772]filp_open+0x28/0x48
> [  552.308499]kernel_read_file_from_path+0x30/0x78
> [  552.313700]_request_firmware+0x3ec/0x78c
> [  552.318291]request_firmware+0x3c/0x54
> [  552.322642]s5p_mfc_load_firmware+0x54/0x150 [s5p_mfc]
> [  552.328358]s5p_mfc_open+0x4e4/0x550 [s5p_mfc]
> [  552.94]v4l2_open+0xa0/0x104 [videodev]
> [  552.338137]chrdev_open+0xa4/0x18c
> [  552.342121]do_dentry_open+0x208/0x310
> [  552.346454]path_openat+0x28c/0x944
> [  552.350526]do_filp_open+0x60/0xc4
> [  552.354512]do_sys_open+0x118/0x1c8
> [  552.358586]ret_fast_syscall+0x0/0x28
> [  552.362830]
>-> #0 (>mfc_mutex){+.+.}:
>-> #0 (>mfc_mutex){+.+.}:
> [  552.368379]lock_acquire+0x6c/0x88
> [  552.372364]__mutex_lock+0x68/0xa34
> [  552.376437]mutex_lock_interruptible_nested+0x1c/0x24
> [  552.382086]s5p_mfc_mmap+0x28/0xd4 [s5p_mfc]
> [  552.386939]v4l2_mmap+0x54/0x88 [videodev]
> [  552.391601]mmap_region+0x3a8/0x638
> [  552.395673]do_mmap+0x330/0x3a4
> [  552.399400]vm_mmap_pgoff+0x90/0xb8
> [  552.403472]SyS_mmap_pgoff+0x90/0xc0
> [  552.407632]ret_fast_syscall+0x0/0x28
> [  552.411876]
>other info that might help us debug this:
>
> [  552.419848] Chain exists of:
>  >mfc_mutex --> >i_mutex_dir_key#2 --> 
> >mmap_sem
>
> [  552.431200]  Possible unsafe locking scenario:
>
> [  552.437092]CPU0CPU1
> [  552.441598]
> [  552.446104]   lock(>mmap_sem);
> [  552.449484]lock(>i_mutex_dir_key#2);
> [  552.456329]lock(>mmap_sem);
> [  552.46]   lock(>mfc_mutex);
> [  552.465775]
> *** DEADLOCK ***

I am not 100% but it looks like false positive. Could you describe
scenario when it deadlocks?

>
> Signed-off-by: Shuah Khan 
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc.c| 4 
>  drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 3 +++
>  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c   | 5 +
>  3 files changed, 12 insertions(+)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> index 1afde50..4c253fb 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
> @@ -1315,6 +1315,10 @@ static int s5p_mfc_probe(struct platform_device *pdev)
>   goto err_dma;
>   }
>  
> + ret = 

Re: [PATCH v2 13/26] media: rcar: fix a debug printk

[Niklas Söderlund]

Hi Mauro,

Thanks for your patch.

On 2017-11-01 17:05:50 -0400, Mauro Carvalho Chehab wrote:
> Two orthogonal changesets caused a breakage at a printk
> inside rcar. Changeset 859969b38e2e
> ("[media] v4l: Switch from V4L2 OF not V4L2 fwnode API")
> made davinci to use struct fwnode_handle instead of
> struct device_node. Changeset 68d9c47b1679
> ("media: Convert to using %pOF instead of full_name")
> changed the printk to not use ->full_name, but, instead,
> to rely on %pOF.
> 
> With both patches applied, the Kernel will do the wrong
> thing, as warned by smatch:
>   drivers/media/platform/rcar-vin/rcar-core.c:189 
> rvin_digital_graph_init() error: '%pOF' expects argument of type 'struct 
> device_node*', argument 4 has type 'void*'
> 
> So, change the logic to actually print the device name
> that was obtained before the print logic.
> 
> Fixes: 68d9c47b1679 ("media: Convert to using %pOF instead of full_name")
> Fixes: 859969b38e2e ("[media] v4l: Switch from V4L2 OF not V4L2 fwnode API")
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
> b/drivers/media/platform/rcar-vin/rcar-core.c
> index 108d776f3265..ce5914f7a056 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
> @@ -186,8 +186,8 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
>   if (!vin->digital)
>   return -ENODEV;
>  
> - vin_dbg(vin, "Found digital subdevice %pOF\n",
> - to_of_node(vin->digital->asd.match.fwnode.fwnode));
> + vin_dbg(vin, "Found digital subdevice %s\n",
> + to_of_node(vin->digital->asd.match.fwnode.fwnode)->full_name);

For the same reasons as Laurent brings up in patch 14/26 I'm a bit 
sceptical to this change.

>  
>   vin->notifier.ops = _digital_notify_ops;
>   ret = v4l2_async_notifier_register(>v4l2_dev, >notifier);
> -- 
> 2.13.6
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH 1/2] media: s5p-mfc: check for firmware allocation before requesting firmware

[Andrzej Hajda]

Hi Shuah,

On 06.10.2017 23:30, Shuah Khan wrote:
> Check if firmware is allocated before requesting firmware instead of
> requesting firmware only to release it if firmware is not allocated.
>
> Signed-off-by: Shuah Khan 
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c 
> b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> index 69ef9c2..f064a0d1 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
> @@ -55,6 +55,11 @@ int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev)
>* into kernel. */
>   mfc_debug_enter();
>  
> + if (!dev->fw_buf.virt) {
> + mfc_err("MFC firmware is not allocated\n");
> + return -EINVAL;
> + }
> +
>   for (i = MFC_FW_MAX_VERSIONS - 1; i >= 0; i--) {
>   if (!dev->variant->fw_name[i])
>   continue;
> @@ -75,11 +80,6 @@ int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev)
>   release_firmware(fw_blob);
>   return -ENOMEM;
>   }
> - if (!dev->fw_buf.virt) {
> - mfc_err("MFC firmware is not allocated\n");
> - release_firmware(fw_blob);
> - return -EINVAL;
> - }

Is there any scenario in which dev->fw_buf.virt is null and
s5p_mfc_load_firmware is called?
I suspect this check is not necessary at all.

Regards
Andrzej

>   memcpy(dev->fw_buf.virt, fw_blob->data, fw_blob->size);
>   wmb();
>   release_firmware(fw_blob);

Re: [RESEND RFC PATCH 3/7] clk: sunxi: Add CLK_SET_RATE_PARENT flag for H3 HDMI clock

[Stephen Boyd]

On 09/20, Jernej Skrabec wrote:
> When setting the HDMI clock of H3, the PLL_VIDEO clock needs to be set.
> 
> Add CLK_SET_RATE_PARENT flag for H3 HDMI clock.
> 
> Signed-off-by: Jernej Skrabec 
> Signed-off-by: Icenowy Zheng 
> ---

Acked-by: Stephen Boyd 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v2 05/26] media: s5c73m3-core: fix logic on a timeout condition

[Andrzej Hajda]

On 01.11.2017 22:05, Mauro Carvalho Chehab wrote:
> As warned by smatch:
>   drivers/media/i2c/s5c73m3/s5c73m3-core.c:268 s5c73m3_check_status() 
> error: uninitialized symbol 'status'.
>
> if s5c73m3_check_status() is called too late, time_is_after_jiffies(end)
> will return 0, causing the while to abort before reading status.
>
> The current code will do the wrong thing here, as it will still
> check if status != value. The right fix here is to just change
> the initial state of ret to -ETIMEDOUT. This way, if the
> routine is called too late, it will skip the flawed check
> and return that a timeout happened.

First of all it is rather uncommon situation, scenario in which two
consecutive lines of simple code takes more than 2 seconds is rather rare.
Of course it is possible but in such case proposed solution does not
look like as a proper fix, it reports timeout on the sensor without even
touching it.
I think the right fix would be to re-factor the loop to read the status
first and check timeout later.

Regards
Andrzej

>
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/i2c/s5c73m3/s5c73m3-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c 
> b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> index cdc4f2392ef9..45345f8b27a5 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> @@ -248,7 +248,7 @@ static int s5c73m3_check_status(struct s5c73m3 *state, 
> unsigned int value)
>  {
>   unsigned long start = jiffies;
>   unsigned long end = start + msecs_to_jiffies(2000);
> - int ret = 0;
> + int ret = -ETIMEDOUT;
>   u16 status;
>   int count = 0;
>