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:   Thu Nov  2 05:00:16 CET 2017
media-tree git hash:9917fbcfa20ab987d6381fd0365665e5c1402d75
media_build git hash:   c93534951f5d66bef7f17f16293acf2be346b726
v4l-utils git hash: 814949be25dffaffa5d6354ce3aaf61f9d3dbbaa
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/Thursday.log

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH] [media] dw9714: Set the v4l2 focus ctrl step as 1

[Tomasz Figa]

On Thu, Nov 2, 2017 at 1:00 PM, Tomasz Figa  wrote:
> Hi Sakari,
>
> On Thu, Aug 31, 2017 at 6:28 AM, Sakari Ailus  wrote:
>> Hi Rajmohan,
>>
>> On Wed, Aug 30, 2017 at 10:48:52AM -0700, Rajmohan Mani wrote:
>>> Current v4l2 focus ctrl step value of 16, limits
>>> the minimum granularity of focus positions to 16.
>>> Setting this value as 1, enables more accurate
>>> focus positions.
>>
>> Thanks for the patch.
>>
>> The recommended limit for line length is 75, not 50 (or 25 or whatever) as
>> it might be in certain Gerrit installations. :-) Please make good use of
>> lines in the future, I've rewrapped the text this time. Thanks.
>
> Has this patch been applied to your tree? I can't find it on
> linux-next or https://git.linuxtv.org/sailus/media_tree.git/ . Just
> want to make sure it doesn't get lost in action.

Okay, my bad. I didn't notice that linux-next is stuck on Oct 18 and
so doesn't include latest media tree.

I confirmed that the patch is indeed present in Mauro's master branch.
Sorry for the noise.

Best regards,
Tomasz

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

[Alex Deucher]

On Wed, Nov 1, 2017 at 10:06 AM, Liviu Dudau  wrote:
> 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: Alex Deucher 

> ---
>  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.
> --
> 2.14.3
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] [media] dw9714: Set the v4l2 focus ctrl step as 1

[Tomasz Figa]

Hi Sakari,

On Thu, Aug 31, 2017 at 6:28 AM, Sakari Ailus  wrote:
> Hi Rajmohan,
>
> On Wed, Aug 30, 2017 at 10:48:52AM -0700, Rajmohan Mani wrote:
>> Current v4l2 focus ctrl step value of 16, limits
>> the minimum granularity of focus positions to 16.
>> Setting this value as 1, enables more accurate
>> focus positions.
>
> Thanks for the patch.
>
> The recommended limit for line length is 75, not 50 (or 25 or whatever) as
> it might be in certain Gerrit installations. :-) Please make good use of
> lines in the future, I've rewrapped the text this time. Thanks.

Has this patch been applied to your tree? I can't find it on
linux-next or https://git.linuxtv.org/sailus/media_tree.git/ . Just
want to make sure it doesn't get lost in action.

Best regards,
Tomasz

>
>>
>> Signed-off-by: Rajmohan Mani 
>> ---
>>  drivers/media/i2c/dw9714.c | 7 ++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/i2c/dw9714.c b/drivers/media/i2c/dw9714.c
>> index 6a607d7..f9212a8 100644
>> --- a/drivers/media/i2c/dw9714.c
>> +++ b/drivers/media/i2c/dw9714.c
>> @@ -22,6 +22,11 @@
>>  #define DW9714_NAME  "dw9714"
>>  #define DW9714_MAX_FOCUS_POS 1023
>>  /*
>> + * This sets the minimum granularity for the focus positions.
>> + * A value of 1 gives maximum accuracy for a desired focus position
>> + */
>> +#define DW9714_FOCUS_STEPS   1
>> +/*
>>   * This acts as the minimum granularity of lens movement.
>>   * Keep this value power of 2, so the control steps can be
>>   * uniformly adjusted for gradual lens movement, with desired
>> @@ -138,7 +143,7 @@ static int dw9714_init_controls(struct dw9714_device 
>> *dev_vcm)
>>   v4l2_ctrl_handler_init(hdl, 1);
>>
>>   v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_ABSOLUTE,
>> -   0, DW9714_MAX_FOCUS_POS, DW9714_CTRL_STEPS, 0);
>> +   0, DW9714_MAX_FOCUS_POS, DW9714_FOCUS_STEPS, 0);
>>
>>   if (hdl->error)
>>   dev_err(>dev, "%s fail error: 0x%x\n",
>
> --
> Regards,
>
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi

Re: [PATCH] media: atmel-isc: get rid of an unused var

[Yang, Wenyou]

On 2017/10/31 17:37, Mauro Carvalho Chehab wrote:

drivers/media/platform/atmel/atmel-isc.c: In function 'isc_async_complete':
drivers/media/platform/atmel/atmel-isc.c:1900:28: warning: variable 'sd_entity' 
set but not used [-Wunused-but-set-variable]
   struct isc_subdev_entity *sd_entity;
 ^

Signed-off-by: Mauro Carvalho Chehab 


Acked-by: Wenyou Yang 


---
  drivers/media/platform/atmel/atmel-isc.c | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 2c40a7886542..8b37656f035d 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1897,7 +1897,6 @@ static int isc_async_complete(struct v4l2_async_notifier 
*notifier)
  {
struct isc_device *isc = container_of(notifier->v4l2_dev,
  struct isc_device, v4l2_dev);
-   struct isc_subdev_entity *sd_entity;
struct video_device *vdev = >video_dev;
struct vb2_queue *q = >vb2_vidq;
int ret;
@@ -1910,8 +1909,6 @@ static int isc_async_complete(struct v4l2_async_notifier 
*notifier)
  
  	isc->current_subdev = container_of(notifier,

   struct isc_subdev_entity, notifier);
-   sd_entity = isc->current_subdev;
-
mutex_init(>lock);
init_completion(>comp);
  


Best Regards,
Wenyou Yang

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

[Laurent Pinchart]

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.

> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/v4l2-core/v4l2-async.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-async.c
> b/drivers/media/v4l2-core/v4l2-async.c index 49f7eccc76db..5bc861c4ae5c
> 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -532,7 +532,7 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
>  {
>   struct v4l2_async_notifier *subdev_notifier;
>   struct v4l2_async_notifier *notifier;
> - int ret;
> + int uninitialized_var(ret);
> 
>   /*
>* No reference taken. The reference is held by the device


-- 
Regards,

Laurent Pinchart

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

[Laurent Pinchart]

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.

> Fixes: 68d9c47b1679 ("media: Convert to using %pOF instead of full_name")
> Fixes: 859969b38e2e ("[media] v4l: Switch from V4L2 OF not V4L2 fwnode API")

When submitting fixes you should CC the author of the original commits. They 
should have more information about the context, and in this case I believe Rob 
would have pointed out that adding back references to full_name would break 
the refactoring he's working on.

> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/platform/xilinx/xilinx-vipp.c | 31 +-
>  1 file changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c
> b/drivers/media/platform/xilinx/xilinx-vipp.c index
> d881cf09876d..dd777c834c43 100644
> --- a/drivers/media/platform/xilinx/xilinx-vipp.c
> +++ b/drivers/media/platform/xilinx/xilinx-vipp.c
> @@ -65,6 +65,9 @@ xvip_graph_find_entity(struct xvip_composite_device *xdev,
> return NULL;
>  }
> 
> +#define LOCAL_NAME(link) to_of_node(link.local_node)->full_name
> +#define REMOTE_NAME(link)to_of_node(link.remote_node)->full_name
> +
>  static int xvip_graph_build_one(struct xvip_composite_device *xdev,
>   struct xvip_graph_entity *entity)
>  {
> @@ -103,9 +106,9 @@ static int xvip_graph_build_one(struct
> xvip_composite_device *xdev,
>* the link.
>*/
>   if (link.local_port >= local->num_pads) {
> - dev_err(xdev->dev, "invalid port number %u for %pOF\n",
> + dev_err(xdev->dev, "invalid port number %u for %s\n",
>   link.local_port,
> - to_of_node(link.local_node));
> + LOCAL_NAME(link));
>   v4l2_fwnode_put_link();
>   ret = -EINVAL;
>   break;
> @@ -114,8 +117,8 @@ static int 

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

[Laurent Pinchart]

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 ?

> AS3645A LED FLASH CONTROLLER DRIVER
> M:Sakari Ailus 
> 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

-- 
Regards,

Laurent Pinchart

MAINTAINERS has a AS3645A LED FLASH duplicated section in -next

[Joe Perches]

MAINTAINERS is not supposed to have duplicated sections.
Can you both please resolve this?

AS3645A LED FLASH CONTROLLER DRIVER
M:  Sakari Ailus 
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

[no subject]

[Roy Cockrum Foundation]

Hallo, Sie machen eine Spende von 4.800.000,00 EUR, ich habe die America Lotto 
in Amerika im Wert von 259,9 Millionen Dollar gewonnen, und ich gebe einen Teil 
davon fünf glückliche Menschen und Wohltätigkeits-Häuser in Erinnerung an meine 
verstorbene Frau, die an Krebs gestorben ist. Kontaktieren Sie mich für weitere 
Informationen: roycockrum2...@gmail.com

Re: [PATCH v4 2/3] media: ov7740: Document device tree bindings

[Rob Herring]

On Tue, Oct 31, 2017 at 09:11:44AM +0800, Wenyou Yang wrote:
> Add the device tree binding documentation for the ov7740 sensor driver.
> 
> Signed-off-by: Wenyou Yang 
> ---
> 
> Changes in v4: None
> Changes in v3:
>  - Explicitly document the "remote-endpoint" property.
> 
> Changes in v2: None
> 
>  .../devicetree/bindings/media/i2c/ov7740.txt   | 47 
> ++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov7740.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov7740.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov7740.txt
> new file mode 100644
> index ..af781c3a5f0e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov7740.txt
> @@ -0,0 +1,47 @@
> +* Omnivision OV7740 CMOS image sensor
> +
> +The Omnivision OV7740 image sensor supports multiple output image
> +size, such as VGA, and QVGA, CIF and any size smaller. It also
> +supports the RAW RGB and YUV output formats.
> +
> +The common video interfaces bindings (see video-interfaces.txt) should
> +be used to specify link to the image data receiver. The OV7740 device
> +node should contain one 'port' child node with an 'endpoint' subnode.
> +
> +Required Properties:
> +- compatible:"ovti,ov7740".
> +- reg:   I2C slave address of the sensor.
> +- clocks:Reference to the xvclk input clock.
> +- clock-names:   "xvclk".
> +
> +Optional Properties:
> +- reset-gpios: Rreference to the GPIO connected to the reset_b pin,
> +  if any. Active low with pull-ip resistor.
> +- powerdown-gpios: Reference to the GPIO connected to the pwdn pin,
> +  if any. Active high with pull-down resistor.
> +
> +Endpoint node mandatory properties:
> +- remote-endpoint: A phandle to the bus receiver's endpoint node.

This is not really necessary. What's required is documenting how many 
ports and how many endpoints for each port which you have above.

Acked-by: Rob Herring 

RE: [PATCH v4 03/12] intel-ipu3: Add IOMMU based dmamap support

[Zhi, Yong]

Hi, Sakari,

> -Original Message-
> From: Sakari Ailus [mailto:sakari.ai...@iki.fi]
> Sent: Friday, October 20, 2017 2:20 AM
> To: Zhi, Yong 
> Cc: linux-media@vger.kernel.org; sakari.ai...@linux.intel.com; Zheng, Jian
> Xu ; Mani, Rajmohan
> ; Toivonen, Tuukka
> ; Hu, Jerry W ;
> a...@arndb.de; h...@lst.de; robin.mur...@arm.com; iommu@lists.linux-
> foundation.org; Tomasz Figa 
> Subject: Re: [PATCH v4 03/12] intel-ipu3: Add IOMMU based dmamap
> support
> 
> Hi Yong,
> 
> On Tue, Oct 17, 2017 at 10:48:59PM -0500, Yong Zhi wrote:
> > From: Tomasz Figa 
> >
> > This patch adds driver to support IPU3-specific MMU-aware memory
> > alloc/free and sg mapping functions.
> >
> > Signed-off-by: Tomasz Figa 
> > Signed-off-by: Yong Zhi 
> > ---
> >  drivers/media/pci/intel/ipu3/Kconfig   |   7 +
> >  drivers/media/pci/intel/ipu3/Makefile  |   2 +-
> >  drivers/media/pci/intel/ipu3/ipu3-dmamap.c | 342
> > +
> > drivers/media/pci/intel/ipu3/ipu3-dmamap.h |  33 +++
> >  4 files changed, 383 insertions(+), 1 deletion(-)  create mode 100644
> > drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> >  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-dmamap.h
> >
> > diff --git a/drivers/media/pci/intel/ipu3/Kconfig
> > b/drivers/media/pci/intel/ipu3/Kconfig
> > index 46ff138f3e50..d7dab52dc881 100644
> > --- a/drivers/media/pci/intel/ipu3/Kconfig
> > +++ b/drivers/media/pci/intel/ipu3/Kconfig
> > @@ -26,3 +26,10 @@ config INTEL_IPU3_MMU
> > ---help---
> >   For IPU3, this option enables its MMU driver to translate its internal
> >   virtual address to 39 bits wide physical address for 64GBytes space
> access.
> > +
> > +config INTEL_IPU3_DMAMAP
> > +   tristate
> > +   default n
> > +   select IOMMU_IOVA
> > +   ---help---
> > + This is IPU3 IOMMU domain specific DMA driver.
> > diff --git a/drivers/media/pci/intel/ipu3/Makefile
> > b/drivers/media/pci/intel/ipu3/Makefile
> > index 91cac9cb7401..651773231496 100644
> > --- a/drivers/media/pci/intel/ipu3/Makefile
> > +++ b/drivers/media/pci/intel/ipu3/Makefile
> > @@ -13,4 +13,4 @@
> >
> >  obj-$(CONFIG_VIDEO_IPU3_CIO2) += ipu3-cio2.o
> >  obj-$(CONFIG_INTEL_IPU3_MMU) += ipu3-mmu.o
> > -
> > +obj-$(CONFIG_INTEL_IPU3_DMAMAP) += ipu3-dmamap.o
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> > b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> > new file mode 100644
> > index ..e54bd9dfa302
> > --- /dev/null
> > +++ b/drivers/media/pci/intel/ipu3/ipu3-dmamap.c
> > @@ -0,0 +1,342 @@
> > +/*
> > + * Copyright (c) 2017 Intel Corporation.
> > + * Copyright (C) 2017 Google, Inc.
> > + *
> > + * 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.
> > + *
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> 
> Do you need this for something?
> 

Ouch, will remove the un-needed headers.

> > +#include 
> > +
> > +#include "ipu3-css-pool.h"
> > +#include "ipu3.h"
> > +
> > +/*
> > + * Based on arch/arm64/mm/dma-mapping.c, with simplifications
> > +possible due
> > + * to driver-specific character of this file.
> > + */
> > +
> > +static int dma_direction_to_prot(enum dma_data_direction dir, bool
> > +coherent) {
> > +   int prot = coherent ? IOMMU_CACHE : 0;
> > +
> > +   switch (dir) {
> > +   case DMA_BIDIRECTIONAL:
> > +   return prot | IOMMU_READ | IOMMU_WRITE;
> > +   case DMA_TO_DEVICE:
> > +   return prot | IOMMU_READ;
> > +   case DMA_FROM_DEVICE:
> > +   return prot | IOMMU_WRITE;
> > +   default:
> > +   return 0;
> > +   }
> > +}
> > +
> > +/*
> > + * Free a buffer allocated by ipu3_dmamap_alloc_buffer()  */ static
> > +void ipu3_dmamap_free_buffer(struct page **pages,
> > +   size_t size)
> > +{
> > +   int count = size >> PAGE_SHIFT;
> > +
> > +   while (count--)
> > +   __free_page(pages[count]);
> > +   kvfree(pages);
> > +}
> > +
> > +/*
> > + * Based on the implementation of __iommu_dma_alloc_pages()
> > + * defined in drivers/iommu/dma-iommu.c  */ static struct page
> > +**ipu3_dmamap_alloc_buffer(size_t size,
> > + unsigned long order_mask,
> > + gfp_t gfp)
> > +{
> > +   struct page **pages;
> > +   unsigned int i = 0, count = size >> PAGE_SHIFT;
> > +  

[PATCH v2 02/26] media: dvb_frontend: be sure to init dvb_frontend_handle_ioctl() return code

[Mauro Carvalho Chehab]

As smatch warned:
drivers/media/dvb-core/dvb_frontend.c:2468 dvb_frontend_handle_ioctl() 
error: uninitialized symbol 'err'.

The ioctl handler actually got a regression here: before changeset
d73dcf0cdb95 ("media: dvb_frontend: cleanup ioctl handling logic"),
the code used to return -EOPNOTSUPP if an ioctl handler was not
implemented on a driver. After the change, it may return a random
value.

Fixes: d73dcf0cdb95 ("media: dvb_frontend: cleanup ioctl handling logic")
# For 4.14
Cc: sta...@vger.kernel.org
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-core/dvb_frontend.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index daaf969719e4..bcf0cbcbf7b2 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2109,7 +2109,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
struct dvb_frontend *fe = dvbdev->priv;
struct dvb_frontend_private *fepriv = fe->frontend_priv;
struct dtv_frontend_properties *c = >dtv_property_cache;
-   int i, err;
+   int i, err = -EOPNOTSUPP;
 
dev_dbg(fe->dvb->device, "%s:\n", __func__);
 
@@ -2144,6 +2144,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
}
}
kfree(tvp);
+   err = 0;
break;
}
case FE_GET_PROPERTY: {
@@ -2195,6 +2196,7 @@ static int dvb_frontend_handle_ioctl(struct file *file,
return -EFAULT;
}
kfree(tvp);
+   err = 0;
break;
}
 
-- 
2.13.6

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

[Mauro Carvalho Chehab]

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.

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

[PATCH v2 22/26] [RFC] media: cxd2841er: ensure that status will always be available

[Mauro Carvalho Chehab]

The loop with read status use a dynamic timeout value, calculated
from symbol rate. It should run the loop at least one time for
the status to be handled after the loop.

While this should, in practice, happen every time, it doesn't
hurt to change the logic to make it explicit.

This solves a smatch warning:
drivers/media/dvb-frontends/cxd2841er.c:3350 cxd2841er_set_frontend_s() 
error: uninitialized symbol 'status'.

Signed-off-by: Mauro Carvalho Chehab 

---

On a separate note, it looks weird to have something there waiting
for lock. This should happen on userspace, except if there are
some bugs at the hardware that prevent it to work otherwise.
---
 drivers/media/dvb-frontends/cxd2841er.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-frontends/cxd2841er.c 
b/drivers/media/dvb-frontends/cxd2841er.c
index 48ee9bc00c06..98e40b7adad5 100644
--- a/drivers/media/dvb-frontends/cxd2841er.c
+++ b/drivers/media/dvb-frontends/cxd2841er.c
@@ -3340,13 +3340,17 @@ static int cxd2841er_set_frontend_s(struct dvb_frontend 
*fe)
 
cxd2841er_tune_done(priv);
timeout = ((300 + (symbol_rate - 1)) / symbol_rate) + 150;
-   for (i = 0; i < timeout / CXD2841ER_DVBS_POLLING_INVL; i++) {
+
+   i = 0;
+   do {
usleep_range(CXD2841ER_DVBS_POLLING_INVL*1000,
(CXD2841ER_DVBS_POLLING_INVL + 2) * 1000);
cxd2841er_read_status_s(fe, );
if (status & FE_HAS_LOCK)
break;
-   }
+   i++;
+   } while (i < timeout / CXD2841ER_DVBS_POLLING_INVL);
+
if (status & FE_HAS_LOCK) {
if (cxd2841er_get_carrier_offset_s_s2(
priv, _offset)) {
-- 
2.13.6

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

[Mauro Carvalho Chehab]

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)

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);
 
-- 
2.13.6

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

[Mauro Carvalho Chehab]

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);
 
vin->notifier.ops = _digital_notify_ops;
ret = v4l2_async_notifier_register(>v4l2_dev, >notifier);
-- 
2.13.6

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

[Mauro Carvalho Chehab]

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.

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/xilinx/xilinx-vipp.c | 31 -
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c 
b/drivers/media/platform/xilinx/xilinx-vipp.c
index d881cf09876d..dd777c834c43 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -65,6 +65,9 @@ xvip_graph_find_entity(struct xvip_composite_device *xdev,
return NULL;
 }
 
+#define LOCAL_NAME(link)   to_of_node(link.local_node)->full_name
+#define REMOTE_NAME(link)  to_of_node(link.remote_node)->full_name
+
 static int xvip_graph_build_one(struct xvip_composite_device *xdev,
struct xvip_graph_entity *entity)
 {
@@ -103,9 +106,9 @@ static int xvip_graph_build_one(struct 
xvip_composite_device *xdev,
 * the link.
 */
if (link.local_port >= local->num_pads) {
-   dev_err(xdev->dev, "invalid port number %u for %pOF\n",
+   dev_err(xdev->dev, "invalid port number %u for %s\n",
link.local_port,
-   to_of_node(link.local_node));
+   LOCAL_NAME(link));
v4l2_fwnode_put_link();
ret = -EINVAL;
break;
@@ -114,8 +117,8 @@ static int xvip_graph_build_one(struct 
xvip_composite_device *xdev,
local_pad = >pads[link.local_port];
 
if (local_pad->flags & MEDIA_PAD_FL_SINK) {
-   dev_dbg(xdev->dev, "skipping sink port %pOF:%u\n",
-   to_of_node(link.local_node),
+   dev_dbg(xdev->dev, "skipping sink port %s:%u\n",
+   LOCAL_NAME(link),
link.local_port);
v4l2_fwnode_put_link();
continue;
@@ -123,8 +126,8 @@ static int xvip_graph_build_one(struct 
xvip_composite_device *xdev,
 
/* Skip DMA engines, they will be processed separately. */
if (link.remote_node == of_fwnode_handle(xdev->dev->of_node)) {
-   dev_dbg(xdev->dev, "skipping DMA port %pOF:%u\n",
-   to_of_node(link.local_node),
+   dev_dbg(xdev->dev, "skipping DMA port %s:%u\n",
+   REMOTE_NAME(link),
link.local_port);
v4l2_fwnode_put_link();
continue;
@@ -134,8 +137,8 @@ static int xvip_graph_build_one(struct 
xvip_composite_device *xdev,
ent = xvip_graph_find_entity(xdev,
 to_of_node(link.remote_node));
if (ent == NULL) {
-   dev_err(xdev->dev, "no entity found for %pOF\n",
-   

[PATCH v2 16/26] media: drxd_hard: better handle I2C errors

[Mauro Carvalho Chehab]

As warned by smatch:
drivers/media/dvb-frontends/drxd_hard.c:989 HI_Command() error: 
uninitialized symbol 'waitCmd'.
drivers/media/dvb-frontends/drxd_hard.c:1306 SC_WaitForReady() error: 
uninitialized symbol 'curCmd'.
drivers/media/dvb-frontends/drxd_hard.c:1322 SC_SendCommand() error: 
uninitialized symbol 'errCode'.
drivers/media/dvb-frontends/drxd_hard.c:1339 SC_ProcStartCommand() 
error: uninitialized symbol 'scExec'.

The error handling on several places are somewhat flawed, as
they don't check if Read16() returns an error.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-frontends/drxd_hard.c | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/media/dvb-frontends/drxd_hard.c 
b/drivers/media/dvb-frontends/drxd_hard.c
index 3bdf9b1f4e7c..79210db117a7 100644
--- a/drivers/media/dvb-frontends/drxd_hard.c
+++ b/drivers/media/dvb-frontends/drxd_hard.c
@@ -972,7 +972,6 @@ static int DownloadMicrocode(struct drxd_state *state,
 static int HI_Command(struct drxd_state *state, u16 cmd, u16 * pResult)
 {
u32 nrRetries = 0;
-   u16 waitCmd;
int status;
 
status = Write16(state, HI_RA_RAM_SRV_CMD__A, cmd, 0);
@@ -985,8 +984,8 @@ static int HI_Command(struct drxd_state *state, u16 cmd, 
u16 * pResult)
status = -1;
break;
}
-   status = Read16(state, HI_RA_RAM_SRV_CMD__A, , 0);
-   } while (waitCmd != 0);
+   status = Read16(state, HI_RA_RAM_SRV_CMD__A, NULL, 0);
+   } while (status != 0);
 
if (status >= 0)
status = Read16(state, HI_RA_RAM_SRV_RES__A, pResult, 0);
@@ -1298,12 +1297,11 @@ static int InitFT(struct drxd_state *state)
 
 static int SC_WaitForReady(struct drxd_state *state)
 {
-   u16 curCmd;
int i;
 
for (i = 0; i < DRXD_MAX_RETRIES; i += 1) {
-   int status = Read16(state, SC_RA_RAM_CMD__A, , 0);
-   if (status == 0 || curCmd == 0)
+   int status = Read16(state, SC_RA_RAM_CMD__A, NULL, 0);
+   if (status == 0)
return status;
}
return -1;
@@ -1311,15 +1309,15 @@ static int SC_WaitForReady(struct drxd_state *state)
 
 static int SC_SendCommand(struct drxd_state *state, u16 cmd)
 {
-   int status = 0;
+   int status = 0, ret;
u16 errCode;
 
Write16(state, SC_RA_RAM_CMD__A, cmd, 0);
SC_WaitForReady(state);
 
-   Read16(state, SC_RA_RAM_CMD_ADDR__A, , 0);
+   ret = Read16(state, SC_RA_RAM_CMD_ADDR__A, , 0);
 
-   if (errCode == 0x) {
+   if (ret < 0 || errCode == 0x) {
printk(KERN_ERR "Command Error\n");
status = -1;
}
@@ -1330,13 +1328,13 @@ static int SC_SendCommand(struct drxd_state *state, u16 
cmd)
 static int SC_ProcStartCommand(struct drxd_state *state,
   u16 subCmd, u16 param0, u16 param1)
 {
-   int status = 0;
+   int ret, status = 0;
u16 scExec;
 
mutex_lock(>mutex);
do {
-   Read16(state, SC_COMM_EXEC__A, , 0);
-   if (scExec != 1) {
+   ret = Read16(state, SC_COMM_EXEC__A, , 0);
+   if (ret < 0 || scExec != 1) {
status = -1;
break;
}
-- 
2.13.6

[PATCH v2 06/26] media: xc5000: better handle I2C error messages

[Mauro Carvalho Chehab]

As warned by smatch, there are several places where the I2C
transfer may fail, leading into inconsistent behavior:

drivers/media/tuners/xc5000.c:689 xc_debug_dump() error: uninitialized 
symbol 'regval'.
drivers/media/tuners/xc5000.c:841 xc5000_is_firmware_loaded() error: 
uninitialized symbol 'id'.
drivers/media/tuners/xc5000.c:939 xc5000_set_tv_freq() error: 
uninitialized symbol 'pll_lock_status'.
drivers/media/tuners/xc5000.c:1195 xc_load_fw_and_init_tuner() error: 
uninitialized symbol 'pll_lock_status'.

Handle the return codes from the I2C transfer, in order to
address those issues.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/tuners/xc5000.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/media/tuners/xc5000.c b/drivers/media/tuners/xc5000.c
index 0e7e4fdf9e50..98ba177dbc29 100644
--- a/drivers/media/tuners/xc5000.c
+++ b/drivers/media/tuners/xc5000.c
@@ -685,8 +685,8 @@ static void xc_debug_dump(struct xc5000_priv *priv)
(totalgain % 256) * 100 / 256);
 
if (priv->pll_register_no) {
-   xc5000_readreg(priv, priv->pll_register_no, );
-   dprintk(1, "*** PLL lock status = 0x%04x\n", regval);
+   if (!xc5000_readreg(priv, priv->pll_register_no, ))
+   dprintk(1, "*** PLL lock status = 0x%04x\n", regval);
}
 }
 
@@ -831,15 +831,16 @@ static int xc5000_is_firmware_loaded(struct dvb_frontend 
*fe)
u16 id;
 
ret = xc5000_readreg(priv, XREG_PRODUCT_ID, );
-   if (ret == 0) {
+   if (!ret) {
if (id == XC_PRODUCT_ID_FW_NOT_LOADED)
ret = -ENOENT;
else
ret = 0;
+   dprintk(1, "%s() returns id = 0x%x\n", __func__, id);
+   } else {
+   dprintk(1, "%s() returns error %d\n", __func__, ret);
}
 
-   dprintk(1, "%s() returns %s id = 0x%x\n", __func__,
-   ret == 0 ? "True" : "False", id);
return ret;
 }
 
@@ -935,7 +936,10 @@ static int xc5000_set_tv_freq(struct dvb_frontend *fe)
 
if (priv->pll_register_no != 0) {
msleep(20);
-   xc5000_readreg(priv, priv->pll_register_no, _lock_status);
+   ret = xc5000_readreg(priv, priv->pll_register_no,
+_lock_status);
+   if (ret)
+   return ret;
if (pll_lock_status > 63) {
/* PLL is unlocked, force reload of the firmware */
dprintk(1, "xc5000: PLL not locked (0x%x).  
Reloading...\n",
@@ -1190,8 +1194,10 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend 
*fe, int force)
}
 
if (priv->pll_register_no) {
-   xc5000_readreg(priv, priv->pll_register_no,
-  _lock_status);
+   ret = xc5000_readreg(priv, priv->pll_register_no,
+_lock_status);
+   if (ret)
+   continue;
if (pll_lock_status > 63) {
/* PLL is unlocked, force reload of the 
firmware */
printk(KERN_ERR
-- 
2.13.6

[PATCH v2 17/26] media: mxl111sf: improve error handling logic

[Mauro Carvalho Chehab]

As reported by smatch:
drivers/media/usb/dvb-usb-v2/mxl111sf-demod.c:485 
mxl111sf_demod_read_signal_strength() error: uninitialized symbol 'modulation'.

The mxl111sf_demod_read_signal_strength() just ignores if something
gets wrong while reading snr or modulation.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/dvb-usb-v2/mxl111sf-demod.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/mxl111sf-demod.c 
b/drivers/media/usb/dvb-usb-v2/mxl111sf-demod.c
index f0ed37da73d4..221cf46b4140 100644
--- a/drivers/media/usb/dvb-usb-v2/mxl111sf-demod.c
+++ b/drivers/media/usb/dvb-usb-v2/mxl111sf-demod.c
@@ -477,10 +477,15 @@ static int mxl111sf_demod_read_signal_strength(struct 
dvb_frontend *fe,
 {
struct mxl111sf_demod_state *state = fe->demodulator_priv;
enum fe_modulation modulation;
+   int ret;
u16 snr;
 
-   mxl111sf_demod_calc_snr(state, );
-   mxl1x1sf_demod_get_tps_modulation(state, );
+   ret = mxl111sf_demod_calc_snr(state, );
+   if (ret < 0)
+   return ret;
+   ret = mxl1x1sf_demod_get_tps_modulation(state, );
+   if (ret < 0)
+   return ret;
 
switch (modulation) {
case QPSK:
-- 
2.13.6

[PATCH v2 12/26] media: davinci: fix a debug printk

[Mauro Carvalho Chehab]

Two orthogonal changesets caused a breakage at a printk
inside davinci. Changeset a2d17962c9ca
("[media] davinci: Switch from V4L2 OF to V4L2 fwnode")
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/davinci/vpif_capture.c:1399 vpif_async_bound() 
error: '%pOF' expects argument of type 'struct device_node*', argument 5 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: a2d17962c9ca ("[media] davinci: Switch from V4L2 OF to V4L2 fwnode")
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/davinci/vpif_capture.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index a89367ab1e06..1a443181a320 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1397,9 +1397,9 @@ static int vpif_async_bound(struct v4l2_async_notifier 
*notifier,
vpif_obj.config->chan_config->inputs[i].subdev_name =
(char *)to_of_node(subdev->fwnode)->full_name;
vpif_dbg(2, debug,
-"%s: setting input %d subdev_name = %pOF\n",
+"%s: setting input %d subdev_name = %s\n",
 __func__, i,
-to_of_node(subdev->fwnode));
+   
vpif_obj.config->chan_config->inputs[i].subdev_name);
return 0;
}
}
-- 
2.13.6

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

[Mauro Carvalho Chehab]

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.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/v4l2-core/v4l2-async.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-async.c 
b/drivers/media/v4l2-core/v4l2-async.c
index 49f7eccc76db..5bc861c4ae5c 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -532,7 +532,7 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
 {
struct v4l2_async_notifier *subdev_notifier;
struct v4l2_async_notifier *notifier;
-   int ret;
+   int uninitialized_var(ret);
 
/*
 * No reference taken. The reference is held by the device
-- 
2.13.6

[PATCH v2 09/26] media: cx25821-alsa: fix usage of a pointer printk

[Mauro Carvalho Chehab]

As warned by smatch:
drivers/media/pci/cx25821/cx25821-alsa.c:155 cx25821_alsa_dma_init() 
warn: argument 3 to %08lx specifier is cast from pointer

Use the standard %p to print a pointer.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/cx25821/cx25821-alsa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/cx25821/cx25821-alsa.c 
b/drivers/media/pci/cx25821/cx25821-alsa.c
index 2b34990e86f2..a45bf0331eeb 100644
--- a/drivers/media/pci/cx25821/cx25821-alsa.c
+++ b/drivers/media/pci/cx25821/cx25821-alsa.c
@@ -152,8 +152,8 @@ static int cx25821_alsa_dma_init(struct cx25821_audio_dev 
*chip, int nr_pages)
return -ENOMEM;
}
 
-   dprintk(1, "vmalloc is at addr 0x%08lx, size=%d\n",
-   (unsigned long)buf->vaddr,
+   dprintk(1, "vmalloc is at addr 0x%p, size=%d\n",
+   buf->vaddr,
nr_pages << PAGE_SHIFT);
 
memset(buf->vaddr, 0, nr_pages << PAGE_SHIFT);
-- 
2.13.6

[PATCH v2 18/26] media: dvbsky: shut up a bogus warning

[Mauro Carvalho Chehab]

Smatch gives two bogus warnings on this driver:
drivers/media/usb/dvb-usb-v2/dvbsky.c:336 dvbsky_s960_attach() error: 
uninitialized symbol 'i2c_adapter'.
drivers/media/usb/dvb-usb-v2/dvbsky.c:459 dvbsky_s960c_attach() error: 
uninitialized symbol 'i2c_adapter'.

Shut them up.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/usb/dvb-usb-v2/dvbsky.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/dvbsky.c 
b/drivers/media/usb/dvb-usb-v2/dvbsky.c
index 131b6c08e199..f90540f947f8 100644
--- a/drivers/media/usb/dvb-usb-v2/dvbsky.c
+++ b/drivers/media/usb/dvb-usb-v2/dvbsky.c
@@ -309,7 +309,7 @@ static int dvbsky_s960_attach(struct dvb_usb_adapter *adap)
struct dvb_usb_device *d = adap_to_d(adap);
int ret = 0;
/* demod I2C adapter */
-   struct i2c_adapter *i2c_adapter;
+   struct i2c_adapter *i2c_adapter = NULL;
struct i2c_client *client;
struct i2c_board_info info;
struct ts2020_config ts2020_config = {};
@@ -431,7 +431,7 @@ static int dvbsky_s960c_attach(struct dvb_usb_adapter *adap)
struct dvb_usb_device *d = adap_to_d(adap);
int ret = 0;
/* demod I2C adapter */
-   struct i2c_adapter *i2c_adapter;
+   struct i2c_adapter *i2c_adapter = NULL;
struct i2c_client *client_tuner, *client_ci;
struct i2c_board_info info;
struct sp2_config sp2_config;
-- 
2.13.6

[PATCH v2 21/26] media: m88rs2000: handle the case where tuner doesn't have get_frequency

[Mauro Carvalho Chehab]

If the tuner doesn't have get_frequency() callback, the current
code will place a random value as the frequency offset. That
doesn't seem right! The better is to just assume that, on such
case, the tuner was able to set the exact frequency that was
requested.

Fixes a smatch warning:
drivers/media/dvb-frontends/m88rs2000.c:639 m88rs2000_set_frontend() 
error: uninitialized symbol 'tuner_freq'.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-frontends/m88rs2000.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb-frontends/m88rs2000.c 
b/drivers/media/dvb-frontends/m88rs2000.c
index ce6c21d405ee..e34dab41d104 100644
--- a/drivers/media/dvb-frontends/m88rs2000.c
+++ b/drivers/media/dvb-frontends/m88rs2000.c
@@ -630,13 +630,16 @@ static int m88rs2000_set_frontend(struct dvb_frontend *fe)
if (ret < 0)
return -ENODEV;
 
-   if (fe->ops.tuner_ops.get_frequency)
+   if (fe->ops.tuner_ops.get_frequency) {
ret = fe->ops.tuner_ops.get_frequency(fe, _freq);
 
-   if (ret < 0)
-   return -ENODEV;
+   if (ret < 0)
+   return -ENODEV;
 
-   offset = (s16)((s32)tuner_freq - c->frequency);
+   offset = (s16)((s32)tuner_freq - c->frequency);
+   } else {
+   offset = 0;
+   }
 
/* default mclk value 96.4285 * 2 * 1000 = 192857 */
if (((c->frequency % 192857) >= (192857 - 3000)) ||
-- 
2.13.6

[PATCH v2 15/26] media: pt1: fix logic when pt1_nr_tables is zero or negative

[Mauro Carvalho Chehab]

pt1_nr_tables is a modprobe parameter. The way the logic
handles it, it can't be negative. However, user can
set it to zero.

If set to zero, however, it will cause troubles at
pt1_init_tables(), as reported by smatch:
drivers/media/pci/pt1/pt1.c:468 pt1_init_tables() error: uninitialized 
symbol 'first_pfn'.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/pt1/pt1.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/pci/pt1/pt1.c b/drivers/media/pci/pt1/pt1.c
index b6b1a8d20d86..acc3afeb6224 100644
--- a/drivers/media/pci/pt1/pt1.c
+++ b/drivers/media/pci/pt1/pt1.c
@@ -116,8 +116,8 @@ static u32 pt1_read_reg(struct pt1 *pt1, int reg)
return readl(pt1->regs + reg * 4);
 }
 
-static int pt1_nr_tables = 8;
-module_param_named(nr_tables, pt1_nr_tables, int, 0);
+static unsigned int pt1_nr_tables = 8;
+module_param_named(nr_tables, pt1_nr_tables, uint, 0);
 
 static void pt1_increment_table_count(struct pt1 *pt1)
 {
@@ -443,6 +443,9 @@ static int pt1_init_tables(struct pt1 *pt1)
int i, ret;
u32 first_pfn, pfn;
 
+   if (!pt1_nr_tables)
+   return 0;
+
tables = vmalloc(sizeof(struct pt1_table) * pt1_nr_tables);
if (tables == NULL)
return -ENOMEM;
@@ -450,12 +453,10 @@ static int pt1_init_tables(struct pt1 *pt1)
pt1_init_table_count(pt1);
 
i = 0;
-   if (pt1_nr_tables) {
-   ret = pt1_init_table(pt1, [0], _pfn);
-   if (ret)
-   goto err;
-   i++;
-   }
+   ret = pt1_init_table(pt1, [0], _pfn);
+   if (ret)
+   goto err;
+   i++;
 
while (i < pt1_nr_tables) {
ret = pt1_init_table(pt1, [i], );
-- 
2.13.6

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

[Mauro Carvalho Chehab]

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.

So, let's stick with the syntax that won't cause read
issues.

Signed-off-by: Mauro Carvalho Chehab 
---
 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 v2 26/26] media: mb86a16: avoid division by zero

[Mauro Carvalho Chehab]

As warned by smatch:
drivers/media/dvb-frontends/mb86a16.c:1690 mb86a16_read_ber() error: 
uninitialized symbol 'timer'.
drivers/media/dvb-frontends/mb86a16.c:1706 mb86a16_read_ber() error: 
uninitialized symbol 'timer'.

There is a potential risk of doing a division by zero if
timer is not handled well. Enforce it by setting a bit mask
for the values used to select the timer.

While here, optimize the logic to prevent uneeded tests.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-frontends/mb86a16.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/dvb-frontends/mb86a16.c 
b/drivers/media/dvb-frontends/mb86a16.c
index f1aad52094c3..5d2bb76bc07a 100644
--- a/drivers/media/dvb-frontends/mb86a16.c
+++ b/drivers/media/dvb-frontends/mb86a16.c
@@ -1677,15 +1677,15 @@ static int mb86a16_read_ber(struct dvb_frontend *fe, 
u32 *ber)
 * the deinterleaver output.
 * monitored BER is expressed as a 20 bit output in 
total
 */
-   ber_rst = ber_mon >> 3;
+   ber_rst = (ber_mon >> 3) & 0x03;
*ber = (((ber_msb << 8) | ber_mid) << 8) | ber_lsb;
if (ber_rst == 0)
timer =  1250;
-   if (ber_rst == 1)
+   else if (ber_rst == 1)
timer =  2500;
-   if (ber_rst == 2)
+   else if (ber_rst == 2)
timer =  5000;
-   if (ber_rst == 3)
+   else /* ber_rst == 3 */
timer = 1;
 
*ber /= timer;
@@ -1697,11 +1697,11 @@ static int mb86a16_read_ber(struct dvb_frontend *fe, 
u32 *ber)
 * QPSK demodulator output.
 * monitored BER is expressed as a 24 bit output in 
total
 */
-   ber_tim = ber_mon >> 1;
+   ber_tim = (ber_mon >> 1) & 0x01;
*ber = (((ber_msb << 8) | ber_mid) << 8) | ber_lsb;
if (ber_tim == 0)
timer = 16;
-   if (ber_tim == 1)
+   else /* ber_tim == 1 */
timer = 24;
 
*ber /= 2 ^ timer;
-- 
2.13.6

[PATCH v2 25/26] media: mb86a16: be more resilient if I2C fails on sync

[Mauro Carvalho Chehab]

If the I2C read fails while check for sync, there's no point
on doing adjusting the tuner due to a random value that might
be at VIRM var. So, set VIRM to zero, as that makes the caller
for check_sync() to return an error.

Fix those smatch warnings:
drivers/media/dvb-frontends/mb86a16.c:1460 mb86a16_set_fe() error: 
uninitialized symbol 'VIRM'.
drivers/media/dvb-frontends/mb86a16.c:1461 mb86a16_set_fe() error: 
uninitialized symbol 'VIRM'.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-frontends/mb86a16.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/dvb-frontends/mb86a16.c 
b/drivers/media/dvb-frontends/mb86a16.c
index dfe322eccaa1..f1aad52094c3 100644
--- a/drivers/media/dvb-frontends/mb86a16.c
+++ b/drivers/media/dvb-frontends/mb86a16.c
@@ -635,6 +635,7 @@ static int sync_chk(struct mb86a16_state *state,
return sync;
 err:
dprintk(verbose, MB86A16_ERROR, 1, "I2C transfer error");
+   *VIRM = 0;
return -EREMOTEIO;
 
 }
-- 
2.13.6

Re: [PATCH v2 01/26] media: atmel-isc: avoid returning a random value at isc_parse_dt()

[Mauro Carvalho Chehab]

Em Wed, 1 Nov 2017 18:59:48 -0200
Mauro Carvalho Chehab  escreveu:

> Em Wed,  1 Nov 2017 16:56:33 -0400
> Mauro Carvalho Chehab  escreveu:
> 
> > As warned by smatch:
> > drivers/media/platform/atmel/atmel-isc.c:2097 isc_parse_dt() error: 
> > uninitialized symbol 'ret'.
> > 
> > The problem here is that of_graph_get_next_endpoint() can
> > potentially return NULL on its first pass, with would make
> > it return a random value, as ret is not initialized.
> > 
> > While here, use while(1) instead of for(; ;), as while is
> > the preferred syntax for such kind of loops.
> 
> Sorry, please discard this e-mail... there's something wrong on my
> environment.
> 
> git send-email is dying after the first e-mail:
> 
>   Died at /usr/libexec/git-core/git-send-email line 1350.
> 
> I'll try to fix and re-send it.

Found the issue... on patch 2 of this series, it was marked with:

Cc: sta...@vger.kernel.org # for 4.14

It seems that my environment didn't like this...

Just resent the hole stuff


Thanks,
Mauro

[PATCH v2 24/26] media: stv090x: Only print tuner lock if get_status is available

[Mauro Carvalho Chehab]

The current code doesn't report tuner lock properly if the
tuner get_status callback is not available, as reported by
smatch:
drivers/media/dvb-frontends/stv090x.c:2220 stv090x_get_coldlock() 
error: uninitialized symbol 'reg'.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-frontends/stv090x.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/media/dvb-frontends/stv090x.c 
b/drivers/media/dvb-frontends/stv090x.c
index 7ef469c0c866..0f375df13fbe 100644
--- a/drivers/media/dvb-frontends/stv090x.c
+++ b/drivers/media/dvb-frontends/stv090x.c
@@ -2215,13 +2215,12 @@ static int stv090x_get_coldlock(struct stv090x_state 
*state, s32 timeout_dmd)
if (state->config->tuner_get_status) {
if (state->config->tuner_get_status(fe, ) < 0)
goto err_gateoff;
+   if (reg)
+   dprintk(FE_DEBUG, 1, "Tuner phase locked");
+   else
+   dprintk(FE_DEBUG, 1, "Tuner unlocked");
}
 
-   if (reg)
-   dprintk(FE_DEBUG, 1, "Tuner phase locked");
-   else
-   dprintk(FE_DEBUG, 1, "Tuner unlocked");
-
if (stv090x_i2c_gate_ctrl(state, 0) < 0)
goto err;
 
-- 
2.13.6

[PATCH v2 11/26] media: qt1010: fix bogus warnings

[Mauro Carvalho Chehab]

The logic at qt1010_init_meas1() and qt1010_init_meas2()
are too complex for static analizers to identify that
some vars are always be initialized.

That causes smatch to produce the following warnings:
drivers/media/tuners/qt1010.c:248 qt1010_init_meas1() error: 
uninitialized symbol 'val2'.
drivers/media/tuners/qt1010.c:282 qt1010_init_meas2() error: 
uninitialized symbol 'val'.

So, add annotations to prevent those bogus warnings.

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

diff --git a/drivers/media/tuners/qt1010.c b/drivers/media/tuners/qt1010.c
index ee33b7cc7682..b92be882ab3c 100644
--- a/drivers/media/tuners/qt1010.c
+++ b/drivers/media/tuners/qt1010.c
@@ -224,7 +224,7 @@ static int qt1010_set_params(struct dvb_frontend *fe)
 static int qt1010_init_meas1(struct qt1010_priv *priv,
 u8 oper, u8 reg, u8 reg_init_val, u8 *retval)
 {
-   u8 i, val1, val2;
+   u8 i, val1, uninitialized_var(val2);
int err;
 
qt1010_i2c_oper_t i2c_data[] = {
@@ -259,7 +259,7 @@ static int qt1010_init_meas1(struct qt1010_priv *priv,
 static int qt1010_init_meas2(struct qt1010_priv *priv,
u8 reg_init_val, u8 *retval)
 {
-   u8 i, val;
+   u8 i, uninitialized_var(val);
int err;
qt1010_i2c_oper_t i2c_data[] = {
{ QT1010_WR, 0x07, reg_init_val },
-- 
2.13.6

[PATCH v2 23/26] media: drxj: better handle errors

[Mauro Carvalho Chehab]

as reported by smatch:
drivers/media/dvb-frontends/drx39xyj/drxj.c:2157 
drxj_dap_atomic_read_write_block() error: uninitialized symbol 'word'.

The driver doesn't check if a read error occurred. Add such
check.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-frontends/drx39xyj/drxj.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-frontends/drx39xyj/drxj.c 
b/drivers/media/dvb-frontends/drx39xyj/drxj.c
index 499ccff557bf..28e24d5b7fb3 100644
--- a/drivers/media/dvb-frontends/drx39xyj/drxj.c
+++ b/drivers/media/dvb-frontends/drx39xyj/drxj.c
@@ -2151,9 +2151,13 @@ int drxj_dap_atomic_read_write_block(struct 
i2c_device_addr *dev_addr,
if (read_flag) {
/* read data from buffer */
for (i = 0; i < (datasize / 2); i++) {
-   drxj_dap_read_reg16(dev_addr,
-   (DRXJ_HI_ATOMIC_BUF_START + i),
-  , 0);
+   rc = drxj_dap_read_reg16(dev_addr,
+(DRXJ_HI_ATOMIC_BUF_START + i),
+, 0);
+   if (rc) {
+   pr_err("error %d\n", rc);
+   goto rw_error;
+   }
data[2 * i] = (u8) (word & 0xFF);
data[(2 * i) + 1] = (u8) (word >> 8);
}
-- 
2.13.6

[PATCH v2 07/26] media: radio-si476x: fix behavior when seek->range* are defined

[Mauro Carvalho Chehab]

The logic at si476x_radio_s_hw_freq_seek() checks if the
frequency range that will be used to handle hardware seek
has the minimal frequency under rangelow. That works fine
if userspace zeros both fields. However, if userspace
fills either seek->rangelow or seek-rangehigh, it won't
read the corresponding range from the device, causing the
values to be unitialized, as warned by smatch:

drivers/media/radio/radio-si476x.c:789 si476x_radio_s_hw_freq_seek() 
error: uninitialized symbol 'rangelow'.
drivers/media/radio/radio-si476x.c:789 si476x_radio_s_hw_freq_seek() 
error: uninitialized symbol 'rangehigh'.

Fix it by initializing those vars from the values present at
the struct v4l2_hw_freq_seek.

While here, simplify the logic which reads such values from
the hardware limits.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/radio/radio-si476x.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/media/radio/radio-si476x.c 
b/drivers/media/radio/radio-si476x.c
index 271f725b17e8..740714d9dd85 100644
--- a/drivers/media/radio/radio-si476x.c
+++ b/drivers/media/radio/radio-si476x.c
@@ -755,7 +755,7 @@ static int si476x_radio_s_hw_freq_seek(struct file *file, 
void *priv,
 {
int err;
enum si476x_func func;
-   u32 rangelow, rangehigh;
+   u32 rangelow = seek->rangelow, rangehigh = seek->rangehigh;
struct si476x_radio *radio = video_drvdata(file);
 
if (file->f_flags & O_NONBLOCK)
@@ -767,23 +767,21 @@ static int si476x_radio_s_hw_freq_seek(struct file *file, 
void *priv,
 
si476x_core_lock(radio->core);
 
-   if (!seek->rangelow) {
+   if (!rangelow) {
err = regmap_read(radio->core->regmap,
  SI476X_PROP_SEEK_BAND_BOTTOM,
  );
-   if (!err)
-   rangelow = si476x_to_v4l2(radio->core, rangelow);
-   else
+   if (err)
goto unlock;
+   rangelow = si476x_to_v4l2(radio->core, rangelow);
}
-   if (!seek->rangehigh) {
+   if (!rangehigh) {
err = regmap_read(radio->core->regmap,
  SI476X_PROP_SEEK_BAND_TOP,
  );
-   if (!err)
-   rangehigh = si476x_to_v4l2(radio->core, rangehigh);
-   else
+   if (err)
goto unlock;
+   rangehigh = si476x_to_v4l2(radio->core, rangehigh);
}
 
if (rangelow > rangehigh) {
-- 
2.13.6

[PATCH v2 04/26] media: tda8290: initialize agc gain

[Mauro Carvalho Chehab]

The tuning logic at tda8290 relies on agc_stat and
adc_sat to be initialized. However, as warned by smatch:

drivers/media/tuners/tda8290.c:261 tda8290_set_params() error: 
uninitialized symbol 'agc_stat'.
drivers/media/tuners/tda8290.c:261 tda8290_set_params() error: 
uninitialized symbol 'adc_sat'.
drivers/media/tuners/tda8290.c:262 tda8290_set_params() error: 
uninitialized symbol 'adc_sat'.

That could cause an erratic behavior if PLL is not locked,
as the code will only work if
!(pll_stat & 0x80) && (adc_sat < 20)

So, initialize it to zero, in order to let the code below
to be called, with should give more chances to adjust the
tuner gain, in order to get a PLL lock.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/tuners/tda8290.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/tuners/tda8290.c b/drivers/media/tuners/tda8290.c
index a59c567c55d6..f226e6ecc175 100644
--- a/drivers/media/tuners/tda8290.c
+++ b/drivers/media/tuners/tda8290.c
@@ -196,7 +196,7 @@ static void tda8290_set_params(struct dvb_frontend *fe,
unsigned char addr_adc_sat  = 0x1a;
unsigned char addr_agc_stat = 0x1d;
unsigned char addr_pll_stat = 0x1b;
-   unsigned char adc_sat, agc_stat,
+   unsigned char adc_sat = 0, agc_stat = 0,
  pll_stat;
int i;
 
-- 
2.13.6

[PATCH v2 20/26] media: imx274: don't randomly return if range_count is zero

[Mauro Carvalho Chehab]

As smatch reported:
drivers/media/i2c/imx274.c:659 imx274_regmap_util_write_table_8() 
error: uninitialized symbol 'err'.

There is a bug at imx274_regmap_util_write_table_8() with causes
it to randomly return a random error if range_count is zero.

Worse than that, the logic there starts with range_count
equal to zero, and periodically resets it to zero again.

As it is a way more likely that err assumes a non-zero value,
I suspect that the chance of this code to run is very small,
so, it would be worth to review the entire function.

Anyway, clearly it shouldn't be returning error if range_count
is zero. So, let's fix it.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/i2c/imx274.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index ab6a5f31da74..0d8314bfd3cb 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -655,6 +655,8 @@ static int imx274_regmap_util_write_table_8(struct regmap 
*regmap,
err = regmap_bulk_write(regmap, range_start,
_vals[0],
range_count);
+   else
+   err = 0;
 
if (err)
return err;
-- 
2.13.6

[PATCH v2 10/26] media: xc4000: don't ignore error if hwmodel fails

[Mauro Carvalho Chehab]

If, for some reason, reading the hwmodel register on
xc4000 fails, it will cause the following logig to
use a random value, as reported by smatch:

drivers/media/tuners/xc4000.c:1047 check_firmware() error: 
uninitialized symbol 'hwmodel'.
drivers/media/tuners/xc4000.c:1060 check_firmware() error: 
uninitialized symbol 'hwmodel'.
drivers/media/tuners/xc4000.c:1064 check_firmware() error: 
uninitialized symbol 'hwmodel'.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/tuners/xc4000.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/tuners/xc4000.c b/drivers/media/tuners/xc4000.c
index e30948e4ff87..2113ce594f75 100644
--- a/drivers/media/tuners/xc4000.c
+++ b/drivers/media/tuners/xc4000.c
@@ -1036,7 +1036,10 @@ static int check_firmware(struct dvb_frontend *fe, 
unsigned int type,
dprintk(1, "load scode failed %d\n", rc);
 
 check_device:
-   rc = xc4000_readreg(priv, XREG_PRODUCT_ID, );
+   if (xc4000_readreg(priv, XREG_PRODUCT_ID, ) < 0) {
+   printk(KERN_ERR "Unable to read tuner registers.\n");
+   goto fail;
+   }
 
if (xc_get_version(priv, _major, _minor, _major,
   _minor) != 0) {
-- 
2.13.6

Re: [PATCH v2 01/26] media: atmel-isc: avoid returning a random value at isc_parse_dt()

[Mauro Carvalho Chehab]

Em Wed, 1 Nov 2017 18:59:48 -0200
Mauro Carvalho Chehab  escreveu:

> Em Wed,  1 Nov 2017 16:56:33 -0400
> Mauro Carvalho Chehab  escreveu:
> 
> > As warned by smatch:
> > drivers/media/platform/atmel/atmel-isc.c:2097 isc_parse_dt() error: 
> > uninitialized symbol 'ret'.
> > 
> > The problem here is that of_graph_get_next_endpoint() can
> > potentially return NULL on its first pass, with would make
> > it return a random value, as ret is not initialized.
> > 
> > While here, use while(1) instead of for(; ;), as while is
> > the preferred syntax for such kind of loops.
> 
> Sorry, please discard this e-mail... there's something wrong on my
> environment.
> 
> git send-email is dying after the first e-mail:
> 
>   Died at /usr/libexec/git-core/git-send-email line 1350.

Btw, this never happened before today... probably it is because I'm trying
to send a pull request between Halloween and the Day of the Dead :-p

Thanks,
Mauro

Re: [PATCH v2 01/26] media: atmel-isc: avoid returning a random value at isc_parse_dt()

[Mauro Carvalho Chehab]

Em Wed,  1 Nov 2017 16:56:33 -0400
Mauro Carvalho Chehab  escreveu:

> As warned by smatch:
> drivers/media/platform/atmel/atmel-isc.c:2097 isc_parse_dt() error: 
> uninitialized symbol 'ret'.
> 
> The problem here is that of_graph_get_next_endpoint() can
> potentially return NULL on its first pass, with would make
> it return a random value, as ret is not initialized.
> 
> While here, use while(1) instead of for(; ;), as while is
> the preferred syntax for such kind of loops.

Sorry, please discard this e-mail... there's something wrong on my
environment.

git send-email is dying after the first e-mail:

Died at /usr/libexec/git-core/git-send-email line 1350.

I'll try to fix and re-send it.

Thanks,
Mauro

[PATCH v2 01/26] media: atmel-isc: avoid returning a random value at isc_parse_dt()

[Mauro Carvalho Chehab]

As warned by smatch:
drivers/media/platform/atmel/atmel-isc.c:2097 isc_parse_dt() error: 
uninitialized symbol 'ret'.

The problem here is that of_graph_get_next_endpoint() can
potentially return NULL on its first pass, with would make
it return a random value, as ret is not initialized.

While here, use while(1) instead of for(; ;), as while is
the preferred syntax for such kind of loops.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/atmel/atmel-isc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 13f1c1c797b0..0c2635647f69 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -2039,10 +2039,10 @@ static int isc_parse_dt(struct device *dev, struct 
isc_device *isc)
 
INIT_LIST_HEAD(>subdev_entities);
 
-   for (; ;) {
+   while (1) {
epn = of_graph_get_next_endpoint(np, epn);
if (!epn)
-   break;
+   return 0;
 
rem = of_graph_get_remote_port_parent(epn);
if (!rem) {
-- 
2.13.6

Re: [PATCH v4 10/12] intel-ipu3: css pipeline

[Sakari Ailus]

Hi Yong,

Apologies for the late reply. Please find my (few) comments below.

On Tue, Oct 17, 2017 at 10:54:55PM -0500, Yong Zhi wrote:
> Add css pipeline and v4l code.
> 
> Signed-off-by: Yong Zhi 
> ---
>  drivers/media/pci/intel/ipu3/ipu3-css.c | 1761 
> ++-
>  drivers/media/pci/intel/ipu3/ipu3-css.h |   89 ++
>  2 files changed, 1849 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-css.c 
> b/drivers/media/pci/intel/ipu3/ipu3-css.c
> index 6e615bf9378a..11f7ad3514c3 100644
> --- a/drivers/media/pci/intel/ipu3/ipu3-css.c
> +++ b/drivers/media/pci/intel/ipu3/ipu3-css.c
> @@ -13,9 +13,16 @@
>  
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
> +#include 
> +#include 
> +#include 
> +
>  #include "ipu3-css.h"
>  #include "ipu3-css-fw.h"
> +#include "ipu3-css-params.h"
>  #include "ipu3-tables.h"
>  
>  /* IRQ configuration */
> @@ -24,6 +31,159 @@
>IMGU_IRQCTRL_IRQ_SW_PIN(0) | \
>IMGU_IRQCTRL_IRQ_SW_PIN(1))
>  
> +#define IPU3_CSS_FORMAT_BPP_DEN  50  /* Denominator */
> +
> +/* Some sane limits for resolutions */
> +#define IPU3_CSS_MIN_RES 32
> +#define IPU3_CSS_MAX_H   3136
> +#define IPU3_CSS_MAX_W   4224
> +
> +/* filter size from graph settings is fixed as 4 */
> +#define FILTER_SIZE 4
> +#define MIN_ENVELOPE8
> +
> +/* Formats supported by IPU3 Camera Sub System */
> +static const struct ipu3_css_format ipu3_css_formats[] = {
> + {
> + .pixelformat = V4L2_PIX_FMT_NV12,
> + .colorspace = V4L2_COLORSPACE_SRGB,
> + .frame_format = IMGU_ABI_FRAME_FORMAT_NV12,
> + .osys_format = IMGU_ABI_OSYS_FORMAT_NV12,
> + .osys_tiling = IMGU_ABI_OSYS_TILING_NONE,
> + .bytesperpixel_num = 1 * IPU3_CSS_FORMAT_BPP_DEN,
> + .chroma_decim = 4,
> + .width_align = IPU3_UAPI_ISP_VEC_ELEMS,
> + .flags = IPU3_CSS_FORMAT_FL_OUT | IPU3_CSS_FORMAT_FL_VF,
> + }, {
> + /* Each 32 bytes contains 25 10-bit pixels */
> + .pixelformat = V4L2_PIX_FMT_IPU3_SBGGR10,
> + .colorspace = V4L2_COLORSPACE_RAW,
> + .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED,
> + .bayer_order = IMGU_ABI_BAYER_ORDER_BGGR,
> + .bit_depth = 10,
> + .bytesperpixel_num = 64,
> + .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS,
> + .flags = IPU3_CSS_FORMAT_FL_IN,
> + }, {
> + .pixelformat = V4L2_PIX_FMT_IPU3_SGBRG10,
> + .colorspace = V4L2_COLORSPACE_RAW,
> + .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED,
> + .bayer_order = IMGU_ABI_BAYER_ORDER_GBRG,
> + .bit_depth = 10,
> + .bytesperpixel_num = 64,
> + .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS,
> + .flags = IPU3_CSS_FORMAT_FL_IN,
> + }, {
> + .pixelformat = V4L2_PIX_FMT_IPU3_SGRBG10,
> + .colorspace = V4L2_COLORSPACE_RAW,
> + .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED,
> + .bayer_order = IMGU_ABI_BAYER_ORDER_GRBG,
> + .bit_depth = 10,
> + .bytesperpixel_num = 64,
> + .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS,
> + .flags = IPU3_CSS_FORMAT_FL_IN,
> + }, {
> + .pixelformat = V4L2_PIX_FMT_IPU3_SRGGB10,
> + .colorspace = V4L2_COLORSPACE_RAW,
> + .frame_format = IMGU_ABI_FRAME_FORMAT_RAW_PACKED,
> + .bayer_order = IMGU_ABI_BAYER_ORDER_RGGB,
> + .bit_depth = 10,
> + .bytesperpixel_num = 64,
> + .width_align = 2 * IPU3_UAPI_ISP_VEC_ELEMS,
> + .flags = IPU3_CSS_FORMAT_FL_IN,
> + },
> +};
> +
> +static const struct {
> + enum imgu_abi_queue_id qid;
> + size_t ptr_ofs;
> +} ipu3_css_queues[IPU3_CSS_QUEUES] = {
> + [IPU3_CSS_QUEUE_IN] = {
> + IMGU_ABI_QUEUE_C_ID,
> + offsetof(struct imgu_abi_buffer, payload.frame.frame_data)
> + },
> + [IPU3_CSS_QUEUE_OUT] = {
> + IMGU_ABI_QUEUE_D_ID,
> + offsetof(struct imgu_abi_buffer, payload.frame.frame_data)
> + },
> + [IPU3_CSS_QUEUE_VF] = {
> + IMGU_ABI_QUEUE_E_ID,
> + offsetof(struct imgu_abi_buffer, payload.frame.frame_data)
> + },
> + [IPU3_CSS_QUEUE_STAT_3A] = {
> + IMGU_ABI_QUEUE_F_ID,
> + offsetof(struct imgu_abi_buffer, payload.s3a.data_ptr)
> + },
> + [IPU3_CSS_QUEUE_STAT_DVS] = {
> + IMGU_ABI_QUEUE_G_ID,
> + offsetof(struct imgu_abi_buffer, payload.skc_dvs_statistics)
> + }
> +};
> +
> +/* Initialize queue based on given format, adjust format as needed */
> +static int ipu3_css_queue_init(struct ipu3_css_queue *queue,
> +struct 

Alert

[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

Re: Nokia N9: fun with camera

[Pavel Machek]

Hi!

> > Sakari, I am actually playing with N9 camera, not N950. That comes
> > next.
> > 
> > And the clock error I mentioned ... seems to be
> > -EPROBE_DEFER. So... not an issue.
> 
> Hmm, and with similar config, I got N950 to work. ... which should
> give me enough clues to get N9 to work. I guess I forgot to reset the
> pipeline between the tries, or something.
> 
> For the record, this got me some data on n950:
> 
>  m.media_ctl( [ '-f', '"OMAP3 ISP CSI2a":0 [fmt:%s/%dx%d]' % (m.fmt, m.cap_x, 
> m.cap_y) ] )
>  m.media_ctl( [ '-l', '"OMAP3 ISP CSI2a":1 -> "OMAP3 ISP CSI2a output":0[1]' 
> ] )
> 
>  # WORKS
>  # pavel@n900:~/g/tui/camera$ sudo /my/tui/yavta/yavta
>  # --capture=8 --skip 0 --format SGRBG10 --size 4272x3016 /dev/video1 
> --file=/tmp/delme#
> 
> ...ouch. It only worked twice :-(. Either driver gets confused by my
> attempts, or it relied on some other initialization code. Strange.

Hmm, so it works "reliably" after boot. But it also locks up machine
with high probability. If you have N950, it might still be handy...

Messages are:

['-r']
['-f', '"OMAP3 ISP CSI2a":0 [fmt:SGRBG10/4272x3016]']
Warning: the -f option is deprecated and has been replaced by -V.
['-l', '"OMAP3 ISP CSI2a":1 -> "OMAP3 ISP CSI2a output":0[1]']
Testing: is raw
Testing:  /my/tui/yavta/yavta --capture=8 --skip 0 --format SGRBG10
--size 4272x3016 /dev/video_sensor --file=/tmp/delme#
Error opening device /dev/video_sensor: Permission denied (13).
Raw mode is  1
Raw mode is  1
Testing: is raw
Testing:  /my/tui/yavta/yavta --capture=8 --skip 0 --format SGRBG10
--size 4272x3016 /dev/video_sensor --file=/tmp/delme#
Device /dev/video_sensor opened.
Device `OMAP3 ISP CSI2a output' on `media' is a video capture (without
mplanes) device.
Video format set: SGRBG10 (30314142) 4272x3016 (stride 8544) field
none buffer size 25768704
Video format: SGRBG10 (30314142) 4272x3016 (stride 8544) field none
buffer size 25768704
1 buffers requested.
length: 25768704 offset: 0 timestamp type/source: mono/EoF
Buffer 0/0 mapped at address 0xb54e9000.
0 (0) [-] none 0 25768704 B 691.756907 691.758403 6.212 fps ts
mono/EoF
1 (0) [-] none 1 25768704 B 694.821025 694.821422 0.326 fps ts
mono/EoF
2 (0) [E] none 2 25768704 B 698.530833 698.534739 0.270 fps ts
mono/EoF
3 (0) [-] none 3 25768704 B 700.788127 700.790996 0.443 fps ts
mono/EoF
(and here it locked up :-(. Sometimes it captures more.)

Thanks,

Pavel




-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

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

[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(-)

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

Re: [PATCH] [RFC] media: camss-vfe: always initialize reg at vfe_set_xbar_cfg()

[Todor Tomov]

On  1.11.2017 15:25, Mauro Carvalho Chehab wrote:
> Em Wed, 1 Nov 2017 15:09:36 +0200
> Todor Tomov  escreveu:
> 
>> On  1.11.2017 15:03, Mauro Carvalho Chehab wrote:
>>> Hi Todor,
>>>
>>> Em Wed, 1 Nov 2017 14:38:02 +0200
>>> Todor Tomov  escreveu:
>>>   
 Hi Mauro,

 Thank you for pointing to this.

 On  1.11.2017 14:16, Mauro Carvalho Chehab wrote:  
> if output->wm_num is bigger than 1, the value for reg is
 If output->wn_num equals 2, we handle all cases (i == 0, i == 1) and set 
 reg properly.
 If output->wn_num is bigger than 2, then reg will not be initialized. 
 However this is something that "cannot happen" and because of this the 
 case is not handled.

 So I think that there is nothing wrong really but we have to do something 
 to remove the warning. I agree with your patch, it is technically not a 
 right value for reg but any cases in which wm_num is bigger than 2 are not 
 supported anyway and should not happen.  
>>>
>>> Thanks for your promptly answer. Well, if i is always at the [0..1] range,
>>> then I guess the enclosed patch is actually better.  
>>
>> I don't think that there is a lot of difference practically. If this one 
>> fixes the warning too, then it is fine for me. Thank you for working on this.
> 
> Yes, it fixes the warning. I guess the main advantage of version 2 is
> that, if it would ever be possible to have more than 2 outputs, it
> should be clearer that the logic at break need changes.
> 
> So, if ok for you, I'll stick with version 2. It would be great if
> you could give your ack on that.

Sure, and you can add:
Acked-by: Todor Tomov 

Thank you.
Todor

Re: [PATCH] media: v4l2-fwnode: use a typedef for a function callback

[Sakari Ailus]

Hi Mauro,

Thanks for the patch.

There's btw. something unexpected in the To header that mutt doesn't handle
well; the end result is that the mail, by default, will be addressed to the
sender (you) only. That has likely been the reason why the list (as well as
other recipients) have been occasionally dropped.

Please see my comments below.

On Wed, Nov 01, 2017 at 09:22:50AM -0400, Mauro Carvalho Chehab wrote:
> That allows having a kernel-doc markup for the function
> prototype. It also prevents the need of describing the
> return values twice.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  include/media/v4l2-fwnode.h | 37 +++--
>  1 file changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
> index ca50108dfd8f..9b04692e4fde 100644
> --- a/include/media/v4l2-fwnode.h
> +++ b/include/media/v4l2-fwnode.h
> @@ -203,6 +203,27 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode,
>   */
>  void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);
>  
> +
> +/**
> + * typedef parse_endpoint_fnc - Driver's callback function to be called on
> + *   each V4L2 fwnode endpoint.
> + *
> + * @dev: pointer to  device
> + * @vep: pointer to  v4l2_fwnode_endpoint
> + * @asd: pointer to  v4l2_async_subdev
> + *
> + * Return:
> + * * %0 on success
> + * * %-ENOTCONN if the endpoint is to be skipped but this
> + *   should not be considered as an error
> + * * %-EINVAL if the endpoint configuration is invalid
> + */
> +
> +typedef  int (*parse_endpoint_fnc)(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd);
> +
> +

Extra newline. I'd call the typedef parse_endpoint_func; dropping the "u"
doesn't really shorten it much, just makes it look odd IMO.

With that,

Acked-by: Sakari Ailus 

>  /**
>   * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode endpoints 
> in a
>   *   device node
> @@ -215,10 +236,6 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);
>   *begin at the same memory address.
>   * @parse_endpoint: Driver's callback function called on each V4L2 fwnode
>   *   endpoint. Optional.
> - *   Return: %0 on success
> - *   %-ENOTCONN if the endpoint is to be skipped but this
> - *  should not be considered as an error
> - *   %-EINVAL if the endpoint configuration is invalid
>   *
>   * Parse the fwnode endpoints of the @dev device and populate the async sub-
>   * devices array of the notifier. The @parse_endpoint callback function is
> @@ -253,9 +270,7 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);
>  int v4l2_async_notifier_parse_fwnode_endpoints(
>   struct device *dev, struct v4l2_async_notifier *notifier,
>   size_t asd_struct_size,
> - int (*parse_endpoint)(struct device *dev,
> -   struct v4l2_fwnode_endpoint *vep,
> -   struct v4l2_async_subdev *asd));
> + parse_endpoint_fnc parse_endpoint);
>  
>  /**
>   * v4l2_async_notifier_parse_fwnode_endpoints_by_port - Parse V4L2 fwnode
> @@ -271,10 +286,6 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
>   * @port: port number where endpoints are to be parsed
>   * @parse_endpoint: Driver's callback function called on each V4L2 fwnode
>   *   endpoint. Optional.
> - *   Return: %0 on success
> - *   %-ENOTCONN if the endpoint is to be skipped but this
> - *  should not be considered as an error
> - *   %-EINVAL if the endpoint configuration is invalid
>   *
>   * This function is just like v4l2_async_notifier_parse_fwnode_endpoints() 
> with
>   * the exception that it only parses endpoints in a given port. This is 
> useful
> @@ -315,9 +326,7 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
>  int v4l2_async_notifier_parse_fwnode_endpoints_by_port(
>   struct device *dev, struct v4l2_async_notifier *notifier,
>   size_t asd_struct_size, unsigned int port,
> - int (*parse_endpoint)(struct device *dev,
> -   struct v4l2_fwnode_endpoint *vep,
> -   struct v4l2_async_subdev *asd));
> + parse_endpoint_fnc parse_endpoint);
>  
>  /**
>   * v4l2_fwnode_reference_parse_sensor_common - parse common references on
> -- 
> 2.13.6
> 

-- 
Kind regards,

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

Re: [PATCH] [RFC] media: camss-vfe: always initialize reg at vfe_set_xbar_cfg()

[Mauro Carvalho Chehab]

Em Wed, 1 Nov 2017 15:09:36 +0200
Todor Tomov  escreveu:

> On  1.11.2017 15:03, Mauro Carvalho Chehab wrote:
> > Hi Todor,
> > 
> > Em Wed, 1 Nov 2017 14:38:02 +0200
> > Todor Tomov  escreveu:
> >   
> >> Hi Mauro,
> >>
> >> Thank you for pointing to this.
> >>
> >> On  1.11.2017 14:16, Mauro Carvalho Chehab wrote:  
> >>> if output->wm_num is bigger than 1, the value for reg is
> >> If output->wn_num equals 2, we handle all cases (i == 0, i == 1) and set 
> >> reg properly.
> >> If output->wn_num is bigger than 2, then reg will not be initialized. 
> >> However this is something that "cannot happen" and because of this the 
> >> case is not handled.
> >>
> >> So I think that there is nothing wrong really but we have to do something 
> >> to remove the warning. I agree with your patch, it is technically not a 
> >> right value for reg but any cases in which wm_num is bigger than 2 are not 
> >> supported anyway and should not happen.  
> > 
> > Thanks for your promptly answer. Well, if i is always at the [0..1] range,
> > then I guess the enclosed patch is actually better.  
> 
> I don't think that there is a lot of difference practically. If this one 
> fixes the warning too, then it is fine for me. Thank you for working on this.

Yes, it fixes the warning. I guess the main advantage of version 2 is
that, if it would ever be possible to have more than 2 outputs, it
should be clearer that the logic at break need changes.

So, if ok for you, I'll stick with version 2. It would be great if
you could give your ack on that.

Regards,
Mauro

[PATCH] media: v4l2-fwnode: use a typedef for a function callback

[Mauro Carvalho Chehab]

That allows having a kernel-doc markup for the function
prototype. It also prevents the need of describing the
return values twice.

Signed-off-by: Mauro Carvalho Chehab 
---
 include/media/v4l2-fwnode.h | 37 +++--
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index ca50108dfd8f..9b04692e4fde 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -203,6 +203,27 @@ int v4l2_fwnode_parse_link(struct fwnode_handle *fwnode,
  */
 void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);
 
+
+/**
+ * typedef parse_endpoint_fnc - Driver's callback function to be called on
+ * each V4L2 fwnode endpoint.
+ *
+ * @dev: pointer to  device
+ * @vep: pointer to  v4l2_fwnode_endpoint
+ * @asd: pointer to  v4l2_async_subdev
+ *
+ * Return:
+ * * %0 on success
+ * * %-ENOTCONN if the endpoint is to be skipped but this
+ *   should not be considered as an error
+ * * %-EINVAL if the endpoint configuration is invalid
+ */
+
+typedefint (*parse_endpoint_fnc)(struct device *dev,
+   struct v4l2_fwnode_endpoint *vep,
+   struct v4l2_async_subdev *asd);
+
+
 /**
  * v4l2_async_notifier_parse_fwnode_endpoints - Parse V4L2 fwnode endpoints in 
a
  * device node
@@ -215,10 +236,6 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);
  *  begin at the same memory address.
  * @parse_endpoint: Driver's callback function called on each V4L2 fwnode
  * endpoint. Optional.
- * Return: %0 on success
- * %-ENOTCONN if the endpoint is to be skipped but this
- *should not be considered as an error
- * %-EINVAL if the endpoint configuration is invalid
  *
  * Parse the fwnode endpoints of the @dev device and populate the async sub-
  * devices array of the notifier. The @parse_endpoint callback function is
@@ -253,9 +270,7 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link *link);
 int v4l2_async_notifier_parse_fwnode_endpoints(
struct device *dev, struct v4l2_async_notifier *notifier,
size_t asd_struct_size,
-   int (*parse_endpoint)(struct device *dev,
- struct v4l2_fwnode_endpoint *vep,
- struct v4l2_async_subdev *asd));
+   parse_endpoint_fnc parse_endpoint);
 
 /**
  * v4l2_async_notifier_parse_fwnode_endpoints_by_port - Parse V4L2 fwnode
@@ -271,10 +286,6 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
  * @port: port number where endpoints are to be parsed
  * @parse_endpoint: Driver's callback function called on each V4L2 fwnode
  * endpoint. Optional.
- * Return: %0 on success
- * %-ENOTCONN if the endpoint is to be skipped but this
- *should not be considered as an error
- * %-EINVAL if the endpoint configuration is invalid
  *
  * This function is just like v4l2_async_notifier_parse_fwnode_endpoints() with
  * the exception that it only parses endpoints in a given port. This is useful
@@ -315,9 +326,7 @@ int v4l2_async_notifier_parse_fwnode_endpoints(
 int v4l2_async_notifier_parse_fwnode_endpoints_by_port(
struct device *dev, struct v4l2_async_notifier *notifier,
size_t asd_struct_size, unsigned int port,
-   int (*parse_endpoint)(struct device *dev,
- struct v4l2_fwnode_endpoint *vep,
- struct v4l2_async_subdev *asd));
+   parse_endpoint_fnc parse_endpoint);
 
 /**
  * v4l2_fwnode_reference_parse_sensor_common - parse common references on
-- 
2.13.6

Re: [PATCH] [RFC] media: camss-vfe: always initialize reg at vfe_set_xbar_cfg()

[Todor Tomov]

On  1.11.2017 15:03, Mauro Carvalho Chehab wrote:
> Hi Todor,
> 
> Em Wed, 1 Nov 2017 14:38:02 +0200
> Todor Tomov  escreveu:
> 
>> Hi Mauro,
>>
>> Thank you for pointing to this.
>>
>> On  1.11.2017 14:16, Mauro Carvalho Chehab wrote:
>>> if output->wm_num is bigger than 1, the value for reg is  
>> If output->wn_num equals 2, we handle all cases (i == 0, i == 1) and set reg 
>> properly.
>> If output->wn_num is bigger than 2, then reg will not be initialized. 
>> However this is something that "cannot happen" and because of this the case 
>> is not handled.
>>
>> So I think that there is nothing wrong really but we have to do something to 
>> remove the warning. I agree with your patch, it is technically not a right 
>> value for reg but any cases in which wm_num is bigger than 2 are not 
>> supported anyway and should not happen.
> 
> Thanks for your promptly answer. Well, if i is always at the [0..1] range,
> then I guess the enclosed patch is actually better.

I don't think that there is a lot of difference practically. If this one fixes 
the warning too, then it is fine for me. Thank you for working on this.

Best regards,
Todor

> 
> 
> Thanks,
> Mauro
> 
> 
> [PATCH] media: camss-vfe: always initialize reg at vfe_set_xbar_cfg()
> 
> if output->wm_num is bigger than 2, the value for reg is
> not initialized, as warned by smatch:
>   drivers/media/platform/qcom/camss-8x16/camss-vfe.c:633 
> vfe_set_xbar_cfg() error: uninitialized symbol 'reg'.
>   drivers/media/platform/qcom/camss-8x16/camss-vfe.c:637 
> vfe_set_xbar_cfg() error: uninitialized symbol 'reg'.
> 
> That shouldn't happen in practice, so add a logic that will
> break the loop if i > 1, fixing the warnings.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c 
> b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> index b22d2dfcd3c2..55232a912950 100644
> --- a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> @@ -622,6 +622,9 @@ static void vfe_set_xbar_cfg(struct vfe_device *vfe, 
> struct vfe_output *output,
>   reg = VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_EN;
>   if (p == V4L2_PIX_FMT_NV12 || p == V4L2_PIX_FMT_NV16)
>   reg |= 
> VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_SWAP_INTER_INTRA;
> + } else {
> + /* On current devices output->wm_num is always <= 2 */
> + break;
>   }
>  
>   if (output->wm_idx[i] % 2 == 1)
> 

Re: [PATCH] [RFC] media: camss-vfe: always initialize reg at vfe_set_xbar_cfg()

[Mauro Carvalho Chehab]

Hi Todor,

Em Wed, 1 Nov 2017 14:38:02 +0200
Todor Tomov  escreveu:

> Hi Mauro,
> 
> Thank you for pointing to this.
> 
> On  1.11.2017 14:16, Mauro Carvalho Chehab wrote:
> > if output->wm_num is bigger than 1, the value for reg is  
> If output->wn_num equals 2, we handle all cases (i == 0, i == 1) and set reg 
> properly.
> If output->wn_num is bigger than 2, then reg will not be initialized. However 
> this is something that "cannot happen" and because of this the case is not 
> handled.
> 
> So I think that there is nothing wrong really but we have to do something to 
> remove the warning. I agree with your patch, it is technically not a right 
> value for reg but any cases in which wm_num is bigger than 2 are not 
> supported anyway and should not happen.

Thanks for your promptly answer. Well, if i is always at the [0..1] range,
then I guess the enclosed patch is actually better.


Thanks,
Mauro


[PATCH] media: camss-vfe: always initialize reg at vfe_set_xbar_cfg()

if output->wm_num is bigger than 2, the value for reg is
not initialized, as warned by smatch:
drivers/media/platform/qcom/camss-8x16/camss-vfe.c:633 
vfe_set_xbar_cfg() error: uninitialized symbol 'reg'.
drivers/media/platform/qcom/camss-8x16/camss-vfe.c:637 
vfe_set_xbar_cfg() error: uninitialized symbol 'reg'.

That shouldn't happen in practice, so add a logic that will
break the loop if i > 1, fixing the warnings.

Signed-off-by: Mauro Carvalho Chehab 

diff --git a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c 
b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
index b22d2dfcd3c2..55232a912950 100644
--- a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
@@ -622,6 +622,9 @@ static void vfe_set_xbar_cfg(struct vfe_device *vfe, struct 
vfe_output *output,
reg = VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_EN;
if (p == V4L2_PIX_FMT_NV12 || p == V4L2_PIX_FMT_NV16)
reg |= 
VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_SWAP_INTER_INTRA;
+   } else {
+   /* On current devices output->wm_num is always <= 2 */
+   break;
}
 
if (output->wm_idx[i] % 2 == 1)

[PATCH 01/14] media: atmel-isc: avoid returning a random value at isc_parse_dt()

[Mauro Carvalho Chehab]

As warned by smatch:
drivers/media/platform/atmel/atmel-isc.c:2097 isc_parse_dt() error: 
uninitialized symbol 'ret'.

The problem here is that of_graph_get_next_endpoint() can
potentially return NULL on its first pass, with would make
it return a random value, as ret is not initialized.

While here, use while(1) instead of for(; ;), as while is
the preferred syntax for such kind of loops.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/atmel/atmel-isc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 13f1c1c797b0..0c2635647f69 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -2039,10 +2039,10 @@ static int isc_parse_dt(struct device *dev, struct 
isc_device *isc)
 
INIT_LIST_HEAD(>subdev_entities);
 
-   for (; ;) {
+   while (1) {
epn = of_graph_get_next_endpoint(np, epn);
if (!epn)
-   break;
+   return 0;
 
rem = of_graph_get_remote_port_parent(epn);
if (!rem) {
-- 
2.13.6

Re: [PATCH] media: rc: remove ir-rx51 in favour of generic pwm-ir-tx

[Sean Young]

On Wed, Nov 01, 2017 at 12:58:21PM +0100, Pali Rohár wrote:
> On Wednesday 01 November 2017 11:55:33 Sean Young wrote:
> > The ir-rx51 is a pwm-based TX driver specific to the n900. This can be
> > handled entirely by the generic pwm-ir-tx driver.
> > 
> > Note that the suspend code in the ir-rx51 driver is unnecessary, since
> > during transmit, the current process is not in interruptable sleep. The
> > process is not put to sleep until the transmit completes.
> 
> Hello, have you tested this patch that IR transmitter is still working
> fine on the real Nokia N900 device?

No, I have not. My n900 died many years ago. I was hoping someone could
this for me please.

I should have called this out more explicitly.

If anyone could test please, thank you very much!

Regards,

Sean

Re: [PATCH] [RFC] media: camss-vfe: always initialize reg at vfe_set_xbar_cfg()

[Todor Tomov]

Hi Mauro,

Thank you for pointing to this.

On  1.11.2017 14:16, Mauro Carvalho Chehab wrote:
> if output->wm_num is bigger than 1, the value for reg is
If output->wn_num equals 2, we handle all cases (i == 0, i == 1) and set reg 
properly.
If output->wn_num is bigger than 2, then reg will not be initialized. However 
this is something that "cannot happen" and because of this the case is not 
handled.

So I think that there is nothing wrong really but we have to do something to 
remove the warning. I agree with your patch, it is technically not a right 
value for reg but any cases in which wm_num is bigger than 2 are not supported 
anyway and should not happen.

> not initialized, as warned by smatch:
>   drivers/media/platform/qcom/camss-8x16/camss-vfe.c:633 
> vfe_set_xbar_cfg() error: uninitialized symbol 'reg'.
>   drivers/media/platform/qcom/camss-8x16/camss-vfe.c:637 
> vfe_set_xbar_cfg() error: uninitialized symbol 'reg'.
> 
> I didn't check the logic into its details, but there is at least
> one point where wm_num is made equal to two. So, something
> seem broken.
> 
> For now, I just reset it to zero, and added a FIXME. Hopefully,
> the driver authors will know if this is OK, or if something else
> is needed there.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/platform/qcom/camss-8x16/camss-vfe.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c 
> b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> index b22d2dfcd3c2..388431f747fa 100644
> --- a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
> @@ -622,6 +622,8 @@ static void vfe_set_xbar_cfg(struct vfe_device *vfe, 
> struct vfe_output *output,
>   reg = VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_EN;
>   if (p == V4L2_PIX_FMT_NV12 || p == V4L2_PIX_FMT_NV16)
>   reg |= 
> VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_SWAP_INTER_INTRA;
> + } else {
> + reg = 0;/* FIXME: is it the right value for i > 
> 1? */
>   }
>  
>   if (output->wm_idx[i] % 2 == 1)
> 

-- 
Best regards,
Todor Tomov

Re: [PATCHv3 1/2] tda18250: support for new silicon tuner

[Mauro Carvalho Chehab]

Em Mon, 30 Oct 2017 05:31:40 +0200
Olli Salonen  escreveu:

> Hello Michael,
> 
> Many thanks for taking the time to review the patches.
> 
> On 27 October 2017 at 13:27, Michael Ira Krufky  wrote:
> >> +static int tda18250_sleep(struct dvb_frontend *fe)
> >> +{
> >> +   struct i2c_client *client = fe->tuner_priv;
> >> +   struct tda18250_dev *dev = i2c_get_clientdata(client);
> >> +   int ret;
> >> +
> >> +   dev_dbg(>dev, "\n");
> >> +
> >> +   /* power down LNA */
> >> +   ret = regmap_write_bits(dev->regmap, R0C_AGC11, 0x80, 0x00);
> >> +   if (ret)
> >> +   return ret;
> >> +
> >> +   ret = tda18250_power_control(fe, TDA18250_POWER_STANDBY);
> >> +   return ret;
> >> +}  
> >
> > Do we know for sure if the IF_FREQUENCY is preserved after returning
> > from a sleep?   It might be a good idea to set `dev->if_frequency = 0`
> > within `tda18250_sleep` to be sure that it gets set again on the next
> > tune, but you may want to check the specification first, if its
> > available.
> >
> > This is not a show-stopper -- We can merge this as-is and this can be
> > handled in a follow-up patch.  
> 
> I will look into this and send a patch on top of this one if needed.
> 
> Thank you for pointing it out.

There is a show-stopper issue here, though: it lacks adding an
entry for the driver at MAINTAINERS file :-)

Please add it.

While here, please look at the checkpatch warnings:

WARNING: msleep < 20ms can sleep for up to 20ms; see 
Documentation/timers/timers-howto.txt
#746: FILE: drivers/media/tuners/tda18250.c:678:
+ msleep(5);

and the 80 column ones. I was unable to see, at the places it complained,
a reason why not limit the lines to 80 columns.

Regards,

Thanks,
Mauro

Re: [PATCH] media: rc: remove ir-rx51 in favour of generic pwm-ir-tx

[Sebastian Reichel]

Hi,

On Wed, Nov 01, 2017 at 11:55:33AM +, Sean Young wrote:
> The ir-rx51 is a pwm-based TX driver specific to the n900. This can be
> handled entirely by the generic pwm-ir-tx driver.
> 
> Note that the suspend code in the ir-rx51 driver is unnecessary, since
> during transmit, the current process is not in interruptable sleep. The
> process is not put to sleep until the transmit completes.

Nice to see, that we can use generic driver now :)

> Cc: Ivaylo Dimitrov 
> Cc: Pali Rohár 
> Cc: Pavel Machek 
> Cc: Timo Kokkonen 
> Signed-off-by: Sean Young 
> ---
>  arch/arm/boot/dts/omap3-n900.dts |   2 +-
>  drivers/media/rc/Kconfig |  10 --
>  drivers/media/rc/Makefile|   1 -
>  drivers/media/rc/ir-rx51.c   | 316 
> ---
>  4 files changed, 1 insertion(+), 328 deletions(-)
>  delete mode 100644 drivers/media/rc/ir-rx51.c
> 
> diff --git a/arch/arm/boot/dts/omap3-n900.dts 
> b/arch/arm/boot/dts/omap3-n900.dts
> index 4acd32a1c4ef..fccd2b119c0a 100644
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -152,7 +152,7 @@
>   };
>  
>   ir: n900-ir {
> - compatible = "nokia,n900-ir";
> + compatible = "pwm-ir-tx";

I think we should update DTS to look like this:

compatible = "nokia,n900-ir", "pwm-ir-tx";

This will keep new DTS working with old kernel. Also we want it
working the other way around (old DTS with new kernel), so we
need to add the "nokia,n900-ir" compatible to the generic PWM
IR driver.

Also the DT binding document needs update:

Documentation/devicetree/bindings/media/nokia,n900-ir

[Adding Rob Herring to Cc]

-- Sebastian

>   pwms = < 0 26316 0>; /* 38000 Hz */
>   };
>  
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index 0f863822889e..354ee5224758 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -353,16 +353,6 @@ config IR_TTUSBIR
>  To compile this driver as a module, choose M here: the module will
>  be called ttusbir.
>  
> -config IR_RX51
> - tristate "Nokia N900 IR transmitter diode"
> - depends on (OMAP_DM_TIMER && PWM_OMAP_DMTIMER && ARCH_OMAP2PLUS || 
> COMPILE_TEST) && RC_CORE
> - ---help---
> -Say Y or M here if you want to enable support for the IR
> -transmitter diode built in the Nokia N900 (RX51) device.
> -
> -The driver uses omap DM timers for generating the carrier
> -wave and pulses.
> -
>  source "drivers/media/rc/img-ir/Kconfig"
>  
>  config RC_LOOPBACK
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index a1ef86767aef..59b99a2dc2d4 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -25,7 +25,6 @@ obj-$(CONFIG_IR_MESON) += meson-ir.o
>  obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
>  obj-$(CONFIG_IR_ENE) += ene_ir.o
>  obj-$(CONFIG_IR_REDRAT3) += redrat3.o
> -obj-$(CONFIG_IR_RX51) += ir-rx51.o
>  obj-$(CONFIG_IR_SPI) += ir-spi.o
>  obj-$(CONFIG_IR_STREAMZAP) += streamzap.o
>  obj-$(CONFIG_IR_WINBOND_CIR) += winbond-cir.o
> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
> deleted file mode 100644
> index 49265f02e772..
> --- a/drivers/media/rc/ir-rx51.c
> +++ /dev/null
> @@ -1,316 +0,0 @@
> -/*
> - *  Copyright (C) 2008 Nokia Corporation
> - *
> - *  Based on lirc_serial.c
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; either version 2 of the License, or
> - *  (at your option) any later version.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - */
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#include 
> -#include 
> -
> -#define WBUF_LEN 256
> -
> -struct ir_rx51 {
> - struct rc_dev *rcdev;
> - struct pwm_device *pwm;
> - struct hrtimer timer;
> - struct device*dev;
> - struct ir_rx51_platform_data *pdata;
> - wait_queue_head_t wqueue;
> -
> - unsigned intfreq;   /* carrier frequency */
> - unsigned intduty_cycle; /* carrier duty cycle */
> - int wbuf[WBUF_LEN];
> - int wbuf_index;
> - unsigned long   device_is_open;
> -};
> -
> -static inline void ir_rx51_on(struct ir_rx51 *ir_rx51)
> -{
> - pwm_enable(ir_rx51->pwm);
> -}
> -
> -static inline void ir_rx51_off(struct ir_rx51 *ir_rx51)
> -{
> - pwm_disable(ir_rx51->pwm);
> -}
> -
> -static int init_timing_params(struct ir_rx51 *ir_rx51)
> -{
> - struct pwm_device 

[ragnatech:media-tree] BUILD SUCCESS 1acce5f72cfabcafee5e101b9ac7d71ebe1c7af9

[kbuild test robot]

tree/branch: git://git.ragnatech.se/linux  media-tree
branch HEAD: 1acce5f72cfabcafee5e101b9ac7d71ebe1c7af9  media: v4l2-fwnode: use 
the cached value instead of getting again

elapsed time: 173m

configs tested: 155

The following configs have been built successfully.
More configs may be tested in the coming days.

shtitan_defconfig
sh  rsk7269_defconfig
sh  sh7785lcr_32bit_defconfig
shallnoconfig
i386   randconfig-x019-201744
i386   randconfig-x013-201744
i386   randconfig-x014-201744
i386   randconfig-x015-201744
i386   randconfig-x011-201744
i386   randconfig-x018-201744
i386   randconfig-x010-201744
i386   randconfig-x017-201744
i386   randconfig-x012-201744
i386   randconfig-x016-201744
x86_64 randconfig-x000-201744
x86_64 randconfig-x001-201744
x86_64 randconfig-x008-201744
x86_64 randconfig-x004-201744
x86_64 randconfig-x005-201744
x86_64 randconfig-x006-201744
x86_64 randconfig-x007-201744
x86_64 randconfig-x003-201744
x86_64 randconfig-x009-201744
x86_64 randconfig-x002-201744
powerpc defconfig
s390default_defconfig
powerpc   ppc64_defconfig
powerpc   allnoconfig
x86_64  defconfig
armu300_defconfig
powerpc canyonlands_defconfig
alpha   defconfig
pariscallnoconfig
parisc b180_defconfig
pariscc3000_defconfig
parisc  defconfig
x86_64 randconfig-x018-201744
x86_64 randconfig-x011-201744
x86_64 randconfig-x019-201744
x86_64 randconfig-x013-201744
x86_64 randconfig-x012-201744
x86_64 randconfig-x017-201744
x86_64 randconfig-x016-201744
x86_64 randconfig-x015-201744
x86_64 randconfig-x014-201744
x86_64 randconfig-x010-201744
arm   omap2plus_defconfig
armsa1100
arm  allmodconfig
arm   samsung
armmvebu_v7_defconfig
arm  ixp4xx_defconfig
arm   imx_v6_v7_defconfig
arm64allmodconfig
arm   tegra_defconfig
arm  arm5
arm64alldefconfig
armsh
arm arm67
i386 randconfig-a0-201744
i386 randconfig-a1-201744
frv defconfig
mn10300 asb2364_defconfig
openriscor1ksim_defconfig
tile tilegx_defconfig
um i386_defconfig
um   x86_64_defconfig
cris etrax-100lx_v2_defconfig
blackfin  TCM-BF537_defconfig
blackfinBF561-EZKIT-SMP_defconfig
blackfinBF533-EZKIT_defconfig
blackfinBF526-EZBRD_defconfig
arm  nuc960_defconfig
arm palmz72_defconfig
mipsmalta_kvm_guest_defconfig
powerpc sequoia_defconfig
powerpc tqm8555_defconfig
i386  allnoconfig
i386defconfig
i386 alldefconfig
m68k   sun3_defconfig
m68k  multi_defconfig
m68k   m5475evb_defconfig
i386 randconfig-s0-201744
i386 randconfig-s1-201744
x86_64   allmodconfig
i386 allmodconfig
x86_64   randconfig-i0-201744
sparc   defconfig
sparc64   allnoconfig
sparc64 defconfig
i386 randconfig-i1-201744
i386 randconfig-i0-201744
m32roaks32r_defconfig
mips  lasat_defconfig
powerpc mpc8540_ads_defconfig
i386   tinyconfig
mips   jz4740
mips  malta_kvm_defconfig
mips 64r6el_defconfig
mips   32r2_defconfig
mips  

[PATCH] [RFC] media: camss-vfe: always initialize reg at vfe_set_xbar_cfg()

[Mauro Carvalho Chehab]

if output->wm_num is bigger than 1, the value for reg is
not initialized, as warned by smatch:
drivers/media/platform/qcom/camss-8x16/camss-vfe.c:633 
vfe_set_xbar_cfg() error: uninitialized symbol 'reg'.
drivers/media/platform/qcom/camss-8x16/camss-vfe.c:637 
vfe_set_xbar_cfg() error: uninitialized symbol 'reg'.

I didn't check the logic into its details, but there is at least
one point where wm_num is made equal to two. So, something
seem broken.

For now, I just reset it to zero, and added a FIXME. Hopefully,
the driver authors will know if this is OK, or if something else
is needed there.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/platform/qcom/camss-8x16/camss-vfe.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c 
b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
index b22d2dfcd3c2..388431f747fa 100644
--- a/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss-8x16/camss-vfe.c
@@ -622,6 +622,8 @@ static void vfe_set_xbar_cfg(struct vfe_device *vfe, struct 
vfe_output *output,
reg = VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_EN;
if (p == V4L2_PIX_FMT_NV12 || p == V4L2_PIX_FMT_NV16)
reg |= 
VFE_0_BUS_XBAR_CFG_x_M_PAIR_STREAM_SWAP_INTER_INTRA;
+   } else {
+   reg = 0;/* FIXME: is it the right value for i > 
1? */
}
 
if (output->wm_idx[i] % 2 == 1)
-- 
2.13.6

Re: [PATCH] media: rc: remove ir-rx51 in favour of generic pwm-ir-tx

[Pali Rohár]

On Wednesday 01 November 2017 11:55:33 Sean Young wrote:
> The ir-rx51 is a pwm-based TX driver specific to the n900. This can be
> handled entirely by the generic pwm-ir-tx driver.
> 
> Note that the suspend code in the ir-rx51 driver is unnecessary, since
> during transmit, the current process is not in interruptable sleep. The
> process is not put to sleep until the transmit completes.

Hello, have you tested this patch that IR transmitter is still working
fine on the real Nokia N900 device?

> Cc: Ivaylo Dimitrov 
> Cc: Pali Rohár 
> Cc: Pavel Machek 
> Cc: Timo Kokkonen 
> Signed-off-by: Sean Young 
> ---
>  arch/arm/boot/dts/omap3-n900.dts |   2 +-
>  drivers/media/rc/Kconfig |  10 --
>  drivers/media/rc/Makefile|   1 -
>  drivers/media/rc/ir-rx51.c   | 316 
> ---
>  4 files changed, 1 insertion(+), 328 deletions(-)
>  delete mode 100644 drivers/media/rc/ir-rx51.c
> 
> diff --git a/arch/arm/boot/dts/omap3-n900.dts 
> b/arch/arm/boot/dts/omap3-n900.dts
> index 4acd32a1c4ef..fccd2b119c0a 100644
> --- a/arch/arm/boot/dts/omap3-n900.dts
> +++ b/arch/arm/boot/dts/omap3-n900.dts
> @@ -152,7 +152,7 @@
>   };
>  
>   ir: n900-ir {
> - compatible = "nokia,n900-ir";
> + compatible = "pwm-ir-tx";
>   pwms = < 0 26316 0>; /* 38000 Hz */
>   };
>  
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index 0f863822889e..354ee5224758 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -353,16 +353,6 @@ config IR_TTUSBIR
>  To compile this driver as a module, choose M here: the module will
>  be called ttusbir.
>  
> -config IR_RX51
> - tristate "Nokia N900 IR transmitter diode"
> - depends on (OMAP_DM_TIMER && PWM_OMAP_DMTIMER && ARCH_OMAP2PLUS || 
> COMPILE_TEST) && RC_CORE
> - ---help---
> -Say Y or M here if you want to enable support for the IR
> -transmitter diode built in the Nokia N900 (RX51) device.
> -
> -The driver uses omap DM timers for generating the carrier
> -wave and pulses.
> -
>  source "drivers/media/rc/img-ir/Kconfig"
>  
>  config RC_LOOPBACK
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index a1ef86767aef..59b99a2dc2d4 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -25,7 +25,6 @@ obj-$(CONFIG_IR_MESON) += meson-ir.o
>  obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
>  obj-$(CONFIG_IR_ENE) += ene_ir.o
>  obj-$(CONFIG_IR_REDRAT3) += redrat3.o
> -obj-$(CONFIG_IR_RX51) += ir-rx51.o
>  obj-$(CONFIG_IR_SPI) += ir-spi.o
>  obj-$(CONFIG_IR_STREAMZAP) += streamzap.o
>  obj-$(CONFIG_IR_WINBOND_CIR) += winbond-cir.o
> diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
> deleted file mode 100644
> index 49265f02e772..
> --- a/drivers/media/rc/ir-rx51.c
> +++ /dev/null
> @@ -1,316 +0,0 @@
> -/*
> - *  Copyright (C) 2008 Nokia Corporation
> - *
> - *  Based on lirc_serial.c
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; either version 2 of the License, or
> - *  (at your option) any later version.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - */
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#include 
> -#include 
> -
> -#define WBUF_LEN 256
> -
> -struct ir_rx51 {
> - struct rc_dev *rcdev;
> - struct pwm_device *pwm;
> - struct hrtimer timer;
> - struct device*dev;
> - struct ir_rx51_platform_data *pdata;
> - wait_queue_head_t wqueue;
> -
> - unsigned intfreq;   /* carrier frequency */
> - unsigned intduty_cycle; /* carrier duty cycle */
> - int wbuf[WBUF_LEN];
> - int wbuf_index;
> - unsigned long   device_is_open;
> -};
> -
> -static inline void ir_rx51_on(struct ir_rx51 *ir_rx51)
> -{
> - pwm_enable(ir_rx51->pwm);
> -}
> -
> -static inline void ir_rx51_off(struct ir_rx51 *ir_rx51)
> -{
> - pwm_disable(ir_rx51->pwm);
> -}
> -
> -static int init_timing_params(struct ir_rx51 *ir_rx51)
> -{
> - struct pwm_device *pwm = ir_rx51->pwm;
> - int duty, period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, ir_rx51->freq);
> -
> - duty = DIV_ROUND_CLOSEST(ir_rx51->duty_cycle * period, 100);
> -
> - pwm_config(pwm, duty, period);
> -
> - return 0;
> -}
> -
> -static enum hrtimer_restart ir_rx51_timer_cb(struct hrtimer *timer)
> -{
> - struct ir_rx51 *ir_rx51 = container_of(timer, struct ir_rx51, 

[PATCH] media: rc: remove ir-rx51 in favour of generic pwm-ir-tx

[Sean Young]

The ir-rx51 is a pwm-based TX driver specific to the n900. This can be
handled entirely by the generic pwm-ir-tx driver.

Note that the suspend code in the ir-rx51 driver is unnecessary, since
during transmit, the current process is not in interruptable sleep. The
process is not put to sleep until the transmit completes.

Cc: Ivaylo Dimitrov 
Cc: Pali Rohár 
Cc: Pavel Machek 
Cc: Timo Kokkonen 
Signed-off-by: Sean Young 
---
 arch/arm/boot/dts/omap3-n900.dts |   2 +-
 drivers/media/rc/Kconfig |  10 --
 drivers/media/rc/Makefile|   1 -
 drivers/media/rc/ir-rx51.c   | 316 ---
 4 files changed, 1 insertion(+), 328 deletions(-)
 delete mode 100644 drivers/media/rc/ir-rx51.c

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 4acd32a1c4ef..fccd2b119c0a 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -152,7 +152,7 @@
};
 
ir: n900-ir {
-   compatible = "nokia,n900-ir";
+   compatible = "pwm-ir-tx";
pwms = < 0 26316 0>; /* 38000 Hz */
};
 
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 0f863822889e..354ee5224758 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -353,16 +353,6 @@ config IR_TTUSBIR
   To compile this driver as a module, choose M here: the module will
   be called ttusbir.
 
-config IR_RX51
-   tristate "Nokia N900 IR transmitter diode"
-   depends on (OMAP_DM_TIMER && PWM_OMAP_DMTIMER && ARCH_OMAP2PLUS || 
COMPILE_TEST) && RC_CORE
-   ---help---
-  Say Y or M here if you want to enable support for the IR
-  transmitter diode built in the Nokia N900 (RX51) device.
-
-  The driver uses omap DM timers for generating the carrier
-  wave and pulses.
-
 source "drivers/media/rc/img-ir/Kconfig"
 
 config RC_LOOPBACK
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index a1ef86767aef..59b99a2dc2d4 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -25,7 +25,6 @@ obj-$(CONFIG_IR_MESON) += meson-ir.o
 obj-$(CONFIG_IR_NUVOTON) += nuvoton-cir.o
 obj-$(CONFIG_IR_ENE) += ene_ir.o
 obj-$(CONFIG_IR_REDRAT3) += redrat3.o
-obj-$(CONFIG_IR_RX51) += ir-rx51.o
 obj-$(CONFIG_IR_SPI) += ir-spi.o
 obj-$(CONFIG_IR_STREAMZAP) += streamzap.o
 obj-$(CONFIG_IR_WINBOND_CIR) += winbond-cir.o
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
deleted file mode 100644
index 49265f02e772..
--- a/drivers/media/rc/ir-rx51.c
+++ /dev/null
@@ -1,316 +0,0 @@
-/*
- *  Copyright (C) 2008 Nokia Corporation
- *
- *  Based on lirc_serial.c
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- */
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-#include 
-
-#define WBUF_LEN 256
-
-struct ir_rx51 {
-   struct rc_dev *rcdev;
-   struct pwm_device *pwm;
-   struct hrtimer timer;
-   struct device*dev;
-   struct ir_rx51_platform_data *pdata;
-   wait_queue_head_t wqueue;
-
-   unsigned intfreq;   /* carrier frequency */
-   unsigned intduty_cycle; /* carrier duty cycle */
-   int wbuf[WBUF_LEN];
-   int wbuf_index;
-   unsigned long   device_is_open;
-};
-
-static inline void ir_rx51_on(struct ir_rx51 *ir_rx51)
-{
-   pwm_enable(ir_rx51->pwm);
-}
-
-static inline void ir_rx51_off(struct ir_rx51 *ir_rx51)
-{
-   pwm_disable(ir_rx51->pwm);
-}
-
-static int init_timing_params(struct ir_rx51 *ir_rx51)
-{
-   struct pwm_device *pwm = ir_rx51->pwm;
-   int duty, period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, ir_rx51->freq);
-
-   duty = DIV_ROUND_CLOSEST(ir_rx51->duty_cycle * period, 100);
-
-   pwm_config(pwm, duty, period);
-
-   return 0;
-}
-
-static enum hrtimer_restart ir_rx51_timer_cb(struct hrtimer *timer)
-{
-   struct ir_rx51 *ir_rx51 = container_of(timer, struct ir_rx51, timer);
-   ktime_t now;
-
-   if (ir_rx51->wbuf_index < 0) {
-   dev_err_ratelimited(ir_rx51->dev,
-   "BUG wbuf_index has value of %i\n",
-   ir_rx51->wbuf_index);
-   goto end;
-   }
-
-   /*
-* If we happen to hit an odd latency spike, loop through the
-* 

[PATCH 1/1] imx274: Fix error handling, add MAINTAINERS entry

[Sakari Ailus]

Add the missing MAINTAINERS entry for imx274, fix error handling in driver
probe and unregister the correct control handler in driver remove.

Signed-off-by: Sakari Ailus 
---
The earlier version of the imx274 patchset got merged, this is the diff
between what was merged and what was intended.

 MAINTAINERS| 8 
 drivers/media/i2c/imx274.c | 5 ++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index adbf69306e9e..a50dc70ae30b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12494,6 +12494,14 @@ S: Maintained
 F: drivers/ssb/
 F: include/linux/ssb/
 
+SONY IMX274 SENSOR DRIVER
+M: Leon Luo 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/imx274.c
+F: Documentation/devicetree/bindings/media/i2c/imx274.txt
+
 SONY MEMORYSTICK CARD SUPPORT
 M: Alex Dubov 
 W: http://tifmxx.berlios.de/
diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
index ab6a5f31da74..737dbf59a0d2 100644
--- a/drivers/media/i2c/imx274.c
+++ b/drivers/media/i2c/imx274.c
@@ -1770,8 +1770,7 @@ static int imx274_probe(struct i2c_client *client,
return 0;
 
 err_ctrls:
-   v4l2_async_unregister_subdev(sd);
-   v4l2_ctrl_handler_free(sd->ctrl_handler);
+   v4l2_ctrl_handler_free(>ctrls.handler);
 err_me:
media_entity_cleanup(>entity);
 err_regmap:
@@ -1788,7 +1787,7 @@ static int imx274_remove(struct i2c_client *client)
imx274_write_table(imx274, mode_table[IMX274_MODE_STOP_STREAM]);
 
v4l2_async_unregister_subdev(sd);
-   v4l2_ctrl_handler_free(sd->ctrl_handler);
+   v4l2_ctrl_handler_free(>ctrls.handler);
media_entity_cleanup(>entity);
mutex_destroy(>lock);
return 0;
-- 
2.11.0

Re: [PATCH] media: v4l2-fwnode: use the cached value instead of getting again

[Sebastian Reichel]

Hi,

On Tue, Oct 31, 2017 at 02:22:59PM -0400, Mauro Carvalho Chehab wrote:
> There is a get/put operation in order to get firmware is_available
> data there at the __v4l2_async_notifier_parse_fwnode_endpoints()
> function. However, instead of using it, the code just reads again
> without the lock. That's probably a mistake, as a similar code on
> another function use the cached value.
> 
> This solves this smatch warning:
> 
> drivers/media/v4l2-core/v4l2-fwnode.c:453:8: warning: variable 'is_available' 
> set but not used [-Wunused-but-set-variable]
>bool is_available;
> ^~~~
> 
> Fixes: 9ca465312132 ("media: v4l: fwnode: Support generic parsing of graph 
> endpoints in a device")
> Cc: Sakari Ailus 
> Signed-off-by: Mauro Carvalho Chehab 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  drivers/media/v4l2-core/v4l2-fwnode.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 3b9c6afb49a3..681b192420d9 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -455,8 +455,7 @@ static int __v4l2_async_notifier_parse_fwnode_endpoints(
>   dev_fwnode = fwnode_graph_get_port_parent(fwnode);
>   is_available = fwnode_device_is_available(dev_fwnode);
>   fwnode_handle_put(dev_fwnode);
> -
> - if (!fwnode_device_is_available(dev_fwnode))
> + if (!is_available)
>   continue;
>  
>   if (WARN_ON(notifier->num_subdevs >= notifier->max_subdevs)) {
> -- 
> 2.13.6
> 


signature.asc
Description: PGP signature

Re: [PATCH 1/7] media: atomisp: fix ident for assert/return

[Mauro Carvalho Chehab]

Em Wed, 01 Nov 2017 10:16:57 +0100
Philipp Zabel  escreveu:

> Hi Mauro,
> 
> On Tue, 2017-10-31 at 12:04 -0400, Mauro Carvalho Chehab wrote:
> > On lots of places, assert/return are starting at the first
> > column, causing indentation issues, as complained by spatch:
> > 
> > drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/irq_private.h:32
> >  irq_reg_store() warn: inconsistent indenting
> > 
> > Used this small script to fix such occurrences:
> > 
> > for i in $(git grep -l -E "^(assert|return)" drivers/staging/media/); do 
> > perl -ne 's/^(assert|return)/\t$1/; print $_' <$i >a && mv a $i; done  
> 
> This also catches labels that start with "return". Adding some
> whitespace to the regular expression may avoid these false positives.

> > -return_bufs:
> > +   return_bufs:
> > spin_lock_irqsave(>q_lock, flags);
> > list_for_each_entry_safe(buf, tmp, >ready_q, list) {
> > list_del(>list);  
> 
> This label should stay at the first column.

Good catch! I was not expecting a label named "return_foo" :-)

I added a \b in order to detect word bound:

for i in $(git grep -l -E "^(assert|return)" drivers/staging/media/); do perl 
-ne 's/^(assert|return)\b/\t$1/; print $_' <$i >a && mv a $i; done

That fixed the regex and it didn't catch the return_bufs anymore.

New version enclosed.

Thanks,
Mauro

[PATCH] media: atomisp: fix ident for assert/return

On lots of places, assert/return are starting at the first
column, causing indentation issues, as complained by spatch:

drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/irq_private.h:32
 irq_reg_store() warn: inconsistent indenting

Used this small script to fix such occurrences:

for i in $(git grep -l -E "^(assert|return)" drivers/staging/media/); do perl 
-ne 's/^(assert|return)\b/\t$1/; print $_' <$i >a && mv a $i; done

Signed-off-by: Mauro Carvalho Chehab 

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/camera/util/src/util.c 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/camera/util/src/util.c
index 08f486e20a65..54193789a809 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/camera/util/src/util.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/camera/util/src/util.c
@@ -111,7 +111,7 @@ unsigned int ia_css_util_input_format_bpp(
break;
 
}
-return rval;
+   return rval;
 }
 
 enum ia_css_err ia_css_util_check_vf_info(
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/event_fifo_private.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/event_fifo_private.h
index 9d3a29696094..bcfb734c2ed3 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/event_fifo_private.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/event_fifo_private.h
@@ -28,7 +28,7 @@ STORAGE_CLASS_EVENT_C void event_wait_for(const event_ID_t ID)
assert(ID < N_EVENT_ID);
assert(event_source_addr[ID] != ((hrt_address)-1));
(void)ia_css_device_load_uint32(event_source_addr[ID]);
-return;
+   return;
 }
 
 STORAGE_CLASS_EVENT_C void cnd_event_wait_for(const event_ID_t ID,
diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/fifo_monitor_private.h
 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/fifo_monitor_private.h
index 618b2f7e9c75..d58cd7d1828d 100644
--- 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/fifo_monitor_private.h
+++ 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/fifo_monitor_private.h
@@ -33,26 +33,26 @@ STORAGE_CLASS_FIFO_MONITOR_C void fifo_switch_set(
const fifo_switch_t switch_id,
const hrt_data  sel)
 {
-assert(ID == FIFO_MONITOR0_ID);
-assert(FIFO_MONITOR_BASE[ID] != (hrt_address)-1);
-assert(switch_id < N_FIFO_SWITCH);
+   assert(ID == FIFO_MONITOR0_ID);
+   assert(FIFO_MONITOR_BASE[ID] != (hrt_address)-1);
+   assert(switch_id < N_FIFO_SWITCH);
(void)ID;
 
gp_device_reg_store(GP_DEVICE0_ID, FIFO_SWITCH_ADDR[switch_id], sel);
 
-return;
+   return;
 }
 
 STORAGE_CLASS_FIFO_MONITOR_C hrt_data fifo_switch_get(
const fifo_monitor_ID_t ID,
const fifo_switch_t switch_id)
 {
-assert(ID == FIFO_MONITOR0_ID);
-assert(FIFO_MONITOR_BASE[ID] != (hrt_address)-1);
-assert(switch_id < N_FIFO_SWITCH);
+   assert(ID == FIFO_MONITOR0_ID);
+   assert(FIFO_MONITOR_BASE[ID] != (hrt_address)-1);
+   assert(switch_id < N_FIFO_SWITCH);
(void)ID;
 
-return gp_device_reg_load(GP_DEVICE0_ID, FIFO_SWITCH_ADDR[switch_id]);
+   return gp_device_reg_load(GP_DEVICE0_ID, FIFO_SWITCH_ADDR[switch_id]);
 }
 
 
@@ -61,19 +61,19 @@ 

Re: [PATCH 1/7] media: atomisp: fix ident for assert/return

[Philipp Zabel]

Hi Mauro,

On Tue, 2017-10-31 at 12:04 -0400, Mauro Carvalho Chehab wrote:
> On lots of places, assert/return are starting at the first
> column, causing indentation issues, as complained by spatch:
> 
> drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/irq_private.h:32
>  irq_reg_store() warn: inconsistent indenting
> 
> Used this small script to fix such occurrences:
> 
> for i in $(git grep -l -E "^(assert|return)" drivers/staging/media/); do perl 
> -ne 's/^(assert|return)/\t$1/; print $_' <$i >a && mv a $i; done

This also catches labels that start with "return". Adding some
whitespace to the regular expression may avoid these false positives.

> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  .../pci/atomisp2/css2400/camera/util/src/util.c|  2 +-
>  .../hive_isp_css_common/host/event_fifo_private.h  |  2 +-
>  .../host/fifo_monitor_private.h| 28 +-
>  .../css2400/hive_isp_css_common/host/gdc.c |  8 +--
>  .../css2400/hive_isp_css_common/host/gp_device.c   |  2 +-
>  .../hive_isp_css_common/host/gp_device_private.h   | 16 +++---
>  .../hive_isp_css_common/host/gpio_private.h|  4 +-
>  .../hive_isp_css_common/host/hmem_private.h|  4 +-
>  .../host/input_formatter_private.h | 16 +++---
>  .../hive_isp_css_common/host/input_system.c| 28 +-
>  .../host/input_system_private.h| 64 
> +++---
>  .../css2400/hive_isp_css_common/host/irq.c | 30 +-
>  .../css2400/hive_isp_css_common/host/irq_private.h | 12 ++--
>  .../css2400/hive_isp_css_common/host/isp.c |  4 +-
>  .../css2400/hive_isp_css_common/host/mmu.c |  6 +-
>  .../css2400/hive_isp_css_common/host/mmu_private.h | 12 ++--
>  .../css2400/hive_isp_css_common/host/sp_private.h  | 60 ++--
>  .../atomisp/pci/atomisp2/css2400/sh_css_hrt.c  |  2 +-
>  drivers/staging/media/imx/imx-media-capture.c  |  2 +-
[...]
> diff --git a/drivers/staging/media/imx/imx-media-capture.c 
> b/drivers/staging/media/imx/imx-media-capture.c
> index ea145bafb880..149f0e1753a1 100644
> --- a/drivers/staging/media/imx/imx-media-capture.c
> +++ b/drivers/staging/media/imx/imx-media-capture.c
> @@ -463,7 +463,7 @@ static int capture_start_streaming(struct vb2_queue *vq, 
> unsigned int count)
>  
>   return 0;
>  
> -return_bufs:
> + return_bufs:
>   spin_lock_irqsave(>q_lock, flags);
>   list_for_each_entry_safe(buf, tmp, >ready_q, list) {
>   list_del(>list);

This label should stay at the first column.

regards
Philipp

Re: Nokia N9: fun with camera

[Pavel Machek]

Hi!

> Sakari, I am actually playing with N9 camera, not N950. That comes
> next.
> 
> And the clock error I mentioned ... seems to be
> -EPROBE_DEFER. So... not an issue.

Hmm, and with similar config, I got N950 to work. ... which should
give me enough clues to get N9 to work. I guess I forgot to reset the
pipeline between the tries, or something.

For the record, this got me some data on n950:

 m.media_ctl( [ '-f', '"OMAP3 ISP CSI2a":0 [fmt:%s/%dx%d]' % (m.fmt, m.cap_x, 
m.cap_y) ] )
 m.media_ctl( [ '-l', '"OMAP3 ISP CSI2a":1 -> "OMAP3 ISP CSI2a output":0[1]' ] )

 # WORKS
 # pavel@n900:~/g/tui/camera$ sudo /my/tui/yavta/yavta
 # --capture=8 --skip 0 --format SGRBG10 --size 4272x3016 /dev/video1 
--file=/tmp/delme#

...ouch. It only worked twice :-(. Either driver gets confused by my
attempts, or it relied on some other initialization code. Strange.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature