cron job: media_tree daily build: ERRORS

2017-06-29 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Fri Jun 30 05:00:20 CEST 2017
media-tree git hash:2748e76ddb2967c4030171342ebdd3faa6a5e8e8
media_build git hash:   bf38fb5438c3cfd90a7496e951c6902111b77587
v4l-utils git hash: 2dbbd35740180feaacd5c968a86687ba1bd6c9bb
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.9.0-164

linux-git-arm-at91: WARNINGS
linux-git-arm-davinci: WARNINGS
linux-git-arm-multi: WARNINGS
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: WARNINGS
linux-2.6.36.4-i686: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: OK
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: WARNINGS
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: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12-rc1-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: 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: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12-rc1-x86_64: WARNINGS
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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


Re: [PATCH RFC 2/2] dt-bindings: add binding documentation for Allwinner CSI

2017-06-29 Thread Chen-Yu Tsai
On Fri, Jun 30, 2017 at 5:19 AM, Rob Herring  wrote:
> On Tue, Jun 27, 2017 at 07:07:34PM +0800, Yong Deng wrote:
>> Add binding documentation for Allwinner CSI.
>
> For the subject:
>
> dt-bindings: media: Add Allwinner Camera Sensor Interface (CSI)
>
> "binding documentation" is redundant.
>
>>
>> Signed-off-by: Yong Deng 
>> ---
>>  .../devicetree/bindings/media/sunxi-csi.txt| 51 
>> ++
>>  1 file changed, 51 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/sunxi-csi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/sunxi-csi.txt 
>> b/Documentation/devicetree/bindings/media/sunxi-csi.txt
>> new file mode 100644
>> index 000..770be0e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/sunxi-csi.txt
>> @@ -0,0 +1,51 @@
>> +Allwinner V3s Camera Sensor Interface
>> +--
>> +
>> +Required properties:
>> +  - compatible: value must be "allwinner,sun8i-v3s-csi"
>> +  - reg: base address and size of the memory-mapped region.
>> +  - interrupts: interrupt associated to this IP
>> +  - clocks: phandles to the clocks feeding the CSI
>> +* ahb: the CSI interface clock
>> +* mod: the CSI module clock
>> +* ram: the CSI DRAM clock
>> +  - clock-names: the clock names mentioned above
>> +  - resets: phandles to the reset line driving the CSI
>> +
>> +- ports: A ports node with endpoint definitions as defined in
>> +  Documentation/devicetree/bindings/media/video-interfaces.txt. The
>> +  first port should be the input endpoints, the second one the outputs
>
> Is there more than one endpoint for each port? If so, need to define
> that numbering too.

It is possible to have multiple camera sensors connected to the same
bus. Think front and back cameras on a cell phone or tablet.

I don't think any kind of numbering makes much sense though. The
system is free to use just one sensor at a time, or use many with
some time multiplexing scheme. What might matter to the end user
is where the camera is placed. But using the position or orientation
as a numbering scheme might not work well either. Someone may end
up using two sensors with the same orientation for stereoscopic
vision.

>
>> +
>> +Example:
>> +
>> + csi1: csi@01cb4000 {
>> + compatible = "allwinner,sun8i-v3s-csi";
>> + reg = <0x01cb4000 0x1000>;

Yong, the address range size is 0x4000, including the CCI (I2C)
controller at offset 0x3000. You should also consider this in
the device tree binding, and the driver.

ChenYu

>> + interrupts = ;
>> + clocks = < CLK_BUS_CSI>,
>> +  < CLK_CSI1_SCLK>,
>> +  < CLK_DRAM_CSI>;
>> + clock-names = "ahb", "mod", "ram";
>> + resets = < RST_BUS_CSI>;
>> +
>> + port {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + /* Parallel bus endpoint */
>> + csi1_0: endpoint@0 {
>> + reg = <0>;
>
> Don't need this and everything associated with it for a single endpoint.
>
>> + remote = <_1>;
>> + bus-width = <16>;
>> + data-shift = <0>;
>> +
>> + /* If hsync-active/vsync-active are missing,
>> +embedded BT.656 sync is used */
>> + hsync-active = <0>; /* Active low */
>> + vsync-active = <0>; /* Active low */
>> + data-active = <1>;  /* Active high */
>> + pclk-sample = <1>;  /* Rising */
>> + };
>> + };
>> + };
>> +
>> --
>> 1.8.3.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 04/19] media: camss: Add CSIPHY files

2017-06-29 Thread Sakari Ailus
Hi Todor,

On Thu, Jun 29, 2017 at 07:36:47PM +0300, Todor Tomov wrote:
> >> +/*
> >> + * csiphy_link_setup - Setup CSIPHY connections
> >> + * @entity: Pointer to media entity structure
> >> + * @local: Pointer to local pad
> >> + * @remote: Pointer to remote pad
> >> + * @flags: Link flags
> >> + *
> >> + * Rreturn 0 on success
> >> + */
> >> +static int csiphy_link_setup(struct media_entity *entity,
> >> +   const struct media_pad *local,
> >> +   const struct media_pad *remote, u32 flags)
> >> +{
> >> +  if ((local->flags & MEDIA_PAD_FL_SOURCE) &&
> >> +  (flags & MEDIA_LNK_FL_ENABLED)) {
> >> +  struct v4l2_subdev *sd;
> >> +  struct csiphy_device *csiphy;
> >> +  struct csid_device *csid;
> >> +
> >> +  if (media_entity_remote_pad((struct media_pad *)local))
> > 
> > This is ugly.
> > 
> > What do you intend to find with media_entity_remote_pad()? The pad flags
> > haven't been assigned to the pad yet, so media_entity_remote_pad() could
> > give you something else than remote.
> 
> This is an attempt to check whether the pad is already linked - to refuse
> a second active connection from the same src pad. As far as I can say, it
> was a successful attempt. Do you see any problem with it?

Ah. So you have multiple links here only one of which may be active?

I guess you can well use media_entity_remote_pad(), but then
media_entity_remote_pad() argument needs to be made const. Feel free to
spin a patch. I don't think it'd have further implications elsewhere.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk


[PATCH] staging: media: atomisp: Remove unnecessary return statement in void function

2017-06-29 Thread Amitoj Kaur Chawla
Return statement at the end of a void function is useless.

The Coccinelle semantic patch used to make this change is as follows:
//
@@
identifier f;
expression e;
@@
void f(...) {
<...
- return
  e;
  ...>
  }
//

Signed-off-by: Amitoj Kaur Chawla 
---
 drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c 
b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
index 5729539..22c0342 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm.c
@@ -636,7 +636,7 @@ void hmm_vunmap(ia_css_ptr virt)
return;
}
 
-   return hmm_bo_vunmap(bo);
+   hmm_bo_vunmap(bo);
 }
 
 int hmm_pool_register(unsigned int pool_size,
-- 
2.7.4



Re: [PATCH v3] media: platform: rcar_imr: add IMR-LSX3 support

2017-06-29 Thread Rob Herring
On Wed, Jun 28, 2017 at 10:56:23PM +0300, Sergei Shtylyov wrote:
> Add support for the image renderer light SRAM extended 3 (IMR-LSX3) found
> only in the R-Car V2H (R8A7792) SoC.  It differs  from IMR-LX4 in that it
> supports only planar video formats but can use the video capture data for
> the textures.
> 
> Signed-off-by: Sergei Shtylyov 
> 
> ---
> This patch  is against the 'media_tree.git' repo's 'master' branch plus the
> latest version of the Renesas IMR driver...
> 
> Changes in version 3:
> - fixed compilation errors, resolved rejects, refreshed the patch atop of the
>   IMR driver patch (version 6).
> 
> Changes in version 2:
> - renamed *enum* 'imr_gen' to 'imr_type' and the *struct* field of this type
>   from 'gen' to 'type';
> - rename *struct* 'imr_type' to 'imr_info' and the fields/variables of this 
> type
>   from 'type' to 'info';
> - added comments to IMR-LX4 only CMRCR2 bits;
> - added IMR type check to the WTS instruction writing to CMRCCR2.
> 
>  Documentation/devicetree/bindings/media/rcar_imr.txt |   11 +-

You missed my ack on v2.

>  drivers/media/platform/rcar_imr.c|  101 
> +++
>  2 files changed, 92 insertions(+), 20 deletions(-)


Re: [PATCH RFC 2/2] dt-bindings: add binding documentation for Allwinner CSI

2017-06-29 Thread Rob Herring
On Tue, Jun 27, 2017 at 07:07:34PM +0800, Yong Deng wrote:
> Add binding documentation for Allwinner CSI.

For the subject:

dt-bindings: media: Add Allwinner Camera Sensor Interface (CSI)

"binding documentation" is redundant.

> 
> Signed-off-by: Yong Deng 
> ---
>  .../devicetree/bindings/media/sunxi-csi.txt| 51 
> ++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/sunxi-csi.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/sunxi-csi.txt 
> b/Documentation/devicetree/bindings/media/sunxi-csi.txt
> new file mode 100644
> index 000..770be0e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/sunxi-csi.txt
> @@ -0,0 +1,51 @@
> +Allwinner V3s Camera Sensor Interface
> +--
> +
> +Required properties:
> +  - compatible: value must be "allwinner,sun8i-v3s-csi"
> +  - reg: base address and size of the memory-mapped region.
> +  - interrupts: interrupt associated to this IP
> +  - clocks: phandles to the clocks feeding the CSI
> +* ahb: the CSI interface clock
> +* mod: the CSI module clock
> +* ram: the CSI DRAM clock
> +  - clock-names: the clock names mentioned above
> +  - resets: phandles to the reset line driving the CSI
> +
> +- ports: A ports node with endpoint definitions as defined in
> +  Documentation/devicetree/bindings/media/video-interfaces.txt. The
> +  first port should be the input endpoints, the second one the outputs

Is there more than one endpoint for each port? If so, need to define 
that numbering too.

> +
> +Example:
> +
> + csi1: csi@01cb4000 {
> + compatible = "allwinner,sun8i-v3s-csi";
> + reg = <0x01cb4000 0x1000>;
> + interrupts = ;
> + clocks = < CLK_BUS_CSI>,
> +  < CLK_CSI1_SCLK>,
> +  < CLK_DRAM_CSI>;
> + clock-names = "ahb", "mod", "ram";
> + resets = < RST_BUS_CSI>;
> +
> + port {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /* Parallel bus endpoint */
> + csi1_0: endpoint@0 {
> + reg = <0>;

Don't need this and everything associated with it for a single endpoint. 

> + remote = <_1>;
> + bus-width = <16>;
> + data-shift = <0>;
> +
> + /* If hsync-active/vsync-active are missing,
> +embedded BT.656 sync is used */
> + hsync-active = <0>; /* Active low */
> + vsync-active = <0>; /* Active low */
> + data-active = <1>;  /* Active high */
> + pclk-sample = <1>;  /* Rising */
> + };
> + };
> + };
> +
> -- 
> 1.8.3.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/8] [media] s5p-jpeg: Various fixes and improvements

2017-06-29 Thread Jacek Anaszewski
Hi Thierry,

For the whole series:

Acked-by: Jacek Anaszewski 

Best regards,
Jacek Anaszewski

On 06/27/2017 06:08 PM, Thierry Escande wrote:
> Hi,
> 
> This series contains various fixes and improvements for the Samsung
> s5p-jpeg driver. Most of these patches come from the Chromium v3.8
> kernel tree.
> 
> In this v3:
> - Remove codec reset patch (Not needed based on documentation and no
>   use case described in original patch commit message).
> - Check for Exynos5420 variant in stream error handling patch.
> - Add use case for resolution change event support in commit message.
> - Move subsampling value decoding in a separate function.
> - Check Exynos variant for 4:1:1 subsampling support.
> 
> v2:
> - Remove IOMMU support patch (mapping now created automatically for
>   single JPEG CODEC device).
> - Remove "Change sclk_jpeg to 166MHz" patch (can be set through DT
>   properties).
> - Remove support for multi-planar APIs (Not needed).
> - Add comment regarding call to jpeg_bound_align_image() after qbuf.
> - Remove unrelated code from resolution change event support patch.
> 
> Thierry Escande (3):
>   [media] s5p-jpeg: Handle parsing error in s5p_jpeg_parse_hdr()
>   [media] s5p-jpeg: Don't use temporary structure in s5p_jpeg_buf_queue
>   [media] s5p-jpeg: Split s5p_jpeg_parse_hdr()
> 
> Tony K Nadackal (3):
>   [media] s5p-jpeg: Call jpeg_bound_align_image after qbuf
>   [media] s5p-jpeg: Correct WARN_ON statement for checking subsampling
>   [media] s5p-jpeg: Decode 4:1:1 chroma subsampling format
> 
> henryhsu (2):
>   [media] s5p-jpeg: Add support for resolution change event
>   [media] s5p-jpeg: Add stream error handling for Exynos5420
> 
>  drivers/media/platform/s5p-jpeg/jpeg-core.c | 186 
> +---
>  drivers/media/platform/s5p-jpeg/jpeg-core.h |   7 ++
>  2 files changed, 148 insertions(+), 45 deletions(-)
> 


Re: [PATCH v3 4/8] [media] s5p-jpeg: Don't use temporary structure in s5p_jpeg_buf_queue

2017-06-29 Thread Jacek Anaszewski
Hi Thierry,

On 06/27/2017 06:08 PM, Thierry Escande wrote:
> If s5p_jpeg_parse_hdr() fails to parse the JPEG header, the passed
> s5p_jpeg_q_data structure is not modify so there is no need to use a

s/modify/modified/

> temporary structure and the field-by-field copy can be avoided.
> 
> Signed-off-by: Thierry Escande 
> ---
>  drivers/media/platform/s5p-jpeg/jpeg-core.c | 23 ---
>  1 file changed, 4 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
> b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index df3e5ee..1769744 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -2500,9 +2500,9 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
>  
>   if (ctx->mode == S5P_JPEG_DECODE &&
>   vb->vb2_queue->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> - struct s5p_jpeg_q_data tmp, *q_data;
> + struct s5p_jpeg_q_data *q_data;
>  
> - ctx->hdr_parsed = s5p_jpeg_parse_hdr(,
> + ctx->hdr_parsed = s5p_jpeg_parse_hdr(>out_q,
>(unsigned long)vb2_plane_vaddr(vb, 0),
>min((unsigned long)ctx->out_q.size,
>vb2_get_plane_payload(vb, 0)), ctx);
> @@ -2511,24 +2511,9 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
>   return;
>   }
>  
> - q_data = >out_q;
> - q_data->w = tmp.w;
> - q_data->h = tmp.h;
> - q_data->sos = tmp.sos;
> - memcpy(q_data->dht.marker, tmp.dht.marker,
> -sizeof(tmp.dht.marker));
> - memcpy(q_data->dht.len, tmp.dht.len, sizeof(tmp.dht.len));
> - q_data->dht.n = tmp.dht.n;
> - memcpy(q_data->dqt.marker, tmp.dqt.marker,
> -sizeof(tmp.dqt.marker));
> - memcpy(q_data->dqt.len, tmp.dqt.len, sizeof(tmp.dqt.len));
> - q_data->dqt.n = tmp.dqt.n;
> - q_data->sof = tmp.sof;
> - q_data->sof_len = tmp.sof_len;
> -
>   q_data = >cap_q;
> - q_data->w = tmp.w;
> - q_data->h = tmp.h;
> + q_data->w = ctx->out_q.w;
> + q_data->h = ctx->out_q.h;
>  
>   /*
>* This call to jpeg_bound_align_image() takes care of width and
> 

-- 
Best regards,
Jacek Anaszewski


Re: [linux-media] How to handle independent CA devices

2017-06-29 Thread Jasmin J.
Hello!

It is now 6,5 years since this eMail ir reply to and a lot of things changed
since then.

As there is currently a lot of effort done to get the newest version of the DD
(Digital Devices) drivers into the Kernel, I want to bring up this topic again.
Yes, this eMail is long but it is necessary to explain all very detailed, so
all people can understand the new concepts.



a) First a description of the current situation:

> VDR already has mechanisms that allow independent handling of CAMs
> and receiving devices. Out of the box this currently only works for
> DVB devices that actually have a frontend and where the 'ca' device
> is under the same 'adapter' as the frontend.
This is still the case and it is good so.
What has been changed is ddbridge. Not in the current Kernel version, but in
the vendor version. Ralph implemented the ddbridge parameter adapter_alloc,
which allows to configure where the caX device is allocated. Moreover, he
implemented a sysFS node (ddbridge?/redirect) to control how the TS stream is
routed through ddbridge.

With both together you can implement all required combinations an application
needs. The Default is a complete independent creation of the caX devices into a
separate adapter$ directory. Also the TS stream is *not* routed through the CI
interface, but to the reused secX device (the DD version uses a new device node
ciX for that instead of secX).

In this setup, VDR doesn't see the caX device and therefore doesn't initialize
it. Which is perfectly OK, because it isn't possible to use it anyway.

> So, the bottom line is: I would appreciate an implementation where,
> given the configuration you described above, I could, e.g., tune using
> /dev/dvb/adapter0/frontend0, read the data stream from /dev/dvb/adapter0/dvr0
> as usual, communicate with the CAM through /dev/dvb/adapter2/ca0 and
> (which is the tricky part, I guess) "tell" the driver or some library
> function to "assign the CAM in /dev/dvb/adapter2/ca0 to the frontend|dvr
> in /dev/dvb/adapter0/frontend0|dvr0).
I guess Ralph implemented the sysFS node (ddbridge?/redirect) and
adapter_alloc because of this request from Klaus and of course that the DD
hardware is usable by any other software, too.

If ddbridge is started with the parameter adapter_alloc=3, then the driver
creates the caX and the secX(ciX) device in the same directory as the frontendX
and dvrX devices. Now VDR will recognize the caX device and initialize the CAM.
Additionally, the user needs to select which caX device shall be used for which
tuner via the mentioned sysFS node (ddbridge?/redirect). With this combination
the DD driver behaves like a DVB card which has the CI interface hard wired to
a tuner. It will route the TS stream through the CAM via the CI hardware before
it can be read out of the dvrX device.
Long time this was the only possibility to use the DD CI with a DD tuner.

> As for decrypting data from several frontends through one CAM: I don't
> see this happening in VDR. Pay tv channels repeat their stuff
In the meantime VDR is able do MTD (multi tuner decoding) by rewriting
the PIDs of the TS stream and feed it into the CAM (connected via a DD CI
hardware) with help of a plugin I wrote.

> However, VDR always assumes that the data to be recorded comes out of
> the 'dvr' device that's under the same adapter as the 'frontend'.
> So requiring that VDR would read from the frontend's 'dvr' device,
> write to the ca-adapter's 'sec' (or whatever) device, and finally read
> from that same 'sec' device again would be something I'd rather avoid.
VDR provides a plugin interface for a CI driver since version 2.1.7. With this
interface it is now possible to connect any CI hardware to VDR, if a plugin
does the caX/secX(ciX) device communication.
As mentioned above the DD driver default is creating the caX/secX(ciX) device
in a dedicated adapter$ directory and the TS stream is available at the dvrX
device in another adapter$ directory (possibly encrypted).

My VDR plugin will now search for adapter$ directories with only caX/secX(ciX)
devices and provide them to VDR. VDR itself will not initialize the CAM
interface, because the caX device is not in the same adapter$ directory as the
dvrX device, although it still searches for them. So VDR would still use the
caX device of a DVB card with a hard wired CI interface.

Moreover, VDR is now able to use the DD CI interface together with a DVB device
from another vendor. It is even possible to use the same CAM for several tuners
from different vendors.



b) Things to do:

>From all of the above we see, that a dedicated CI interface (independent from a
tuner) is a very useful piece of hardware.
Currently DD provides the Octopus CI, the DuoFlex CI (single) and the DuoFlex
CI (dual) (if I forget one, I apologize). The cxd2099 driver is used on the
DuoFlex CI (single) card only (AFAIK). The other cards are handled by ddbrigde
directly. Moreover, ddbridge provides is re-using the secX device (or

Re: Trying to use IR driver for my SoC

2017-06-29 Thread Sean Young
On Thu, Jun 29, 2017 at 09:12:48PM +0200, Mason wrote:
> On 29/06/2017 19:50, Sean Young wrote:
> 
> > On Thu, Jun 29, 2017 at 06:25:55PM +0200, Mason wrote:
> >
> >> $ ir-keytable -v -t
> >> Found device /sys/class/rc/rc0/
> >> Input sysfs node is /sys/class/rc/rc0/input0/
> >> Event sysfs node is /sys/class/rc/rc0/input0/event0/
> >> Parsing uevent /sys/class/rc/rc0/input0/event0/uevent
> >> /sys/class/rc/rc0/input0/event0/uevent uevent MAJOR=13
> >> /sys/class/rc/rc0/input0/event0/uevent uevent MINOR=64
> >> /sys/class/rc/rc0/input0/event0/uevent uevent DEVNAME=input/event0
> >> Parsing uevent /sys/class/rc/rc0/uevent
> >> /sys/class/rc/rc0/uevent uevent NAME=rc-empty
> >> input device is /dev/input/event0
> >> /sys/class/rc/rc0/protocols protocol rc-5 (disabled)
> >> /sys/class/rc/rc0/protocols protocol nec (disabled)
> >> /sys/class/rc/rc0/protocols protocol rc-6 (disabled)
> 
> I had overlooked this. Is it expected for these protocols
> to be marked as "disabled"?

Ah, good point, I forgot about that. :/

"ir-keytable -p all -t -v" should enable all protocols and test.

> >> Opening /dev/input/event0
> >> Input Protocol version: 0x00010001
> >> Testing events. Please, press CTRL-C to abort.
> >> ^C
> >>
> >> Is rc-empty perhaps not the right choice?
> > 
> > rc-empty means there is no mapping from scancode to keycode. When you
> > run "ir-keytable -v -t" you should at see scancodes when the driver
> > generates them with rc_keydown().
> 
> So the mapping can be done either in the kernel, or in
> user-space by the application consuming the scancodes,
> right?

That's right, although I do not know of any user-space application that does
this; scancodes are mostly useful for debugging.

> > From a cursory glance at the driver I can't see anything wrong.
> > 
> > The only thing that stands out is RC5_TIME_BASE. If that is the bit
> > length or shortest pulse/space? In the latter case it should be 888 usec.
> 
> Need to locate some docs.
> 
> > It might be worth trying nec, rc5 and rc6_0 and seeing if any of them 
> > decode.
> 
> What do you mean? How do I try them?

Well, presumably you're using a remote control for testing. It would be 
useful if you had remotes which could do all protocols that the IR 
receiver supports, so you can try all three of them.

Another way of doing this is using an IR transmitter and using ir-ctl to
send scancodes.


Sean


Re: [PATCH] [media] dvb-frontends/cxd2841er: require STATE_ACTIVE_* for agc readout

2017-06-29 Thread Abylay Ospan
Acked-by: Abylay Ospan 

2017-06-25 6:02 GMT-04:00 Daniel Scheller :
> From: Daniel Scheller 
>
> When the demod driver puts the demod into sleep or shutdown state and it's
> status is then polled e.g. via "dvb-fe-tool -m", i2c errors are printed
> to the kernel log. If the last delsys was DVB-T/T2:
>
>   cxd2841er: i2c wr failed=-5 addr=6c reg=00 len=1
>   cxd2841er: i2c rd failed=-5 addr=6c reg=26
>
> and if it was DVB-C:
>
>   cxd2841er: i2c wr failed=-5 addr=6c reg=00 len=1
>   cxd2841er: i2c rd failed=-5 addr=6c reg=49
>
> This happens when read_status unconditionally calls into the
> read_signal_strength() function which triggers the read_agc_gain_*()
> functions, where these registered are polled.
>
> This isn't a critical thing since when the demod is active again, no more
> such errors are logged, however this might make users suspecting defects.
>
> Fix this by requiring STATE_ACTIVE_* in priv->state. If it isn't in any
> active state, additionally set the strength scale to NOT_AVAILABLE.
>
> Signed-off-by: Daniel Scheller 
> ---
> V2/follow-up to https://patchwork.linuxtv.org/patch/42061/, changed as
> requested. Tested, working fine (ie. no "false" i2c failures).
>
>  drivers/media/dvb-frontends/cxd2841er.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/dvb-frontends/cxd2841er.c 
> b/drivers/media/dvb-frontends/cxd2841er.c
> index 08f67d60a7d9..12bff778c97f 100644
> --- a/drivers/media/dvb-frontends/cxd2841er.c
> +++ b/drivers/media/dvb-frontends/cxd2841er.c
> @@ -3279,7 +3279,10 @@ static int cxd2841er_get_frontend(struct dvb_frontend 
> *fe,
> else if (priv->state == STATE_ACTIVE_TC)
> cxd2841er_read_status_tc(fe, );
>
> -   cxd2841er_read_signal_strength(fe);
> +   if (priv->state == STATE_ACTIVE_TC || priv->state == STATE_ACTIVE_S)
> +   cxd2841er_read_signal_strength(fe);
> +   else
> +   p->strength.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
>
> if (status & FE_HAS_LOCK) {
> cxd2841er_read_snr(fe);
> --
> 2.13.0
>



-- 
Abylay Ospan,
NetUP Inc.
http://www.netup.tv


Re: Trying to use IR driver for my SoC

2017-06-29 Thread Mason
On 29/06/2017 19:50, Sean Young wrote:

> On Thu, Jun 29, 2017 at 06:25:55PM +0200, Mason wrote:
>
>> $ ir-keytable -v -t
>> Found device /sys/class/rc/rc0/
>> Input sysfs node is /sys/class/rc/rc0/input0/
>> Event sysfs node is /sys/class/rc/rc0/input0/event0/
>> Parsing uevent /sys/class/rc/rc0/input0/event0/uevent
>> /sys/class/rc/rc0/input0/event0/uevent uevent MAJOR=13
>> /sys/class/rc/rc0/input0/event0/uevent uevent MINOR=64
>> /sys/class/rc/rc0/input0/event0/uevent uevent DEVNAME=input/event0
>> Parsing uevent /sys/class/rc/rc0/uevent
>> /sys/class/rc/rc0/uevent uevent NAME=rc-empty
>> input device is /dev/input/event0
>> /sys/class/rc/rc0/protocols protocol rc-5 (disabled)
>> /sys/class/rc/rc0/protocols protocol nec (disabled)
>> /sys/class/rc/rc0/protocols protocol rc-6 (disabled)

I had overlooked this. Is it expected for these protocols
to be marked as "disabled"?

>> Opening /dev/input/event0
>> Input Protocol version: 0x00010001
>> Testing events. Please, press CTRL-C to abort.
>> ^C
>>
>> Is rc-empty perhaps not the right choice?
> 
> rc-empty means there is no mapping from scancode to keycode. When you
> run "ir-keytable -v -t" you should at see scancodes when the driver
> generates them with rc_keydown().

So the mapping can be done either in the kernel, or in
user-space by the application consuming the scancodes,
right?

> From a cursory glance at the driver I can't see anything wrong.
> 
> The only thing that stands out is RC5_TIME_BASE. If that is the bit
> length or shortest pulse/space? In the latter case it should be 888 usec.

Need to locate some docs.

> It might be worth trying nec, rc5 and rc6_0 and seeing if any of them decode.

What do you mean? How do I try them?

> Failing that some documentation would be great :)

I will try finding some.

Regards.


[PATCH] Added support for the TerraTec T1 DVB-T USB tuner [IT9135 chipset]

2017-06-29 Thread Nuno Henriques
Signed-off-by: Nuno Henriques 
---
 drivers/media/dvb-core/dvb-usb-ids.h  | 1 +
 drivers/media/usb/dvb-usb-v2/af9035.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/media/dvb-core/dvb-usb-ids.h 
b/drivers/media/dvb-core/dvb-usb-ids.h
index e200aa6f2d2f..5b6041d462bc 100644
--- a/drivers/media/dvb-core/dvb-usb-ids.h
+++ b/drivers/media/dvb-core/dvb-usb-ids.h
@@ -279,6 +279,7 @@
 #define USB_PID_TERRATEC_H70x10b4
 #define USB_PID_TERRATEC_H7_2  0x10a3
 #define USB_PID_TERRATEC_H7_3  0x10a5
+#define USB_PID_TERRATEC_T10x10ae
 #define USB_PID_TERRATEC_T30x10a0
 #define USB_PID_TERRATEC_T50x10a1
 #define USB_PID_NOXON_DAB_STICK0x00b3
diff --git a/drivers/media/usb/dvb-usb-v2/af9035.c 
b/drivers/media/usb/dvb-usb-v2/af9035.c
index 4df9486e19b9..ccf4a5c68877 100644
--- a/drivers/media/usb/dvb-usb-v2/af9035.c
+++ b/drivers/media/usb/dvb-usb-v2/af9035.c
@@ -2108,6 +2108,8 @@ static const struct usb_device_id af9035_id_table[] = {
{ DVB_USB_DEVICE(USB_VID_KWORLD_2, USB_PID_CTVDIGDUAL_V2,
_props, "Digital Dual TV Receiver CTVDIGDUAL_V2",
RC_MAP_IT913X_V1) },
+   { DVB_USB_DEVICE(USB_VID_TERRATEC, USB_PID_TERRATEC_T1,
+   _props, "TerraTec T1", RC_MAP_IT913X_V1) },
/* XXX: that same ID [0ccd:0099] is used by af9015 driver too */
{ DVB_USB_DEVICE(USB_VID_TERRATEC, 0x0099,
_props, "TerraTec Cinergy T Stick Dual RC (rev. 2)",
-- 
2.13.2



Re: Trying to use IR driver for my SoC

2017-06-29 Thread Sean Young
On Thu, Jun 29, 2017 at 06:25:55PM +0200, Mason wrote:
> On 29/06/2017 17:55, Sean Young wrote:
> 
> > On Thu, Jun 29, 2017 at 05:29:01PM +0200, Mason wrote:
> > 
> >> I'm trying to use an IR driver written for my SoC:
> >> https://github.com/mansr/linux-tangox/blob/master/drivers/media/rc/tangox-ir.c
> >>
> >> I added these options to my defconfig:
> >>
> >> +CONFIG_MEDIA_SUPPORT=y
> >> +CONFIG_MEDIA_RC_SUPPORT=y
> >> +CONFIG_RC_DEVICES=y
> >> +CONFIG_IR_TANGO=y
> >>
> >> (I don't think I need the RC decoders, because the HW is supposed
> >> to support HW decoding of NEC, RC5, RC6).
> > 
> > I haven't seen this driver before, what hardware is this for?
> 
> Sigma Designs tango3/tango4 (SMP86xx and SMP87xx)
> 
> >> These are the logs printed at boot:
> >>
> >> [1.827842] IR NEC protocol handler initialized
> >> [1.832407] IR RC5(x/sz) protocol handler initialized
> >> [1.837491] IR RC6 protocol handler initialized
> >> [1.842049] IR JVC protocol handler initialized
> >> [1.846606] IR Sony protocol handler initialized
> >> [1.851248] IR SANYO protocol handler initialized
> >> [1.855979] IR Sharp protocol handler initialized
> >> [1.860708] IR MCE Keyboard/mouse protocol handler initialized
> >> [1.866575] IR XMP protocol handler initialized
> >> [1.871232] tango-ir 10518.ir: SMP86xx IR decoder at 0x10518/0x105e0 
> >> IRQ 21
> >> [1.878241] Registered IR keymap rc-empty
> >> [1.882457] input: tango-ir as 
> >> /devices/platform/soc/10518.ir/rc/rc0/input0
> >> [1.889473] tango_ir_open
> >> [1.892105] rc rc0: tango-ir as /devices/platform/soc/10518.ir/rc/rc0
> >>
> >>
> >> I was naively expecting some kind of dev/input/event0 node
> >> I could cat to grab all the remote control key presses.
> >>
> >> But I don't see anything relevant in /dev
> > 
> > Do you have CONFIG_INPUT_EVDEV set? Is udev setup to create the devices?
> 
> I was indeed missing CONFIG_INPUT_EVDEV.
> 
> As for udev:
> [2.199642] udevd[960]: starting eudev-3.2.1
> 
> $ ls -l /dev/input/
> total 0
> drwxr-xr-x2 root root60 Jan  1 00:00 by-path
> crw-rw1 root input  13,  64 Jan  1 00:00 event0
> 
> But still no cookie:
> $ cat /dev/input/event0
> remains mute :-(
> 
> $ ir-keytable -v -t
> Found device /sys/class/rc/rc0/
> Input sysfs node is /sys/class/rc/rc0/input0/
> Event sysfs node is /sys/class/rc/rc0/input0/event0/
> Parsing uevent /sys/class/rc/rc0/input0/event0/uevent
> /sys/class/rc/rc0/input0/event0/uevent uevent MAJOR=13
> /sys/class/rc/rc0/input0/event0/uevent uevent MINOR=64
> /sys/class/rc/rc0/input0/event0/uevent uevent DEVNAME=input/event0
> Parsing uevent /sys/class/rc/rc0/uevent
> /sys/class/rc/rc0/uevent uevent NAME=rc-empty
> input device is /dev/input/event0
> /sys/class/rc/rc0/protocols protocol rc-5 (disabled)
> /sys/class/rc/rc0/protocols protocol nec (disabled)
> /sys/class/rc/rc0/protocols protocol rc-6 (disabled)
> Opening /dev/input/event0
> Input Protocol version: 0x00010001
> Testing events. Please, press CTRL-C to abort.
> ^C
> 
> Is rc-empty perhaps not the right choice?

rc-empty means there is no mapping from scancode to keycode. When you
run "ir-keytable -v -t" you should at see scancodes when the driver
generates them with rc_keydown().

> > By opening the /dev/input/event0 device, tango_ir_open() gets called which
> > presumably enables interrupts or IR decoding for the device. It's hard to
> > say without knowing anything about the soc.
> 
> Actually tango_ir_open() is called at boot, before any process
> has a chance to open /dev/input/event0
> 
> [1.926730] [] (tango_ir_open) from [] 
> (rc_open+0x44/0x6c)
> [1.933994] [] (rc_open) from [] 
> (input_open_device+0x74/0xac)
> [1.941610] [] (input_open_device) from [] 
> (kbd_connect+0x64/0x80)
> [1.949570] [] (kbd_connect) from [] 
> (input_attach_handler+0x1bc/0x1f4)
> [1.957965] [] (input_attach_handler) from [] 
> (input_register_device+0x3b4/0x42c)
> [1.967234] [] (input_register_device) from [] 
> (rc_register_device+0x2d8/0x52c)
> [1.976327] [] (rc_register_device) from [] 
> (tango_ir_probe+0x328/0x3a4)
> [1.984815] [] (tango_ir_probe) from [] 
> (platform_drv_probe+0x34/0x6c)
> [1.993124] [] (platform_drv_probe) from [] 
> (really_probe+0x1c4/0x250)

Ah, that's interesting. The vt console taking a feed from the device, that
makes sense.

> But I have a printk in the ISR, and it's obviously not called.

>From a cursory glance at the driver I can't see anything wrong.

The only thing that stands out is RC5_TIME_BASE. If that is the bit
length or shortest pulse/space? In the latter case it should be 888 usec.

It might be worth trying nec, rc5 and rc6_0 and seeing if any of them decode.

Failing that some documentation would be great :)

> > It would be nice to see this driver merged to mainline.
> 
> +1 (especially if I can get it to work)


Sean


Re: Null Pointer Dereference in mceusb

2017-06-29 Thread Sebastian
Sorry for the long delay, Johan.

2017-06-01 9:20 GMT+02:00 Johan Hovold :
> [ +CC: media list ]
>
> On Wed, May 31, 2017 at 08:25:42PM +0200, Sebastian wrote:
>
> What is the lsusb -v output for your device? And have you successfully
> used this device with this driver before?
>

No, the device wasn't successfully used before that- it crashed every time,
so I threw away the usb receiver. This is also the reason why I cannot give
you the lsusb output. But I can give you the VID:PID -> 03ee:2501 if that
is of any help?

>
> Can you reproduce this with a more recent mainline kernel (e.g.
> 4.11.3)?

Unfortunately no :(

>
> This looks like something which could happen if the device is lacking an
> OUT endpoint, and a sanity check to catch that recently went in (and was
> backported to the non-EOL stable trees).

I could buy the same device again and try?

Thanks for your help,
Sebastian


[PATCH] hdmi: audio infoframe log: corrected channel count

2017-06-29 Thread Martin Bugge
Audio channel count should start from 2.

Reference: CEA-861-F Table 27.

Cc: Hans Verkuil 
Reported-by: Ahung Cheng 
Signed-off-by: Martin Bugge 
---
 drivers/video/hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index 1cf907e..35c0408 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -884,7 +884,7 @@ static void hdmi_audio_infoframe_log(const char *level,
  (struct hdmi_any_infoframe *)frame);
 
if (frame->channels)
-   hdmi_log("channels: %u\n", frame->channels - 1);
+   hdmi_log("channels: %u\n", frame->channels + 1);
else
hdmi_log("channels: Refer to stream header\n");
hdmi_log("coding type: %s\n",
-- 
2.9.4



Re: [PATCH RESEND 0/7] Introduce MEDIA_VERSION to end KENREL_VERSION abuse in media

2017-06-29 Thread Stephen Hemminger
On Thu, 29 Jun 2017 11:42:59 +0200
Johannes Thumshirn  wrote:

> On Sat, Jun 24, 2017 at 05:15:07PM -0300, Mauro Carvalho Chehab wrote:
> > Sorry, but I can't see any advantage on it. On the downside, it
> > includes the media controller header file (media.h) where it
> > is not needed.  
> 
> My reasoning was the differences in semantics. KERNEL_VERSION() is for
> encoding the kernel's version triplet not a API or Hardware or whatever
> version. Other subsystems do this as well, for instance in NVMe we have the
> NVME_VS() macro which is used to encode the NVMe Spec compliance from a human
> readable form to the hardware's u32. Also KERNEL_VERISON() shouldn't have
> in-tree users IMHO. Yes there is _one_ other user of it in-tree which is EXT4
> and I already talked to Jan Kara about it and we decided to leave it in until
> 4.20.
> 
> Byte,
>   Johannes

If you read Linus's comments on version.
Driver version is meaningless and there is a desire to rip it out of all
drivers. The reason is that drivers must always behave the same, i.e you
can't use version to change API/ABI behavior. 

Any upstream driver should never use KERNEL_VERSION().


Re: [PATCH v2 04/19] media: camss: Add CSIPHY files

2017-06-29 Thread Todor Tomov
Hi Sakari,

On 06/29/2017 12:34 AM, Sakari Ailus wrote:
> Hi Todor,
> 
> It's been a while --- how do you do?
> 
> Thanks for the patchset!

Thank you for the review. I'll focus more on this now, so let's see :)

> 
> On Mon, Jun 19, 2017 at 05:48:24PM +0300, Todor Tomov wrote:
>> These files control the CSIPHY modules which are responsible for the physical
>> layer of the CSI2 receivers.
>>
>> Signed-off-by: Todor Tomov 
>> ---
>>  drivers/media/platform/qcom/camss-8x16/csiphy.c | 686 
>> 
>>  drivers/media/platform/qcom/camss-8x16/csiphy.h |  77 +++
>>  2 files changed, 763 insertions(+)
>>  create mode 100644 drivers/media/platform/qcom/camss-8x16/csiphy.c
>>  create mode 100644 drivers/media/platform/qcom/camss-8x16/csiphy.h
>>
>> diff --git a/drivers/media/platform/qcom/camss-8x16/csiphy.c 
>> b/drivers/media/platform/qcom/camss-8x16/csiphy.c
>> new file mode 100644
>> index 000..b9d47ca
>> --- /dev/null
>> +++ b/drivers/media/platform/qcom/camss-8x16/csiphy.c
>> @@ -0,0 +1,686 @@
>> +/*
>> + * csiphy.c
>> + *
>> + * Qualcomm MSM Camera Subsystem - CSIPHY Module
>> + *
>> + * Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
>> + * Copyright (C) 2016 Linaro Ltd.
> 
> How about 2017?

How time flies...

> 
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only 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 
>> +#include 
>> +
> 
> ...
> 
>> +/*
>> + * csiphy_init_formats - Initialize formats on all pads
>> + * @sd: CSIPHY V4L2 subdevice
>> + * @fh: V4L2 subdev file handle
>> + *
>> + * Initialize all pad formats with default values.
>> + *
>> + * Return 0 on success or a negative error code otherwise
>> + */
>> +static int csiphy_init_formats(struct v4l2_subdev *sd,
>> +   struct v4l2_subdev_fh *fh)
>> +{
>> +struct v4l2_subdev_format format;
> 
> You can do:
> 
> struct v4l2_subdev_format format = { 0 };
> 
> And drop the memset. Or even better, assign the fields in declaration.

Yes. I'll do so for all memsets in the driver.

> 
>> +
>> +memset(, 0, sizeof(format));
>> +format.pad = MSM_CSIPHY_PAD_SINK;
>> +format.which = fh ? V4L2_SUBDEV_FORMAT_TRY : V4L2_SUBDEV_FORMAT_ACTIVE;
>> +format.format.code = MEDIA_BUS_FMT_UYVY8_2X8;
>> +format.format.width = 1920;
>> +format.format.height = 1080;
>> +
>> +return csiphy_set_format(sd, fh ? fh->pad : NULL, );
>> +}
>> +
>> +/*
>> + * msm_csiphy_subdev_init - Initialize CSIPHY device structure and resources
>> + * @csiphy: CSIPHY device
>> + * @res: CSIPHY module resources table
>> + * @id: CSIPHY module id
>> + *
>> + * Return 0 on success or a negative error code otherwise
>> + */
>> +int msm_csiphy_subdev_init(struct csiphy_device *csiphy,
>> +   struct resources *res, u8 id)
>> +{
>> +struct device *dev = to_device_index(csiphy, id);
>> +struct platform_device *pdev = container_of(dev,
>> +struct platform_device, dev);
> 
> to_platform_device()?

Yes, thanks.

> 
>> +struct resource *r;
>> +int i;
>> +int ret;
>> +
>> +csiphy->id = id;
>> +csiphy->cfg.combo_mode = 0;
>> +
>> +/* Memory */
>> +
>> +r = platform_get_resource_byname(pdev, IORESOURCE_MEM, res->reg[0]);
>> +csiphy->base = devm_ioremap_resource(dev, r);
>> +if (IS_ERR(csiphy->base)) {
>> +dev_err(dev, "could not map memory\n");
>> +return PTR_ERR(csiphy->base);
>> +}
>> +
>> +r = platform_get_resource_byname(pdev, IORESOURCE_MEM, res->reg[1]);
>> +csiphy->base_clk_mux = devm_ioremap_resource(dev, r);
>> +if (IS_ERR(csiphy->base_clk_mux)) {
>> +dev_err(dev, "could not map memory\n");
>> +return PTR_ERR(csiphy->base_clk_mux);
>> +}
>> +
>> +/* Interrupt */
>> +
>> +r = platform_get_resource_byname(pdev, IORESOURCE_IRQ,
>> + res->interrupt[0]);
>> +if (!r) {
>> +dev_err(dev, "missing IRQ\n");
>> +return -EINVAL;
>> +}
>> +
>> +csiphy->irq = r->start;
>> +snprintf(csiphy->irq_name, sizeof(csiphy->irq_name), "%s_%s%d",
>> + dev_name(dev), MSM_CSIPHY_NAME, csiphy->id);
>> +ret = devm_request_irq(dev, csiphy->irq, csiphy_isr,
>> +   IRQF_TRIGGER_RISING, csiphy->irq_name, csiphy);
>> +if (ret < 0) {
>> +dev_err(dev, "request_irq failed\n");
> 
> Printing the error 

Re: Trying to use IR driver for my SoC

2017-06-29 Thread Mason
Hello,

On 29/06/2017 17:55, Sean Young wrote:

> On Thu, Jun 29, 2017 at 05:29:01PM +0200, Mason wrote:
> 
>> I'm trying to use an IR driver written for my SoC:
>> https://github.com/mansr/linux-tangox/blob/master/drivers/media/rc/tangox-ir.c
>>
>> I added these options to my defconfig:
>>
>> +CONFIG_MEDIA_SUPPORT=y
>> +CONFIG_MEDIA_RC_SUPPORT=y
>> +CONFIG_RC_DEVICES=y
>> +CONFIG_IR_TANGO=y
>>
>> (I don't think I need the RC decoders, because the HW is supposed
>> to support HW decoding of NEC, RC5, RC6).
> 
> I haven't seen this driver before, what hardware is this for?

Sigma Designs tango3/tango4 (SMP86xx and SMP87xx)

>> These are the logs printed at boot:
>>
>> [1.827842] IR NEC protocol handler initialized
>> [1.832407] IR RC5(x/sz) protocol handler initialized
>> [1.837491] IR RC6 protocol handler initialized
>> [1.842049] IR JVC protocol handler initialized
>> [1.846606] IR Sony protocol handler initialized
>> [1.851248] IR SANYO protocol handler initialized
>> [1.855979] IR Sharp protocol handler initialized
>> [1.860708] IR MCE Keyboard/mouse protocol handler initialized
>> [1.866575] IR XMP protocol handler initialized
>> [1.871232] tango-ir 10518.ir: SMP86xx IR decoder at 0x10518/0x105e0 IRQ 
>> 21
>> [1.878241] Registered IR keymap rc-empty
>> [1.882457] input: tango-ir as 
>> /devices/platform/soc/10518.ir/rc/rc0/input0
>> [1.889473] tango_ir_open
>> [1.892105] rc rc0: tango-ir as /devices/platform/soc/10518.ir/rc/rc0
>>
>>
>> I was naively expecting some kind of dev/input/event0 node
>> I could cat to grab all the remote control key presses.
>>
>> But I don't see anything relevant in /dev
> 
> Do you have CONFIG_INPUT_EVDEV set? Is udev setup to create the devices?

I was indeed missing CONFIG_INPUT_EVDEV.

As for udev:
[2.199642] udevd[960]: starting eudev-3.2.1

$ ls -l /dev/input/
total 0
drwxr-xr-x2 root root60 Jan  1 00:00 by-path
crw-rw1 root input  13,  64 Jan  1 00:00 event0

But still no cookie:
$ cat /dev/input/event0
remains mute :-(

$ ir-keytable -v -t
Found device /sys/class/rc/rc0/
Input sysfs node is /sys/class/rc/rc0/input0/
Event sysfs node is /sys/class/rc/rc0/input0/event0/
Parsing uevent /sys/class/rc/rc0/input0/event0/uevent
/sys/class/rc/rc0/input0/event0/uevent uevent MAJOR=13
/sys/class/rc/rc0/input0/event0/uevent uevent MINOR=64
/sys/class/rc/rc0/input0/event0/uevent uevent DEVNAME=input/event0
Parsing uevent /sys/class/rc/rc0/uevent
/sys/class/rc/rc0/uevent uevent NAME=rc-empty
input device is /dev/input/event0
/sys/class/rc/rc0/protocols protocol rc-5 (disabled)
/sys/class/rc/rc0/protocols protocol nec (disabled)
/sys/class/rc/rc0/protocols protocol rc-6 (disabled)
Opening /dev/input/event0
Input Protocol version: 0x00010001
Testing events. Please, press CTRL-C to abort.
^C

Is rc-empty perhaps not the right choice?

> By opening the /dev/input/event0 device, tango_ir_open() gets called which
> presumably enables interrupts or IR decoding for the device. It's hard to
> say without knowing anything about the soc.

Actually tango_ir_open() is called at boot, before any process
has a chance to open /dev/input/event0

[1.926730] [] (tango_ir_open) from [] 
(rc_open+0x44/0x6c)
[1.933994] [] (rc_open) from [] 
(input_open_device+0x74/0xac)
[1.941610] [] (input_open_device) from [] 
(kbd_connect+0x64/0x80)
[1.949570] [] (kbd_connect) from [] 
(input_attach_handler+0x1bc/0x1f4)
[1.957965] [] (input_attach_handler) from [] 
(input_register_device+0x3b4/0x42c)
[1.967234] [] (input_register_device) from [] 
(rc_register_device+0x2d8/0x52c)
[1.976327] [] (rc_register_device) from [] 
(tango_ir_probe+0x328/0x3a4)
[1.984815] [] (tango_ir_probe) from [] 
(platform_drv_probe+0x34/0x6c)
[1.993124] [] (platform_drv_probe) from [] 
(really_probe+0x1c4/0x250)

But I have a printk in the ISR, and it's obviously not called.

> It would be nice to see this driver merged to mainline.

+1 (especially if I can get it to work)

Regards.


[PATCH v1.1 2/2] drm: rcar-du: Repair vblank for DRM page flips using the VSP1

2017-06-29 Thread Kieran Bingham
A recent change to the frame completion handling has changed the order
in which the vblank timestamps are updated.

To fix this requires handling the vblank events on the frame end event
which comes from the VSP1 driver on Gen3 instead.

Prevent the CRTC IRQ from being enabled on Gen3 hardware and handle
vblank events through the existing rcar_du_vsp_complete() callback.

The addition of this callback was to provide notification of frame
completions in the event of a race. Further extend the DU callback to
provide notification as to whether the frame was completed or not,
allowing the callback to act as a full vblank interrupt notifier.

The VSP frame end interrupt occurs approximately 50 µs earlier than the
DU frame end interrupt, but this should not cause any undue harm.

Fixes: d503a43ac06a ("drm: rcar-du: Register a completion callback with
VSP1")

Signed-off-by: Kieran Bingham 
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c   | 19 ---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h   |  2 ++
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c|  8 ++--
 drivers/media/platform/vsp1/vsp1_drm.c   |  5 +++--
 drivers/media/platform/vsp1/vsp1_drm.h   |  2 +-
 drivers/media/platform/vsp1/vsp1_pipe.c  | 20 ++--
 drivers/media/platform/vsp1/vsp1_pipe.h  |  2 +-
 drivers/media/platform/vsp1/vsp1_video.c |  6 +-
 include/media/vsp1.h |  2 +-
 9 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 9f53a8243941..e6b8eaa13419 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -621,6 +621,7 @@ static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
 
rcar_du_crtc_write(rcrtc, DSRCR, DSRCR_VBCL);
rcar_du_crtc_set(rcrtc, DIER, DIER_FRE);
+   rcrtc->vblank_enable = true;
 
return 0;
 }
@@ -629,6 +630,7 @@ static void rcar_du_crtc_disable_vblank(struct drm_crtc 
*crtc)
 {
struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
 
+   rcrtc->vblank_enable = false;
rcar_du_crtc_clr(rcrtc, DIER, DIER_FRE);
 }
 
@@ -658,10 +660,12 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
rcar_du_crtc_write(rcrtc, DSRCR, status & DSRCR_MASK);
 
if (status & DSSR_FRM) {
+   /*
+* Gen 3 vblank and page flips are handled through the VSP
+* completion handler
+*/
drm_crtc_handle_vblank(>crtc);
-
-   if (rcdu->info->gen < 3)
-   rcar_du_crtc_finish_page_flip(rcrtc);
+   rcar_du_crtc_finish_page_flip(rcrtc);
 
ret = IRQ_HANDLED;
}
@@ -735,6 +739,15 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, 
unsigned int index)
/* Start with vertical blanking interrupt reporting disabled. */
drm_crtc_vblank_off(crtc);
 
+   /*
+* DU with a VSP1 source uses the VSP1 frame completion event to handle
+* vblanking and page flipping events.
+*
+* Do not register the IRQ handler in this instance.
+*/
+   if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
+   return 0;
+
/* Register the interrupt handler. */
if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ_CLOCK)) {
irq = platform_get_irq(pdev, index);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index b199ed5adf36..cf5fcc3a3418 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -31,6 +31,7 @@ struct rcar_du_vsp;
  * @mmio_offset: offset of the CRTC registers in the DU MMIO block
  * @index: CRTC software and hardware index
  * @started: whether the CRTC has been started and is running
+ * @vblank_enable: whether vblank events are enabled on this CRTC
  * @event: event to post when the pending page flip completes
  * @flip_wait: wait queue used to signal page flip completion
  * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC
@@ -45,6 +46,7 @@ struct rcar_du_crtc {
unsigned int index;
bool started;
 
+   bool vblank_enable;
struct drm_pending_vblank_event *event;
wait_queue_head_t flip_wait;
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c 
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index f870445ebc8d..9144d3d50067 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -30,11 +30,15 @@
 #include "rcar_du_kms.h"
 #include "rcar_du_vsp.h"
 
-static void rcar_du_vsp_complete(void *private)
+static void rcar_du_vsp_complete(void *private, bool completed)
 {
struct rcar_du_crtc *crtc = private;
 
-   rcar_du_crtc_finish_page_flip(crtc);
+   if (crtc->vblank_enable)
+   drm_crtc_handle_vblank(>crtc);
+
+   if (completed)
+  

Re: Trying to use IR driver for my SoC

2017-06-29 Thread Sean Young
Hello,

On Thu, Jun 29, 2017 at 05:29:01PM +0200, Mason wrote:
> I'm trying to use an IR driver written for my SoC:
> https://github.com/mansr/linux-tangox/blob/master/drivers/media/rc/tangox-ir.c
> 
> I added these options to my defconfig:
> 
> +CONFIG_MEDIA_SUPPORT=y
> +CONFIG_MEDIA_RC_SUPPORT=y
> +CONFIG_RC_DEVICES=y
> +CONFIG_IR_TANGO=y
> 
> (I don't think I need the RC decoders, because the HW is supposed
> to support HW decoding of NEC, RC5, RC6).

I haven't seen this driver before, what hardware is this for?

> These are the logs printed at boot:
> 
> [1.827842] IR NEC protocol handler initialized
> [1.832407] IR RC5(x/sz) protocol handler initialized
> [1.837491] IR RC6 protocol handler initialized
> [1.842049] IR JVC protocol handler initialized
> [1.846606] IR Sony protocol handler initialized
> [1.851248] IR SANYO protocol handler initialized
> [1.855979] IR Sharp protocol handler initialized
> [1.860708] IR MCE Keyboard/mouse protocol handler initialized
> [1.866575] IR XMP protocol handler initialized
> [1.871232] tango-ir 10518.ir: SMP86xx IR decoder at 0x10518/0x105e0 IRQ 21
> [1.878241] Registered IR keymap rc-empty
> [1.882457] input: tango-ir as /devices/platform/soc/10518.ir/rc/rc0/input0
> [1.889473] tango_ir_open
> [1.892105] rc rc0: tango-ir as /devices/platform/soc/10518.ir/rc/rc0
> 
> 
> I was naively expecting some kind of dev/input/event0 node
> I could cat to grab all the remote control key presses.
> 
> But I don't see anything relevant in /dev

Do you have CONFIG_INPUT_EVDEV set? Is udev setup to create the devices?
 
> /sys/devices/platform/soc/10518.ir/rc/rc0/input0$ ls -l
> total 0
> drwxr-xr-x2 root root 0 Jan  1 00:00 capabilities
> lrwxrwxrwx1 root root 0 Jan  1 00:07 device -> ../../rc0
> drwxr-xr-x2 root root 0 Jan  1 00:07 id
> -r--r--r--1 root root  4096 Jan  1 00:07 modalias
> -r--r--r--1 root root  4096 Jan  1 00:00 name
> -r--r--r--1 root root  4096 Jan  1 00:07 phys
> -r--r--r--1 root root  4096 Jan  1 00:00 properties
> lrwxrwxrwx1 root root 0 Jan  1 00:07 subsystem -> 
> ../../../../../../../class/input
> -rw-r--r--1 root root  4096 Jan  1 00:00 uevent
> -r--r--r--1 root root  4096 Jan  1 00:07 uniq
> 
> $ cat *
> cat: read error: Is a directory
> cat: read error: Is a directory
> cat: read error: Is a directory
> input:bvpe-e0,1,4,14,k98,ram4,lsfw
> tango-ir
> tango-ir/input0
> 0
> cat: read error: Is a directory
> PRODUCT=0/0/0/0
> NAME="tango-ir"
> PHYS="tango-ir/input0"
> PROP=0
> EV=100013
> KEY=100 0 0 0 0
> MSC=10
> MODALIAS=input:bvpe-e0,1,4,14,k98,ram4,lsfw
> 
> 
> The IR interrupt count remains at 0, even I use the RC nearby.
> (It works in a legacy system, using a different driver.)

By opening the /dev/input/event0 device, tango_ir_open() gets called which
presumably enables interrupts or IR decoding for the device. It's hard to
say without knowing anything about the soc.

It would be nice to see this driver merged to mainline.


Sean


Re: Trying to use IR driver for my SoC

2017-06-29 Thread Mason
On 29/06/2017 17:29, Mason wrote:

> I suppose I am missing some important piece of the puzzle?
> I hope someone can point me in the right direction.

FWIW,

$ ir-keytable -v
Found device /sys/class/rc/rc0/
Input sysfs node is /sys/class/rc/rc0/input0/
Couldn't find any node at /sys/class/rc/rc0/input0/event*.
Segmentation fault

$ ir-keytable --version
IR keytable control version 1.12.2

Regards.


Trying to use IR driver for my SoC

2017-06-29 Thread Mason
Hello,

I'm trying to use an IR driver written for my SoC:
https://github.com/mansr/linux-tangox/blob/master/drivers/media/rc/tangox-ir.c

I added these options to my defconfig:

+CONFIG_MEDIA_SUPPORT=y
+CONFIG_MEDIA_RC_SUPPORT=y
+CONFIG_RC_DEVICES=y
+CONFIG_IR_TANGO=y

(I don't think I need the RC decoders, because the HW is supposed
to support HW decoding of NEC, RC5, RC6).

These are the logs printed at boot:

[1.827842] IR NEC protocol handler initialized
[1.832407] IR RC5(x/sz) protocol handler initialized
[1.837491] IR RC6 protocol handler initialized
[1.842049] IR JVC protocol handler initialized
[1.846606] IR Sony protocol handler initialized
[1.851248] IR SANYO protocol handler initialized
[1.855979] IR Sharp protocol handler initialized
[1.860708] IR MCE Keyboard/mouse protocol handler initialized
[1.866575] IR XMP protocol handler initialized
[1.871232] tango-ir 10518.ir: SMP86xx IR decoder at 0x10518/0x105e0 IRQ 21
[1.878241] Registered IR keymap rc-empty
[1.882457] input: tango-ir as /devices/platform/soc/10518.ir/rc/rc0/input0
[1.889473] tango_ir_open
[1.892105] rc rc0: tango-ir as /devices/platform/soc/10518.ir/rc/rc0


I was naively expecting some kind of dev/input/event0 node
I could cat to grab all the remote control key presses.

But I don't see anything relevant in /dev

/sys/devices/platform/soc/10518.ir/rc/rc0/input0$ ls -l
total 0
drwxr-xr-x2 root root 0 Jan  1 00:00 capabilities
lrwxrwxrwx1 root root 0 Jan  1 00:07 device -> ../../rc0
drwxr-xr-x2 root root 0 Jan  1 00:07 id
-r--r--r--1 root root  4096 Jan  1 00:07 modalias
-r--r--r--1 root root  4096 Jan  1 00:00 name
-r--r--r--1 root root  4096 Jan  1 00:07 phys
-r--r--r--1 root root  4096 Jan  1 00:00 properties
lrwxrwxrwx1 root root 0 Jan  1 00:07 subsystem -> 
../../../../../../../class/input
-rw-r--r--1 root root  4096 Jan  1 00:00 uevent
-r--r--r--1 root root  4096 Jan  1 00:07 uniq

$ cat *
cat: read error: Is a directory
cat: read error: Is a directory
cat: read error: Is a directory
input:bvpe-e0,1,4,14,k98,ram4,lsfw
tango-ir
tango-ir/input0
0
cat: read error: Is a directory
PRODUCT=0/0/0/0
NAME="tango-ir"
PHYS="tango-ir/input0"
PROP=0
EV=100013
KEY=100 0 0 0 0
MSC=10
MODALIAS=input:bvpe-e0,1,4,14,k98,ram4,lsfw


The IR interrupt count remains at 0, even I use the RC nearby.
(It works in a legacy system, using a different driver.)


I suppose I am missing some important piece of the puzzle?
I hope someone can point me in the right direction.

Regards.


Re: [PATCH v1 2/2] drm: rcar-du: Repair vblank for DRM page flips using the VSP1

2017-06-29 Thread Kieran Bingham
> @@ -658,10 +660,14 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
>   rcar_du_crtc_write(rcrtc, DSRCR, status & DSRCR_MASK);
>  
>   if (status & DSSR_FRM) {
> - drm_crtc_handle_vblank(>crtc);
> -
> - if (rcdu->info->gen < 3)
> + /*
> +  * Gen 3 vblank and page flips are handled through the VSP
> +  * completion handler
> +  */
> + if (rcdu->info->gen < 3) {

Of course as is obvious immediately after hitting send, this check was supposed
to be removed now that the interrupt is not registered.

Sorry for the noise and pre-released patch!

--
Kieran


[PATCH v1 0/2] drm: rcar-du: Repair vblank event handling

2017-06-29 Thread Kieran Bingham
The recent changes to the rcar-du driver to fix a race condition inadvertently
change the order of which vblanks are reported.

Correct this by handling vblank events in the same completion handler. This
removes the need for the IRQ handler on DU instances which are sourced by a
VSP1.

For other platforms (Gen2) the vblank handler was enabling the VBK interrupt,
but parsing on the FRM interrupt. Fix this by enabling the FRM interrupt using
the FRE bit in the DIER register

Kieran Bingham (2):
  drm: rcar-du: Enable the FRM interrupt for vblank
  drm: rcar-du: Repair vblank for DRM page flips using the VSP1

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c   | 25 -
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h   |  2 ++-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c|  8 ++--
 drivers/media/platform/vsp1/vsp1_drm.c   |  5 +++--
 drivers/media/platform/vsp1/vsp1_drm.h   |  2 +-
 drivers/media/platform/vsp1/vsp1_pipe.c  | 20 ++--
 drivers/media/platform/vsp1/vsp1_pipe.h  |  2 +-
 drivers/media/platform/vsp1/vsp1_video.c |  6 +-
 include/media/vsp1.h |  2 +-
 9 files changed, 49 insertions(+), 23 deletions(-)

base-commit: fa5b4114202de0c1a7a64fd407af0b81ca529419
-- 
git-series 0.9.1


[PATCH v1 2/2] drm: rcar-du: Repair vblank for DRM page flips using the VSP1

2017-06-29 Thread Kieran Bingham
A recent change to the frame completion handling has changed the order
in which the vblank timestamps are updated.

To fix this requires handling the vblank events on the frame end event
which comes from the VSP1 driver on Gen3 instead.

Prevent the CRTC IRQ from being enabled on Gen3 hardware and handle
vblank events through the existing rcar_du_vsp_complete() callback.

The addition of this callback was to provide notification of frame
completions in the event of a race. Further extend the DU callback to
provide notification as to whether the frame was completed or not,
allowing the callback to act as a full vblank interrupt notifier.

The VSP frame end interrupt occurs approximately 50 µs earlier than the
DU frame end interrupt, but this should not cause any undue harm.

Fixes: d503a43ac06a ("drm: rcar-du: Register a completion callback with
VSP1")

Signed-off-by: Kieran Bingham 
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c   | 21 ++---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.h   |  2 ++
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c|  8 ++--
 drivers/media/platform/vsp1/vsp1_drm.c   |  5 +++--
 drivers/media/platform/vsp1/vsp1_drm.h   |  2 +-
 drivers/media/platform/vsp1/vsp1_pipe.c  | 20 ++--
 drivers/media/platform/vsp1/vsp1_pipe.h  |  2 +-
 drivers/media/platform/vsp1/vsp1_video.c |  6 +-
 include/media/vsp1.h |  2 +-
 9 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 9f53a8243941..c3b9ecf456d5 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -621,6 +621,7 @@ static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
 
rcar_du_crtc_write(rcrtc, DSRCR, DSRCR_VBCL);
rcar_du_crtc_set(rcrtc, DIER, DIER_FRE);
+   rcrtc->vblank_enable = true;
 
return 0;
 }
@@ -629,6 +630,7 @@ static void rcar_du_crtc_disable_vblank(struct drm_crtc 
*crtc)
 {
struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
 
+   rcrtc->vblank_enable = false;
rcar_du_crtc_clr(rcrtc, DIER, DIER_FRE);
 }
 
@@ -658,10 +660,14 @@ static irqreturn_t rcar_du_crtc_irq(int irq, void *arg)
rcar_du_crtc_write(rcrtc, DSRCR, status & DSRCR_MASK);
 
if (status & DSSR_FRM) {
-   drm_crtc_handle_vblank(>crtc);
-
-   if (rcdu->info->gen < 3)
+   /*
+* Gen 3 vblank and page flips are handled through the VSP
+* completion handler
+*/
+   if (rcdu->info->gen < 3) {
+   drm_crtc_handle_vblank(>crtc);
rcar_du_crtc_finish_page_flip(rcrtc);
+   }
 
ret = IRQ_HANDLED;
}
@@ -735,6 +741,15 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp, 
unsigned int index)
/* Start with vertical blanking interrupt reporting disabled. */
drm_crtc_vblank_off(crtc);
 
+   /*
+* DU with a VSP1 source uses the VSP1 frame completion event to handle
+* vblanking and page flipping events.
+*
+* Do not register the IRQ handler in this instance.
+*/
+   if (rcar_du_has(rcdu, RCAR_DU_FEATURE_VSP1_SOURCE))
+   return 0;
+
/* Register the interrupt handler. */
if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CRTC_IRQ_CLOCK)) {
irq = platform_get_irq(pdev, index);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
index b199ed5adf36..cf5fcc3a3418 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
@@ -31,6 +31,7 @@ struct rcar_du_vsp;
  * @mmio_offset: offset of the CRTC registers in the DU MMIO block
  * @index: CRTC software and hardware index
  * @started: whether the CRTC has been started and is running
+ * @vblank_enable: whether vblank events are enabled on this CRTC
  * @event: event to post when the pending page flip completes
  * @flip_wait: wait queue used to signal page flip completion
  * @outputs: bitmask of the outputs (enum rcar_du_output) driven by this CRTC
@@ -45,6 +46,7 @@ struct rcar_du_crtc {
unsigned int index;
bool started;
 
+   bool vblank_enable;
struct drm_pending_vblank_event *event;
wait_queue_head_t flip_wait;
 
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c 
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index f870445ebc8d..9144d3d50067 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -30,11 +30,15 @@
 #include "rcar_du_kms.h"
 #include "rcar_du_vsp.h"
 
-static void rcar_du_vsp_complete(void *private)
+static void rcar_du_vsp_complete(void *private, bool completed)
 {
struct rcar_du_crtc *crtc = private;
 
-   rcar_du_crtc_finish_page_flip(crtc);
+   if (crtc->vblank_enable)
+   

[PATCH v1 1/2] drm: rcar-du: Enable the FRM interrupt for vblank

2017-06-29 Thread Kieran Bingham
The rcar_du_crtc_{enable,disable}_vblank functions are configured to
control the VBE interrupt event.

The implementation of interlaced support in the rcar-du changes the
required behavior such that vblanks are handled on frame end events, but
does not update the enable register to reflect this.

Enable the FRM interrupt in the DIER register using the FRE bit.

Fixes: 906eff7fcada ("drm: rcar-du: Implement support for interlaced
modes")

Signed-off-by: Kieran Bingham 
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 345eff72f581..9f53a8243941 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -620,7 +620,7 @@ static int rcar_du_crtc_enable_vblank(struct drm_crtc *crtc)
struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
 
rcar_du_crtc_write(rcrtc, DSRCR, DSRCR_VBCL);
-   rcar_du_crtc_set(rcrtc, DIER, DIER_VBE);
+   rcar_du_crtc_set(rcrtc, DIER, DIER_FRE);
 
return 0;
 }
@@ -629,7 +629,7 @@ static void rcar_du_crtc_disable_vblank(struct drm_crtc 
*crtc)
 {
struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
 
-   rcar_du_crtc_clr(rcrtc, DIER, DIER_VBE);
+   rcar_du_crtc_clr(rcrtc, DIER, DIER_FRE);
 }
 
 static const struct drm_crtc_funcs crtc_funcs = {
-- 
git-series 0.9.1


Re: [PATCH v1 4/6] [media] ov9650: use write_array() for resolution sequences

2017-06-29 Thread Hugues FRUCHET


On 06/26/2017 06:33 PM, Sakari Ailus wrote:
> Hi Hugues,
> 
> On Thu, Jun 22, 2017 at 05:05:40PM +0200, Hugues Fruchet wrote:
>> Align resolution sequences on initialization sequence using
>> i2c_rv structure NULL terminated .This add flexibility
>> on resolution sequence size.
>> Document resolution related registers by using corresponding
>> define instead of hexa address/value.
>>
>> Signed-off-by: Hugues Fruchet 
>> ---
>>   drivers/media/i2c/ov9650.c | 98 
>> ++
>>   1 file changed, 64 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
>> index 4311da6..8b283c9 100644
>> --- a/drivers/media/i2c/ov9650.c
>> +++ b/drivers/media/i2c/ov9650.c
>> @@ -227,11 +227,16 @@ struct ov965x_ctrls {
>>  u8 update;
>>   };
>>   
>> +struct i2c_rv {
>> +u8 addr;
>> +u8 value;
>> +};
>> +
>>   struct ov965x_framesize {
>>  u16 width;
>>  u16 height;
>>  u16 max_exp_lines;
>> -const u8 *regs;
>> +const struct i2c_rv *regs;
>>   };
>>   
>>   struct ov965x_interval {
>> @@ -280,9 +285,11 @@ struct ov965x {
>>  u8 apply_frame_fmt;
>>   };
>>   
>> -struct i2c_rv {
>> -u8 addr;
>> -u8 value;
>> +struct ov965x_pixfmt {
>> +u32 code;
>> +u32 colorspace;
>> +/* REG_TSLB value, only bits [3:2] may be set. */
>> +u8 tslb_reg;
>>   };
>>   
>>   static const struct i2c_rv ov965x_init_regs[] = {
>> @@ -342,30 +349,59 @@ struct i2c_rv {
>>  { REG_NULL, 0 }
>>   };
>>   
>> -#define NUM_FMT_REGS 14
>> -/*
>> - * COM7,  COM3,  COM4, HSTART, HSTOP, HREF, VSTART, VSTOP, VREF,
>> - * EXHCH, EXHCL, ADC,  OCOM,   OFON
>> - */
>> -static const u8 frame_size_reg_addr[NUM_FMT_REGS] = {
>> -0x12, 0x0c, 0x0d, 0x17, 0x18, 0x32, 0x19, 0x1a, 0x03,
>> -0x2a, 0x2b, 0x37, 0x38, 0x39,
>> -};
>> -
>> -static const u8 ov965x_sxga_regs[NUM_FMT_REGS] = {
>> -0x00, 0x00, 0x00, 0x1e, 0xbe, 0xbf, 0x01, 0x81, 0x12,
>> -0x10, 0x34, 0x81, 0x93, 0x51,
>> +static const struct i2c_rv ov965x_sxga_regs[] = {
>> +{ REG_COM7, 0x00 },
>> +{ REG_COM3, 0x00 },
>> +{ REG_COM4, 0x00 },
>> +{ REG_HSTART, 0x1e },
>> +{ REG_HSTOP, 0xbe },
>> +{ 0x32, 0xbf },
>> +{ REG_VSTART, 0x01 },
>> +{ REG_VSTOP, 0x81 },
>> +{ REG_VREF, 0x12 },
>> +{ REG_EXHCH, 0x10 },
>> +{ REG_EXHCL, 0x34 },
>> +{ REG_ADC, 0x81 },
>> +{ REG_ACOM, 0x93 },
>> +{ REG_OFON, 0x51 },
>> +{ REG_NULL, 0 },
>>   };
>>   
>> -static const u8 ov965x_vga_regs[NUM_FMT_REGS] = {
>> -0x40, 0x04, 0x80, 0x26, 0xc6, 0xed, 0x01, 0x3d, 0x00,
>> -0x10, 0x40, 0x91, 0x12, 0x43,
>> +static const struct i2c_rv ov965x_vga_regs[] = {
>> +{ REG_COM7, 0x40 },
>> +{ REG_COM3, 0x04 },
>> +{ REG_COM4, 0x80 },
>> +{ REG_HSTART, 0x26 },
>> +{ REG_HSTOP, 0xc6 },
>> +{ 0x32, 0xed },
>> +{ REG_VSTART, 0x01 },
>> +{ REG_VSTOP, 0x3d },
>> +{ REG_VREF, 0x00 },
>> +{ REG_EXHCH, 0x10 },
>> +{ REG_EXHCL, 0x40 },
>> +{ REG_ADC, 0x91 },
>> +{ REG_ACOM, 0x12 },
>> +{ REG_OFON, 0x43 },
>> +{ REG_NULL, 0 },
>>   };
>>   
>>   /* Determined empirically. */
>> -static const u8 ov965x_qvga_regs[NUM_FMT_REGS] = {
>> -0x10, 0x04, 0x80, 0x25, 0xc5, 0xbf, 0x00, 0x80, 0x12,
>> -0x10, 0x40, 0x91, 0x12, 0x43,
>> +static const struct i2c_rv ov965x_qvga_regs[] = {
>> +{ REG_COM7, 0x10 },
>> +{ REG_COM3, 0x04 },
>> +{ REG_COM4, 0x80 },
>> +{ REG_HSTART, 0x25 },
>> +{ REG_HSTOP, 0xc5 },
>> +{ 0x32, 0xbf },
>> +{ REG_VSTART, 0x00 },
>> +{ REG_VSTOP, 0x80 },
>> +{ REG_VREF, 0x12 },
>> +{ REG_EXHCH, 0x10 },
>> +{ REG_EXHCL, 0x40 },
>> +{ REG_ADC, 0x91 },
>> +{ REG_ACOM, 0x12 },
>> +{ REG_OFON, 0x43 },
>> +{ REG_NULL, 0 },
>>   };
>>   
>>   static const struct ov965x_framesize ov965x_framesizes[] = {
>> @@ -387,13 +423,6 @@ struct i2c_rv {
>>  },
>>   };
>>   
>> -struct ov965x_pixfmt {
>> -u32 code;
>> -u32 colorspace;
>> -/* REG_TSLB value, only bits [3:2] may be set. */
>> -u8 tslb_reg;
>> -};
> 
> Any particular reason for moving struct ov965x_pixfmt definition?

Not in the right patch, must be in 5/6 [media] ov9650: add multiple 
variant support, I'll fix in v2.

> 
>> -
>>   static const struct ov965x_pixfmt ov965x_formats[] = {
>>  { MEDIA_BUS_FMT_YUYV8_2X8, V4L2_COLORSPACE_JPEG, 0x00},
>>  { MEDIA_BUS_FMT_YVYU8_2X8, V4L2_COLORSPACE_JPEG, 0x04},
>> @@ -1268,11 +1297,12 @@ static int ov965x_set_fmt(struct v4l2_subdev *sd, 
>> struct v4l2_subdev_pad_config
>>   
>>   static int ov965x_set_frame_size(struct ov965x *ov965x)
>>   {
>> -int i, ret = 0;
>> +int ret = 0;
>> +
>> +v4l2_dbg(1, debug, ov965x->client, "%s\n", __func__);
>>   
>> -for (i = 0; ret == 0 && i < NUM_FMT_REGS; i++)
>> -ret = ov965x_write(ov965x->client, frame_size_reg_addr[i],
>> -   ov965x->frame_size->regs[i]);
>> 

Re: [PATCH] drivers/staging/media: Prefer using __func__ instead

2017-06-29 Thread Joe Perches
On Thu, 2017-06-29 at 16:59 +0530, Pushkar Jambhlekar wrote:
> Function name is hardcoded. replacing with __func__

Please run your proposed patches through checkpatch
before you send them.

> diff --git a/drivers/staging/media/cxd2099/cxd2099.c 
> b/drivers/staging/media/cxd2099/cxd2099.c
[]
> @@ -100,7 +100,7 @@ static int i2c_read_reg(struct i2c_adapter *adapter, u8 
> adr,
>  .buf = val, .len = 1} };
>  
>   if (i2c_transfer(adapter, msgs, 2) != 2) {
> - dev_err(>dev, "error in i2c_read_reg\n");
> + dev_err(>dev, "error in %s\n", __func__);
>   return -1;
>   }
>   return 0;
> @@ -115,7 +115,7 @@ static int i2c_read(struct i2c_adapter *adapter, u8 adr,
>  .buf = data, .len = n} };
>  
>   if (i2c_transfer(adapter, msgs, 2) != 2) {
> - dev_err(>dev, "error in i2c_read\n");
> + dev_err(>dev, "error in %s\n",__func__);

There is a missing space before __func__.

As well, the form for listing a function name
is generally:

print("%s: \n", __func__);

so ideally, these messages would be something like:

dev_err(>dev, "%s: i2c_transfer error\n", __func__);



Re: [PATCH v7 2/2] media: rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver

2017-06-29 Thread Niklas Söderlund
Hi Hans,

Thanks for your feedback.

On 2017-06-19 13:44:22 +0200, Hans Verkuil wrote:
> On 06/12/2017 04:48 PM, Niklas Söderlund wrote:
> > Hi Hans,
> > 
> > Thanks for your comments.
> > 
> > On 2017-05-29 13:16:23 +0200, Hans Verkuil wrote:
> > > On 05/24/2017 02:13 AM, Niklas Söderlund wrote:
> > > > From: Niklas Söderlund 
> > > > 
> > > > A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
> > > > supports the rcar-vin driver on R-Car Gen3 SoCs where separate CSI-2
> > > > hardware blocks are connected between the video sources and the video
> > > > grabbers (VIN).
> > > > 
> > > > Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
> > > > 
> > > > Signed-off-by: Niklas Söderlund 
> > > > ---
> > > >drivers/media/platform/rcar-vin/Kconfig |  12 +
> > > >drivers/media/platform/rcar-vin/Makefile|   1 +
> > > >drivers/media/platform/rcar-vin/rcar-csi2.c | 867 
> > > > 
> > > >3 files changed, 880 insertions(+)
> > > >create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c
> > > > 
> 
> > > > +static int rcar_csi2_registered(struct v4l2_subdev *sd)
> > > > +{
> > > > +   struct rcar_csi2 *priv = container_of(sd, struct rcar_csi2, 
> > > > subdev);
> > > > +   struct v4l2_async_subdev **subdevs = NULL;
> > > > +   int ret;
> > > > +
> > > > +   subdevs = devm_kzalloc(priv->dev, sizeof(*subdevs), GFP_KERNEL);
> > > > +   if (subdevs == NULL)
> > > > +   return -ENOMEM;
> > > > +
> > > > +   subdevs[0] = >remote.asd;
> > > > +
> > > > +   priv->notifier.num_subdevs = 1;
> > > > +   priv->notifier.subdevs = subdevs;
> > > > +   priv->notifier.bound = rcar_csi2_notify_bound;
> > > > +   priv->notifier.unbind = rcar_csi2_notify_unbind;
> > > > +   priv->notifier.complete = rcar_csi2_notify_complete;
> > > > +
> > > > +   ret = v4l2_async_subnotifier_register(>subdev, 
> > > > >notifier);
> > > > +   if (ret < 0) {
> > > > +   dev_err(priv->dev, "Notifier registration failed\n");
> > > > +   return ret;
> > > > +   }
> > > > +
> > > > +   return 0;
> > > > +}
> > > 
> > > Hmm, I'm trying to understand this, and I got one question. There are at 
> > > least
> > > two complete callbacks: rcar_csi2_notify_complete and the bridge driver's
> > > complete callback. Am I right that the bridge driver's complete callback 
> > > is
> > > called as soon as this function exists (assuming this is the only subdev)?
> > 
> > Yes (at least for the async case).
> > 
> > In v4l2_async_test_notify() calls v4l2_device_register_subdev() which in
> > turns calls this registered callback. v4l2_async_test_notify() then go
> > on and calls the notifiers complete callback.
> > 
> > In my case I have (in the simplified case) AD7482 -> CSI-2 -> VIN. Where
> > VIN is the video device and CSI-2 is the subdevice of VIN while the
> > ADV7482 is a subdevice to the CSI-2. In that case the call graph would
> > be:
> > 
> > v4l2_async_test_notify()(From VIN on the CSI-2 subdev)
> >v4l2_device_register_subdev()
> >  sd->internal_ops->registered(sd);   (sd == CSI-2 subdev)
> >v4l2_async_subnotifier_register() (CSI-2 notifier for the ADV7482 
> > subdev)
> >  v4l2_async_test_notify()(From CSI-2 on the ADV7482) [1]
> >notifier->complete(notifier); (on the notifier from VIN)
> > 
> > > 
> > > So the bridge driver thinks it is complete when in reality this subdev may
> > > be waiting on newly registered subdevs?
> > 
> > Yes if the ADV7482 subdevice are not already registered in [1] above the
> > VIN complete callback would be called before the complete callback have
> > been called on the notifier register from the CSI-2 registered callback.
> > Instead that would be called once the ADV7482 calls
> > v4l2_async_register_subdev().
> > 
> > > 
> > > If I am right, then my question is if that is what we want. If I am wrong,
> > > then what did I miss?
> > 
> > I think that is what we want?
> > 
> >  From the VIN point of view all the subdevices it registered in it's
> > notifier have been found and bound right so I think it's correct to call
> > the complete callback for that notifier at this point?  If it really
> > cared about that all devices be present before it calls it complete
> > callback should it not also add all devices to its own notifier list?
> > 
> > But I do see your point that the VIN really have no way of telling if
> > all devices are present and we are ready to start to stream. This
> > however will be found out with a -EPIPE error if a stream is tried to be
> > started since the CSI-2 driver will fail to verify the pipeline since it
> > have no subdevice attached to its source pad. What do you think?
> 
> I think this is a bad idea. From the point of view of the application you
> expect that once the device nodes 

Re: [PATCH v2 01/19] doc: DT: camss: Binding document for Qualcomm Camera subsystem driver

2017-06-29 Thread Todor Tomov
Thank you for review, Rob.

On 06/23/2017 11:41 PM, Rob Herring wrote:
> On Mon, Jun 19, 2017 at 05:48:21PM +0300, Todor Tomov wrote:
>> Add DT binding document for Qualcomm Camera subsystem driver.
> 
> "dt-bindings: media: ..." for the subject please.

I'll change it in the next version.

> 
>>
>> CC: Rob Herring 
>> CC: devicet...@vger.kernel.org
>> Signed-off-by: Todor Tomov 
>> ---
>>  .../devicetree/bindings/media/qcom,camss.txt   | 196 
>> +
>>  1 file changed, 196 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/qcom,camss.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/qcom,camss.txt 
>> b/Documentation/devicetree/bindings/media/qcom,camss.txt
>> new file mode 100644
>> index 000..5213b03
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/qcom,camss.txt
>> @@ -0,0 +1,196 @@
>> +Qualcomm Camera Subsystem
>> +
>> +* Properties
>> +
>> +- compatible:
>> +Usage: required
>> +Value type: 
>> +Definition: Should contain:
>> +- "qcom,msm8916-camss"
> 
> Okay if it is one node, but I'd like to see some agreement from other QC 
> folks that 1 node is appropriate.

Ok.

> 
>> +- reg:
>> +Usage: required
>> +Value type: 
>> +Definition: Register ranges as listed in the reg-names property.
>> +- reg-names:
>> +Usage: required
>> +Value type: 
>> +Definition: Should contain the following entries:
>> +- "csiphy0"
>> +- "csiphy0_clk_mux"
>> +- "csiphy1"
>> +- "csiphy1_clk_mux"
>> +- "csid0"
>> +- "csid1"
>> +- "ispif"
>> +- "csi_clk_mux"
>> +- "vfe0"
>> +- interrupts:
>> +Usage: required
>> +Value type: 
>> +Definition: Interrupts as listed in the interrupt-names property.
>> +- interrupt-names:
>> +Usage: required
>> +Value type: 
>> +Definition: Should contain the following entries:
>> +- "csiphy0"
>> +- "csiphy1"
>> +- "csid0"
>> +- "csid1"
>> +- "ispif"
>> +- "vfe0"
>> +- power-domains:
>> +Usage: required
>> +Value type: 
>> +Definition: A phandle and power domain specifier pairs to the
>> +power domain which is responsible for collapsing
>> +and restoring power to the peripheral.
>> +- clocks:
>> +Usage: required
>> +Value type: 
>> +Definition: A list of phandle and clock specifier pairs as listed
>> +in clock-names property.
>> +- clock-names:
>> +Usage: required
>> +Value type: 
>> +Definition: Should contain the following entries:
>> +- "camss_top_ahb_clk"
>> +- "ispif_ahb_clk"
>> +- "csiphy0_timer_clk"
>> +- "csiphy1_timer_clk"
>> +- "csi0_ahb_clk"
>> +- "csi0_clk"
>> +- "csi0_phy_clk"
>> +- "csi0_pix_clk"
>> +- "csi0_rdi_clk"
>> +- "csi1_ahb_clk"
>> +- "csi1_clk"
>> +- "csi1_phy_clk"
>> +- "csi1_pix_clk"
>> +- "csi1_rdi_clk"
>> +- "camss_ahb_clk"
>> +- "camss_vfe_vfe_clk"
>> +- "camss_csi_vfe_clk"
>> +- "iface_clk"
>> +- "bus_clk"
> 
> "_clk" is redundant.

Yes, it will be better without it.

-- 
Best regards,
Todor Tomov


Re: [PATCH v3 8/8] [media] s5p-jpeg: Add stream error handling for Exynos5420

2017-06-29 Thread Andrzej Pietrasiewicz

W dniu 27.06.2017 o 18:08, Thierry Escande pisze:

From: henryhsu 

On Exynos5420, the STREAM_STAT bit raised on the JPGINTST register means
there is a syntax error or an unrecoverable error on compressed file
when ERR_INT_EN is set to 1.

Fix this case and report BUF_STATE_ERROR to videobuf2.

Signed-off-by: Henry-Ruey Hsu 
Signed-off-by: Thierry Escande 


Acked-by: Andrzej Pietrasiewicz 


---
  drivers/media/platform/s5p-jpeg/jpeg-core.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 5ad3d43..c35d169 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -2812,6 +2812,7 @@ static irqreturn_t exynos3250_jpeg_irq(int irq, void 
*dev_id)
unsigned long payload_size = 0;
enum vb2_buffer_state state = VB2_BUF_STATE_DONE;
bool interrupt_timeout = false;
+   bool stream_error = false;
u32 irq_status;
  
  	spin_lock(>slock);

@@ -2828,6 +2829,12 @@ static irqreturn_t exynos3250_jpeg_irq(int irq, void 
*dev_id)
  
  	jpeg->irq_status |= irq_status;
  
+	if (jpeg->variant->version == SJPEG_EXYNOS5420 &&

+   irq_status & EXYNOS3250_STREAM_STAT) {
+   stream_error = true;
+   dev_err(jpeg->dev, "Syntax error or unrecoverable error 
occurred.\n");
+   }
+
curr_ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);
  
  	if (!curr_ctx)

@@ -2844,7 +2851,7 @@ static irqreturn_t exynos3250_jpeg_irq(int irq, void 
*dev_id)
EXYNOS3250_RDMA_DONE |
EXYNOS3250_RESULT_STAT))
payload_size = exynos3250_jpeg_compressed_size(jpeg->regs);
-   else if (interrupt_timeout)
+   else if (interrupt_timeout || stream_error)
state = VB2_BUF_STATE_ERROR;
else
goto exit_unlock;





Re: [PATCH v3 6/8] [media] s5p-jpeg: Decode 4:1:1 chroma subsampling format

2017-06-29 Thread Andrzej Pietrasiewicz

W dniu 27.06.2017 o 18:08, Thierry Escande pisze:

From: Tony K Nadackal 

This patch adds support for decoding 4:1:1 chroma subsampling in the
jpeg header parsing function.

Signed-off-by: Tony K Nadackal 
Signed-off-by: Thierry Escande 

Acked-by: Andrzej Pietrasiewicz 


---
  drivers/media/platform/s5p-jpeg/jpeg-core.c | 15 +++
  1 file changed, 15 insertions(+)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 0783809..cca0fb8 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1099,6 +1099,8 @@ static void skip(struct s5p_jpeg_buffer *buf, long len)
  static bool s5p_jpeg_subsampling_decode(struct s5p_jpeg_ctx *ctx,
unsigned int subsampling)
  {
+   unsigned int version;
+
switch (subsampling) {
case 0x11:
ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_444;
@@ -1112,6 +1114,19 @@ static bool s5p_jpeg_subsampling_decode(struct 
s5p_jpeg_ctx *ctx,
case 0x33:
ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY;
break;
+   case 0x41:
+   /*
+* 4:1:1 subsampling only supported by 3250, 5420, and 5433
+* variants
+*/
+   version = ctx->jpeg->variant->version;
+   if (version != SJPEG_EXYNOS3250 &&
+   version != SJPEG_EXYNOS5420 &&
+   version != SJPEG_EXYNOS5433)
+   return false;
+
+   ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_411;
+   break;
default:
return false;
}





Re: [PATCH v3 7/8] [media] s5p-jpeg: Add support for resolution change event

2017-06-29 Thread Andrzej Pietrasiewicz

W dniu 27.06.2017 o 18:08, Thierry Escande pisze:

From: henryhsu 

This patch adds support for resolution change event to notify clients so
they can prepare correct output buffer. When resolution change happened,
G_FMT for CAPTURE should return old resolution and format before CAPTURE
queues streamoff.

This event is used in the Chromium browser project by the V4L2 JPEG
Decode Accelerator (V4L2JDA) to allocate output buffer.

Signed-off-by: Henry-Ruey Hsu 
Signed-off-by: Thierry Escande 


Acked-by: Andrzej Pietrasiewicz 


---
  drivers/media/platform/s5p-jpeg/jpeg-core.c | 106 +---
  drivers/media/platform/s5p-jpeg/jpeg-core.h |   7 ++
  2 files changed, 89 insertions(+), 24 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index cca0fb8..5ad3d43 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -24,6 +24,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -1633,8 +1634,6 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct 
v4l2_format *f)
FMT_TYPE_OUTPUT : FMT_TYPE_CAPTURE;
  
  	q_data->fmt = s5p_jpeg_find_format(ct, pix->pixelformat, f_type);

-   q_data->w = pix->width;
-   q_data->h = pix->height;
if (q_data->fmt->fourcc != V4L2_PIX_FMT_JPEG) {
/*
 * During encoding Exynos4x12 SoCs access wider memory area
@@ -1642,6 +1641,8 @@ static int s5p_jpeg_s_fmt(struct s5p_jpeg_ctx *ct, struct 
v4l2_format *f)
 * the JPEG_IMAGE_SIZE register. In order to avoid sysmmu
 * page fault calculate proper buffer size in such a case.
 */
+   q_data->w = pix->width;
+   q_data->h = pix->height;
if (ct->jpeg->variant->hw_ex4_compat &&
f_type == FMT_TYPE_OUTPUT && ct->mode == S5P_JPEG_ENCODE)
q_data->size = exynos4_jpeg_get_output_buffer_size(ct,
@@ -1717,6 +1718,15 @@ static int s5p_jpeg_s_fmt_vid_out(struct file *file, 
void *priv,
return s5p_jpeg_s_fmt(fh_to_ctx(priv), f);
  }
  
+static int s5p_jpeg_subscribe_event(struct v4l2_fh *fh,

+   const struct v4l2_event_subscription *sub)
+{
+   if (sub->type == V4L2_EVENT_SOURCE_CHANGE)
+   return v4l2_src_change_event_subscribe(fh, sub);
+
+   return -EINVAL;
+}
+
  static int exynos3250_jpeg_try_downscale(struct s5p_jpeg_ctx *ctx,
   struct v4l2_rect *r)
  {
@@ -2042,6 +2052,9 @@ static const struct v4l2_ioctl_ops s5p_jpeg_ioctl_ops = {
  
  	.vidioc_g_selection		= s5p_jpeg_g_selection,

.vidioc_s_selection = s5p_jpeg_s_selection,
+
+   .vidioc_subscribe_event = s5p_jpeg_subscribe_event,
+   .vidioc_unsubscribe_event   = v4l2_event_unsubscribe,
  };
  
  /*

@@ -2434,8 +2447,17 @@ static int s5p_jpeg_job_ready(void *priv)
  {
struct s5p_jpeg_ctx *ctx = priv;
  
-	if (ctx->mode == S5P_JPEG_DECODE)

+   if (ctx->mode == S5P_JPEG_DECODE) {
+   /*
+* We have only one input buffer and one output buffer. If there
+* is a resolution change event, no need to continue decoding.
+*/
+   if (ctx->state == JPEGCTX_RESOLUTION_CHANGE)
+   return 0;
+
return ctx->hdr_parsed;
+   }
+
return 1;
  }
  
@@ -2514,6 +2536,30 @@ static int s5p_jpeg_buf_prepare(struct vb2_buffer *vb)

return 0;
  }
  
+static void s5p_jpeg_set_capture_queue_data(struct s5p_jpeg_ctx *ctx)

+{
+   struct s5p_jpeg_q_data *q_data = >cap_q;
+
+   q_data->w = ctx->out_q.w;
+   q_data->h = ctx->out_q.h;
+
+   /*
+* This call to jpeg_bound_align_image() takes care of width and
+* height values alignment when user space calls the QBUF of
+* OUTPUT buffer after the S_FMT of CAPTURE buffer.
+* Please note that on Exynos4x12 SoCs, resigning from executing
+* S_FMT on capture buffer for each JPEG image can result in a
+* hardware hangup if subsampling is lower than the one of input
+* JPEG.
+*/
+   jpeg_bound_align_image(ctx, _data->w, S5P_JPEG_MIN_WIDTH,
+  S5P_JPEG_MAX_WIDTH, q_data->fmt->h_align,
+  _data->h, S5P_JPEG_MIN_HEIGHT,
+  S5P_JPEG_MAX_HEIGHT, q_data->fmt->v_align);
+
+   q_data->size = q_data->w * q_data->h * q_data->fmt->depth >> 3;
+}
+
  static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
  {
struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -2521,7 +2567,18 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
  
  	if (ctx->mode == S5P_JPEG_DECODE &&

Re: [PATCH v3 5/8] [media] s5p-jpeg: Split s5p_jpeg_parse_hdr()

2017-06-29 Thread Andrzej Pietrasiewicz

W dniu 27.06.2017 o 18:08, Thierry Escande pisze:

This patch moves the subsampling value decoding read from the jpeg
header into its own function. This new function is called
s5p_jpeg_subsampling_decode() and returns true if it successfully
decodes the subsampling value, false otherwise.

Signed-off-by: Thierry Escande 


Acked-by: Andrzej Pietrasiewicz 


---
  drivers/media/platform/s5p-jpeg/jpeg-core.c | 42 -
  1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 1769744..0783809 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1096,6 +1096,29 @@ static void skip(struct s5p_jpeg_buffer *buf, long len)
get_byte(buf);
  }
  
+static bool s5p_jpeg_subsampling_decode(struct s5p_jpeg_ctx *ctx,

+   unsigned int subsampling)
+{
+   switch (subsampling) {
+   case 0x11:
+   ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_444;
+   break;
+   case 0x21:
+   ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_422;
+   break;
+   case 0x22:
+   ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_420;
+   break;
+   case 0x33:
+   ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY;
+   break;
+   default:
+   return false;
+   }
+
+   return true;
+}
+
  static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data *result,
   unsigned long buffer, unsigned long size,
   struct s5p_jpeg_ctx *ctx)
@@ -1207,26 +1230,9 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data 
*result,
}
}
  
-	if (notfound || !sos)

+   if (notfound || !sos || !s5p_jpeg_subsampling_decode(ctx, subsampling))
return false;
  
-	switch (subsampling) {

-   case 0x11:
-   ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_444;
-   break;
-   case 0x21:
-   ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_422;
-   break;
-   case 0x22:
-   ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_420;
-   break;
-   case 0x33:
-   ctx->subsampling = V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY;
-   break;
-   default:
-   return false;
-   }
-
result->w = width;
result->h = height;
result->sos = sos;





Re: [PATCH v3 4/8] [media] s5p-jpeg: Don't use temporary structure in s5p_jpeg_buf_queue

2017-06-29 Thread Andrzej Pietrasiewicz

W dniu 27.06.2017 o 18:08, Thierry Escande pisze:

If s5p_jpeg_parse_hdr() fails to parse the JPEG header, the passed
s5p_jpeg_q_data structure is not modify so there is no need to use a
temporary structure and the field-by-field copy can be avoided.

Signed-off-by: Thierry Escande 


Acked-by: Andrzej Pietrasiewicz 


---
  drivers/media/platform/s5p-jpeg/jpeg-core.c | 23 ---
  1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index df3e5ee..1769744 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -2500,9 +2500,9 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
  
  	if (ctx->mode == S5P_JPEG_DECODE &&

vb->vb2_queue->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
-   struct s5p_jpeg_q_data tmp, *q_data;
+   struct s5p_jpeg_q_data *q_data;
  
-		ctx->hdr_parsed = s5p_jpeg_parse_hdr(,

+   ctx->hdr_parsed = s5p_jpeg_parse_hdr(>out_q,
 (unsigned long)vb2_plane_vaddr(vb, 0),
 min((unsigned long)ctx->out_q.size,
 vb2_get_plane_payload(vb, 0)), ctx);
@@ -2511,24 +2511,9 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
return;
}
  
-		q_data = >out_q;

-   q_data->w = tmp.w;
-   q_data->h = tmp.h;
-   q_data->sos = tmp.sos;
-   memcpy(q_data->dht.marker, tmp.dht.marker,
-  sizeof(tmp.dht.marker));
-   memcpy(q_data->dht.len, tmp.dht.len, sizeof(tmp.dht.len));
-   q_data->dht.n = tmp.dht.n;
-   memcpy(q_data->dqt.marker, tmp.dqt.marker,
-  sizeof(tmp.dqt.marker));
-   memcpy(q_data->dqt.len, tmp.dqt.len, sizeof(tmp.dqt.len));
-   q_data->dqt.n = tmp.dqt.n;
-   q_data->sof = tmp.sof;
-   q_data->sof_len = tmp.sof_len;
-
q_data = >cap_q;
-   q_data->w = tmp.w;
-   q_data->h = tmp.h;
+   q_data->w = ctx->out_q.w;
+   q_data->h = ctx->out_q.h;
  
  		/*

 * This call to jpeg_bound_align_image() takes care of width and





Re: [PATCH v3 3/8] [media] s5p-jpeg: Handle parsing error in s5p_jpeg_parse_hdr()

2017-06-29 Thread Andrzej Pietrasiewicz

W dniu 27.06.2017 o 18:08, Thierry Escande pisze:

This patch modifies the s5p_jpeg_parse_hdr() function so it only
modifies the passed s5p_jpeg_q_data structure if the jpeg header parsing
is successful.

Signed-off-by: Thierry Escande 


Acked-by: Andrzej Pietrasiewicz 


---
  drivers/media/platform/s5p-jpeg/jpeg-core.c | 38 -
  1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 0d935f5..df3e5ee 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1206,22 +1206,9 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data 
*result,
break;
}
}
-   result->w = width;
-   result->h = height;
-   result->sos = sos;
-   result->dht.n = n_dht;
-   while (n_dht--) {
-   result->dht.marker[n_dht] = dht[n_dht];
-   result->dht.len[n_dht] = dht_len[n_dht];
-   }
-   result->dqt.n = n_dqt;
-   while (n_dqt--) {
-   result->dqt.marker[n_dqt] = dqt[n_dqt];
-   result->dqt.len[n_dqt] = dqt_len[n_dqt];
-   }
-   result->sof = sof;
-   result->sof_len = sof_len;
-   result->size = result->components = components;
+
+   if (notfound || !sos)
+   return false;
  
  	switch (subsampling) {

case 0x11:
@@ -1240,7 +1227,24 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data 
*result,
return false;
}
  
-	return !notfound && sos;

+   result->w = width;
+   result->h = height;
+   result->sos = sos;
+   result->dht.n = n_dht;
+   while (n_dht--) {
+   result->dht.marker[n_dht] = dht[n_dht];
+   result->dht.len[n_dht] = dht_len[n_dht];
+   }
+   result->dqt.n = n_dqt;
+   while (n_dqt--) {
+   result->dqt.marker[n_dqt] = dqt[n_dqt];
+   result->dqt.len[n_dqt] = dqt_len[n_dqt];
+   }
+   result->sof = sof;
+   result->sof_len = sof_len;
+   result->size = result->components = components;
+
+   return true;
  }
  
  static int s5p_jpeg_querycap(struct file *file, void *priv,






Re: [PATCH v3 2/8] [media] s5p-jpeg: Correct WARN_ON statement for checking subsampling

2017-06-29 Thread Andrzej Pietrasiewicz

W dniu 27.06.2017 o 18:08, Thierry Escande pisze:

From: Tony K Nadackal 

Corrects the WARN_ON statement for subsampling based on the
JPEG Hardware version.

Signed-off-by: Tony K Nadackal 
Signed-off-by: Thierry Escande 


Acked-by: Andrzej Pietrasiewicz 


---
  drivers/media/platform/s5p-jpeg/jpeg-core.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 623508d..0d935f5 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -614,24 +614,26 @@ static inline struct s5p_jpeg_ctx *fh_to_ctx(struct 
v4l2_fh *fh)
  
  static int s5p_jpeg_to_user_subsampling(struct s5p_jpeg_ctx *ctx)

  {
-   WARN_ON(ctx->subsampling > 3);
-
switch (ctx->jpeg->variant->version) {
case SJPEG_S5P:
+   WARN_ON(ctx->subsampling > 3);
if (ctx->subsampling > 2)
return V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY;
return ctx->subsampling;
case SJPEG_EXYNOS3250:
case SJPEG_EXYNOS5420:
+   WARN_ON(ctx->subsampling > 6);
if (ctx->subsampling > 3)
return V4L2_JPEG_CHROMA_SUBSAMPLING_411;
return exynos3250_decoded_subsampling[ctx->subsampling];
case SJPEG_EXYNOS4:
case SJPEG_EXYNOS5433:
+   WARN_ON(ctx->subsampling > 3);
if (ctx->subsampling > 2)
return V4L2_JPEG_CHROMA_SUBSAMPLING_420;
return exynos4x12_decoded_subsampling[ctx->subsampling];
default:
+   WARN_ON(ctx->subsampling > 3);
return V4L2_JPEG_CHROMA_SUBSAMPLING_GRAY;
}
  }





Re: [PATCH v3 1/8] [media] s5p-jpeg: Call jpeg_bound_align_image after qbuf

2017-06-29 Thread Andrzej Pietrasiewicz

W dniu 27.06.2017 o 18:08, Thierry Escande pisze:

From: Tony K Nadackal 

When queuing an OUTPUT buffer for decoder, s5p_jpeg_parse_hdr()
function parses the input jpeg file and takes the width and height
parameters from its header. These new width/height values will be used
for the calculation of stride. HX_JPEG Hardware needs the width and
height values aligned on a 16 bits boundary. This width/height alignment
is handled in the s5p_jpeg_s_fmt_vid_cap() function during the S_FMT
ioctl call.

But if user space calls the QBUF of OUTPUT buffer after the S_FMT of
CAPTURE buffer, these aligned values will be replaced by the values in
jpeg header. If the width/height values of jpeg are not aligned, the
decoder output will be corrupted. So in this patch we call
jpeg_bound_align_image() to align the width/height values of Capture
buffer in s5p_jpeg_buf_queue().

Signed-off-by: Tony K Nadackal 
Signed-off-by: Thierry Escande 


Acked-by: Andrzej Pietrasiewicz 


---
  drivers/media/platform/s5p-jpeg/jpeg-core.c | 19 +++
  1 file changed, 19 insertions(+)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c 
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 52dc794..623508d 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -2523,6 +2523,25 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
q_data = >cap_q;
q_data->w = tmp.w;
q_data->h = tmp.h;
+
+   /*
+* This call to jpeg_bound_align_image() takes care of width and
+* height values alignment when user space calls the QBUF of
+* OUTPUT buffer after the S_FMT of CAPTURE buffer.
+* Please note that on Exynos4x12 SoCs, resigning from executing
+* S_FMT on capture buffer for each JPEG image can result in a
+* hardware hangup if subsampling is lower than the one of input
+* JPEG.
+*/
+   jpeg_bound_align_image(ctx,
+  _data->w,
+  S5P_JPEG_MIN_WIDTH, S5P_JPEG_MAX_WIDTH,
+  q_data->fmt->h_align,
+  _data->h,
+  S5P_JPEG_MIN_HEIGHT, S5P_JPEG_MAX_HEIGHT,
+  q_data->fmt->v_align);
+
+   q_data->size = q_data->w * q_data->h * q_data->fmt->depth >> 3;
}
  
  	v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);






Re: [PATCH v3 0/8] [media] s5p-jpeg: Various fixes and improvements

2017-06-29 Thread Andrzej Pietrasiewicz

Hi Thierry,

W dniu 27.06.2017 o 18:08, Thierry Escande pisze:

Hi,

This series contains various fixes and improvements for the Samsung
s5p-jpeg driver. Most of these patches come from the Chromium v3.8
kernel tree.



Thank you for the series. It looks good to me.

Andrzej


[PATCH] drivers/staging/media: Prefer using __func__ instead

2017-06-29 Thread Pushkar Jambhlekar
Function name is hardcoded. replacing with __func__

Signed-off-by: Pushkar Jambhlekar 
---
 drivers/staging/media/cxd2099/cxd2099.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/cxd2099/cxd2099.c 
b/drivers/staging/media/cxd2099/cxd2099.c
index f28916e..c866752 100644
--- a/drivers/staging/media/cxd2099/cxd2099.c
+++ b/drivers/staging/media/cxd2099/cxd2099.c
@@ -100,7 +100,7 @@ static int i2c_read_reg(struct i2c_adapter *adapter, u8 adr,
   .buf = val, .len = 1} };
 
if (i2c_transfer(adapter, msgs, 2) != 2) {
-   dev_err(>dev, "error in i2c_read_reg\n");
+   dev_err(>dev, "error in %s\n", __func__);
return -1;
}
return 0;
@@ -115,7 +115,7 @@ static int i2c_read(struct i2c_adapter *adapter, u8 adr,
   .buf = data, .len = n} };
 
if (i2c_transfer(adapter, msgs, 2) != 2) {
-   dev_err(>dev, "error in i2c_read\n");
+   dev_err(>dev, "error in %s\n",__func__);
return -1;
}
return 0;
-- 
2.7.4



[bug report] media: v4l: omap_vout: vrfb: Convert to dmaengine

2017-06-29 Thread Dan Carpenter
Hello Peter Ujfalusi,

The patch 6a1560ecaa8c: "media: v4l: omap_vout: vrfb: Convert to
dmaengine" from May 3, 2017, leads to the following static checker
warning:

drivers/media/platform/omap/omap_vout_vrfb.c:273 
omap_vout_prepare_vrfb()
error: uninitialized symbol 'flags'.

drivers/media/platform/omap/omap_vout_vrfb.c
   232  int omap_vout_prepare_vrfb(struct omap_vout_device *vout,
   233 struct videobuf_buffer *vb)
   234  {
   235  struct dma_async_tx_descriptor *tx;
   236  enum dma_ctrl_flags flags;
^

   237  struct dma_chan *chan = vout->vrfb_dma_tx.chan;
   238  struct dma_device *dmadev = chan->device;
   239  struct dma_interleaved_template *xt = vout->vrfb_dma_tx.xt;
   240  dma_cookie_t cookie;
   241  enum dma_status status;
   242  enum dss_rotation rotation;
   243  size_t dst_icg;
   244  u32 pixsize;
   245  
   246  if (!is_rotation_enabled(vout))
   247  return 0;
   248  
   249  /* If rotation is enabled, copy input buffer into VRFB
   250   * memory space using DMA. We are copying input buffer
   251   * into VRFB memory space of desired angle and DSS will
   252   * read image VRFB memory for 0 degree angle
   253   */
   254  
   255  pixsize = vout->bpp * vout->vrfb_bpp;
   256  dst_icg = ((MAX_PIXELS_PER_LINE * pixsize) -
   257(vout->pix.width * vout->bpp)) + 1;
   258  
   259  xt->src_start = vout->buf_phy_addr[vb->i];
   260  xt->dst_start = vout->vrfb_context[vb->i].paddr[0];
   261  
   262  xt->numf = vout->pix.height;
   263  xt->frame_size = 1;
   264  xt->sgl[0].size = vout->pix.width * vout->bpp;
   265  xt->sgl[0].icg = dst_icg;
   266  
   267  xt->dir = DMA_MEM_TO_MEM;
   268  xt->src_sgl = false;
   269  xt->src_inc = true;
   270  xt->dst_sgl = true;
   271  xt->dst_inc = true;
   272  
   273  tx = dmadev->device_prep_interleaved_dma(chan, xt, flags);
   ^
I'm surprised new versions of GCC don't complain about this.

   274  if (tx == NULL) {
   275  pr_err("%s: DMA interleaved prep error\n", __func__);
   276  return -EINVAL;

regards,
dan carpenter


[PATCH v5 1/4] [media] platform: Add Synopsys Designware HDMI RX PHY e405 Driver

2017-06-29 Thread Jose Abreu
This adds support for the Synopsys Designware HDMI RX PHY e405. This
phy receives and decodes HDMI video that is delivered to a controller.

Main features included in this driver are:
- Equalizer algorithm that chooses the phy best settings
according to the detected HDMI cable characteristics.
- Support for scrambling
- Support for HDMI 2.0 modes up to 6G (HDMI 4k@60Hz).

The driver was implemented as a standalone V4L2 subdevice and the
phy interface with the controller was implemented using V4L2 ioctls. I
do not know if this is the best option but it is not possible to use the
existing API functions directly as we need specific functions that will
be called by the controller at specific configuration stages.

There is also a bidirectional communication between controller and phy:
The phy must provide functions that the controller will call (i.e.
configuration functions) and the controller must provide read/write
callbacks, as well as other specific functions.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Mauro Carvalho Chehab 
Cc: Hans Verkuil 
Cc: Sylwester Nawrocki 

Changes from v4:
- Use usleep_range (Sylwester)
- Remove some comments (Sylwester)
- Parse phy version from of_device_id (Sylwester)
- Use "cfg" instead of "cfg-clk" (Sylwester, Rob)
- Change some messages to dev_dbg (Sylwester)
Changes from v3:
- Use v4l2 async API (Sylwester)
- Use clock API (Sylwester)
- Add compatible string (Sylwester)
Changes from RFC:
- Remove a bunch of functions that can be collapsed into
a single config() function
- Add comments for the callbacks and structures (Hans)
---
 drivers/media/platform/Kconfig|   2 +
 drivers/media/platform/Makefile   |   2 +
 drivers/media/platform/dwc/Kconfig|   8 +
 drivers/media/platform/dwc/Makefile   |   1 +
 drivers/media/platform/dwc/dw-hdmi-phy-e405.c | 844 ++
 drivers/media/platform/dwc/dw-hdmi-phy-e405.h |  63 ++
 include/media/dwc/dw-hdmi-phy-pdata.h | 128 
 7 files changed, 1048 insertions(+)
 create mode 100644 drivers/media/platform/dwc/Kconfig
 create mode 100644 drivers/media/platform/dwc/Makefile
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.c
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.h
 create mode 100644 include/media/dwc/dw-hdmi-phy-pdata.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 1313cd5..47d4a50 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -33,6 +33,8 @@ source "drivers/media/platform/omap/Kconfig"
 
 source "drivers/media/platform/blackfin/Kconfig"
 
+source "drivers/media/platform/dwc/Kconfig"
+
 config VIDEO_SH_VOU
tristate "SuperH VOU video output driver"
depends on MEDIA_CAMERA_SUPPORT
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 9beadc7..e6a55fb 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -86,3 +86,5 @@ obj-$(CONFIG_VIDEO_MEDIATEK_MDP)  += mtk-mdp/
 obj-$(CONFIG_VIDEO_MEDIATEK_JPEG)  += mtk-jpeg/
 
 obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/
+
+obj-y  += dwc/
diff --git a/drivers/media/platform/dwc/Kconfig 
b/drivers/media/platform/dwc/Kconfig
new file mode 100644
index 000..361d38d
--- /dev/null
+++ b/drivers/media/platform/dwc/Kconfig
@@ -0,0 +1,8 @@
+config VIDEO_DWC_HDMI_PHY_E405
+   tristate "Synopsys Designware HDMI RX PHY e405 driver"
+   depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   help
+ Support for Synopsys Designware HDMI RX PHY. Version is e405.
+
+ To compile this driver as a module, choose M here. The module
+ will be called dw-hdmi-phy-e405.
diff --git a/drivers/media/platform/dwc/Makefile 
b/drivers/media/platform/dwc/Makefile
new file mode 100644
index 000..fc3b62c
--- /dev/null
+++ b/drivers/media/platform/dwc/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_DWC_HDMI_PHY_E405) += dw-hdmi-phy-e405.o
diff --git a/drivers/media/platform/dwc/dw-hdmi-phy-e405.c 
b/drivers/media/platform/dwc/dw-hdmi-phy-e405.c
new file mode 100644
index 000..26d70ca
--- /dev/null
+++ b/drivers/media/platform/dwc/dw-hdmi-phy-e405.c
@@ -0,0 +1,844 @@
+/*
+ * Synopsys Designware HDMI PHY E405 driver
+ *
+ * This Synopsys dw-phy-e405 software and associated documentation
+ * (hereinafter the "Software") is an unsupported proprietary work of
+ * Synopsys, Inc. unless otherwise expressly agreed to in writing between
+ * Synopsys and you. The Software IS NOT an item of Licensed Software or a
+ * Licensed Product under any End User Software License Agreement or
+ * Agreement for Licensed Products with Synopsys or any supplement thereto.
+ * Synopsys is a registered 

[PATCH v5 2/4] [media] platform: Add Synopsys Designware HDMI RX Controller Driver

2017-06-29 Thread Jose Abreu
This is an initial submission for the Synopsys Designware HDMI RX
Controller Driver. This driver interacts with a phy driver so that
a communication between them is created and a video pipeline is
configured.

The controller + phy pipeline can then be integrated into a fully
featured system that can be able to receive video up to 4k@60Hz
with deep color 48bit RGB, depending on the platform. Although,
this initial version does not yet handle deep color modes.

This driver was implemented as a standard V4L2 subdevice and its
main features are:
- Internal state machine that reconfigures phy until the
video is not stable
- JTAG communication with phy
- Inter-module communication with phy driver
- Debug write/read ioctls

Some notes:
- RX sense controller (cable connection/disconnection) must
be handled by the platform wrapper as this is not integrated
into the controller RTL
- The same goes for EDID ROM's
- ZCAL calibration is needed only in FPGA platforms, in ASIC
this is not needed
- The state machine is not an ideal solution as it creates a
kthread but it is needed because some sources might not be
very stable at sending the video (i.e. we must react
accordingly).

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Mauro Carvalho Chehab 
Cc: Hans Verkuil 
Cc: Sylwester Nawrocki 

Changes from v4:
- Add flag V4L2_SUBDEV_FL_HAS_DEVNODE (Sylwester)
- Remove some comments and change some messages to dev_dbg (Sylwester)
- Use v4l2_async_subnotifier_register() (Sylwester)
Changes from v3:
- Use v4l2 async API (Sylwester)
- Do not block waiting for phy
- Do not use busy waiting delays (Sylwester)
- Simplify dw_hdmi_power_on (Sylwester)
- Use clock API (Sylwester)
- Use compatible string (Sylwester)
- Minor fixes (Sylwester)
Changes from v2:
- Address review comments from Hans regarding CEC
- Use CEC notifier
- Enable SCDC
Changes from v1:
- Add support for CEC
- Correct typo errors
- Correctly detect interlaced video modes
- Correct VIC parsing
Changes from RFC:
- Add support for HDCP 1.4
- Fixup HDMI_VIC not being parsed (Hans)
- Send source change signal when powering off (Hans)
- Add a "wait stable delay"
- Detect interlaced video modes (Hans)
- Restrain g/s_register from reading/writing to HDCP regs (Hans)
---
 drivers/media/platform/dwc/Kconfig  |   15 +
 drivers/media/platform/dwc/Makefile |1 +
 drivers/media/platform/dwc/dw-hdmi-rx.c | 1824 +++
 drivers/media/platform/dwc/dw-hdmi-rx.h |  441 
 include/media/dwc/dw-hdmi-rx-pdata.h|   97 ++
 5 files changed, 2378 insertions(+)
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.c
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.h
 create mode 100644 include/media/dwc/dw-hdmi-rx-pdata.h

diff --git a/drivers/media/platform/dwc/Kconfig 
b/drivers/media/platform/dwc/Kconfig
index 361d38d..3ddccde 100644
--- a/drivers/media/platform/dwc/Kconfig
+++ b/drivers/media/platform/dwc/Kconfig
@@ -6,3 +6,18 @@ config VIDEO_DWC_HDMI_PHY_E405
 
  To compile this driver as a module, choose M here. The module
  will be called dw-hdmi-phy-e405.
+
+config VIDEO_DWC_HDMI_RX
+   tristate "Synopsys Designware HDMI Receiver driver"
+   depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   help
+ Support for Synopsys Designware HDMI RX controller.
+
+ To compile this driver as a module, choose M here. The module
+ will be called dw-hdmi-rx.
+
+config VIDEO_DWC_HDMI_RX_CEC
+   bool
+   depends on VIDEO_DWC_HDMI_RX
+   select CEC_CORE
+   select CEC_NOTIFIER
diff --git a/drivers/media/platform/dwc/Makefile 
b/drivers/media/platform/dwc/Makefile
index fc3b62c..cd04ca9 100644
--- a/drivers/media/platform/dwc/Makefile
+++ b/drivers/media/platform/dwc/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_VIDEO_DWC_HDMI_PHY_E405) += dw-hdmi-phy-e405.o
+obj-$(CONFIG_VIDEO_DWC_HDMI_RX) += dw-hdmi-rx.o
diff --git a/drivers/media/platform/dwc/dw-hdmi-rx.c 
b/drivers/media/platform/dwc/dw-hdmi-rx.c
new file mode 100644
index 000..4a7b8fc
--- /dev/null
+++ b/drivers/media/platform/dwc/dw-hdmi-rx.c
@@ -0,0 +1,1824 @@
+/*
+ * Synopsys Designware HDMI Receiver controller driver
+ *
+ * This Synopsys dw-hdmi-rx software and associated documentation
+ * (hereinafter the "Software") is an unsupported proprietary work of
+ * Synopsys, Inc. unless otherwise expressly agreed to in writing between
+ * Synopsys and you. The Software IS NOT an item of Licensed Software or a
+ * Licensed Product under any End User Software License Agreement or
+ * Agreement for Licensed Products 

[PATCH v5 4/4] dt-bindings: media: Document Synopsys Designware HDMI RX

2017-06-29 Thread Jose Abreu
Document the bindings for the Synopsys Designware HDMI RX.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Rob Herring 
Cc: Mark Rutland 
Cc: Mauro Carvalho Chehab 
Cc: Hans Verkuil 
Cc: Sylwester Nawrocki 
Cc: devicet...@vger.kernel.org

Changes from v4:
- Use "cfg" instead of "cfg-clk" (Rob)
- Change node names (Rob)
Changes from v3:
- Document the new DT bindings suggested by Sylwester
Changes from v2:
- Document edid-phandle property
---
 .../devicetree/bindings/media/snps,dw-hdmi-rx.txt  | 70 ++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt

diff --git a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt 
b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt
new file mode 100644
index 000..449b8a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt
@@ -0,0 +1,70 @@
+Synopsys DesignWare HDMI RX Decoder
+===
+
+This document defines device tree properties for the Synopsys DesignWare HDMI
+RX Decoder (DWC HDMI RX). It doesn't constitute a device tree binding
+specification by itself but is meant to be referenced by platform-specific
+device tree bindings.
+
+When referenced from platform device tree bindings the properties defined in
+this document are defined as follows.
+
+- compatible: Shall be "snps,dw-hdmi-rx".
+
+- reg: Memory mapped base address and length of the DWC HDMI RX registers.
+
+- interrupts: Reference to the DWC HDMI RX interrupt and 5v sense interrupt.
+
+- clocks: Phandle to the config clock block.
+
+- clock-names: Shall be "cfg".
+
+- edid-phandle: phandle to the EDID handler block.
+
+- #address-cells: Shall be 1.
+
+- #size-cells: Shall be 0.
+
+You also have to create a subnode for phy driver. Phy properties are as 
follows.
+
+- compatible: Shall be "snps,dw-hdmi-phy-e405".
+
+- reg: Shall be JTAG address of phy.
+
+- clocks: Phandle for cfg clock.
+
+- clock-names:Shall be "cfg".
+
+A sample binding is now provided. The compatible string is for a SoC which has
+has a Synopsys DesignWare HDMI RX decoder inside.
+
+Example:
+
+dw_hdmi_soc: dw-hdmi-soc@0 {
+   compatible = "snps,dw-hdmi-soc";
+   reg = <0x11c00 0x1000>; /* EDIDs */
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+
+   hdmi-rx@0 {
+   compatible = "snps,dw-hdmi-rx";
+   reg = <0x0 0x1>;
+   interrupts = <1 2>;
+   edid-phandle = <_hdmi_soc>;
+
+   clocks = <_hdmi_refclk>;
+   clock-names = "cfg";
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   hdmi-phy@fc {
+   compatible = "snps,dw-hdmi-phy-e405";
+   reg = <0xfc>;
+
+   clocks = <_hdmi_refclk>;
+   clock-names = "cfg";
+   };
+   };
+};
-- 
1.9.1




[PATCH v5 0/4] Synopsys Designware HDMI Video Capture Controller + PHY

2017-06-29 Thread Jose Abreu
The Synopsys Designware HDMI RX controller is an HDMI receiver controller that
is responsible to process digital data that comes from a phy. The final result
is a stream of raw video data that can then be connected to a video DMA, for
example, and transfered into RAM so that it can be displayed.

The controller + phy available in this series natively support all HDMI 1.4 and
HDMI 2.0 modes, including deep color. Although, the driver is quite in its
initial stage and unfortunatelly only non deep color modes are supported. Also,
audio is not yet supported in the driver (the controller has several audio
output interfaces).

Version 5 addresses review comments from Sylwester Nawrocki and Rob Herring
regarding device tree bindings, messages printing levels, sleep functions and
uses the new v4l2_async_subnotifier_register() function.

This series depends on the patch at [1].

This series was tested in a FPGA platform.

Jose Abreu (4):
  [media] platform: Add Synopsys Designware HDMI RX PHY e405 Driver
  [media] platform: Add Synopsys Designware HDMI RX Controller Driver
  MAINTAINERS: Add entry for Synopsys Designware HDMI drivers
  dt-bindings: media: Document Synopsys Designware HDMI RX

Cc: Carlos Palminha 
Cc: Mauro Carvalho Chehab 
Cc: Hans Verkuil 
Cc: Rob Herring 
Cc: Mark Rutland 
Cc: Sylwester Nawrocki 

[1] https://patchwork.linuxtv.org/patch/41834/

 .../devicetree/bindings/media/snps,dw-hdmi-rx.txt  |   70 +
 MAINTAINERS|7 +
 drivers/media/platform/Kconfig |2 +
 drivers/media/platform/Makefile|2 +
 drivers/media/platform/dwc/Kconfig |   23 +
 drivers/media/platform/dwc/Makefile|2 +
 drivers/media/platform/dwc/dw-hdmi-phy-e405.c  |  844 +
 drivers/media/platform/dwc/dw-hdmi-phy-e405.h  |   63 +
 drivers/media/platform/dwc/dw-hdmi-rx.c| 1824 
 drivers/media/platform/dwc/dw-hdmi-rx.h|  441 +
 include/media/dwc/dw-hdmi-phy-pdata.h  |  128 ++
 include/media/dwc/dw-hdmi-rx-pdata.h   |   97 ++
 12 files changed, 3503 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.txt
 create mode 100644 drivers/media/platform/dwc/Kconfig
 create mode 100644 drivers/media/platform/dwc/Makefile
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.c
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-phy-e405.h
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.c
 create mode 100644 drivers/media/platform/dwc/dw-hdmi-rx.h
 create mode 100644 include/media/dwc/dw-hdmi-phy-pdata.h
 create mode 100644 include/media/dwc/dw-hdmi-rx-pdata.h

-- 
1.9.1




[PATCH v5 3/4] MAINTAINERS: Add entry for Synopsys Designware HDMI drivers

2017-06-29 Thread Jose Abreu
Add a entry for Synopsys Designware HDMI Receivers drivers
and phys.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c4be6d4..7ebc6dd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11354,6 +11354,13 @@ L: net...@vger.kernel.org
 S: Supported
 F: drivers/net/ethernet/synopsys/
 
+SYNOPSYS DESIGNWARE HDMI RECEIVERS AND PHY DRIVERS
+M: Jose Abreu 
+L: linux-media@vger.kernel.org
+S: Maintained
+F: drivers/media/platform/dwc/*
+F: include/media/dwc/*
+
 SYNOPSYS DESIGNWARE I2C DRIVER
 M: Jarkko Nikula 
 R: Andy Shevchenko 
-- 
1.9.1




Re: [PATCH 2/2] v4l: fwnode: Support generic parsing of graph endpoints in V4L2

2017-06-29 Thread Sakari Ailus
On Thu, Jun 29, 2017 at 12:18:44PM +0200, Niklas Söderlund wrote:
> On 2017-06-29 12:57:55 +0300, Sakari Ailus wrote:
> > On Thu, Jun 29, 2017 at 11:02:03AM +0200, Niklas Söderlund wrote:
> > > Hi Sakari,
> > > 
> > > Thanks for your patch.
> > > 
> > > On 2017-06-29 10:30:10 +0300, Sakari Ailus wrote:
> > > > The current practice is that drivers iterate over their endpoints and
> > > > parse each endpoint separately. This is very similar in a number of
> > > > drivers, implement a generic function for the job. Driver specific 
> > > > matters
> > > > can be taken into account in the driver specific callback.
> > > > 
> > > > Convert the omap3isp as an example.
> > > > 
> > > > Signed-off-by: Sakari Ailus 
> > > > ---
> > > >  drivers/media/platform/omap3isp/isp.c | 91 
> > > > ++---
> > > >  drivers/media/platform/omap3isp/isp.h |  3 --
> > > >  drivers/media/v4l2-core/v4l2-fwnode.c | 94 
> > > > +++
> > > >  include/media/v4l2-async.h|  4 +-
> > > >  include/media/v4l2-fwnode.h   |  9 
> > > >  5 files changed, 132 insertions(+), 69 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/platform/omap3isp/isp.c 
> > > > b/drivers/media/platform/omap3isp/isp.c
> > > > index 9df64c1..9ccf883 100644
> > > > --- a/drivers/media/platform/omap3isp/isp.c
> > > > +++ b/drivers/media/platform/omap3isp/isp.c
> > > > @@ -2008,43 +2008,42 @@ enum isp_of_phy {
> > > > ISP_OF_PHY_CSIPHY2,
> > > >  };
> > > >  
> > > > -static int isp_fwnode_parse(struct device *dev, struct fwnode_handle 
> > > > *fwnode,
> > > > -   struct isp_async_subdev *isd)
> > > > +static int isp_fwnode_parse(struct device *dev,
> > > > +   struct v4l2_fwnode_endpoint *vep,
> > > > +   struct v4l2_async_subdev *asd)
> > > >  {
> > > > +   struct isp_async_subdev *isd =
> > > > +   container_of(asd, struct isp_async_subdev, asd);
> > > > struct isp_bus_cfg *buscfg = >bus;
> > > > -   struct v4l2_fwnode_endpoint vep;
> > > > unsigned int i;
> > > > -   int ret;
> > > > -
> > > > -   ret = v4l2_fwnode_endpoint_parse(fwnode, );
> > > > -   if (ret)
> > > > -   return ret;
> > > >  
> > > > dev_dbg(dev, "parsing endpoint %s, interface %u\n",
> > > > -   to_of_node(fwnode)->full_name, vep.base.port);
> > > > +   to_of_node(vep->base.local_fwnode)->full_name, 
> > > > vep->base.port);
> > > >  
> > > > -   switch (vep.base.port) {
> > > > +   switch (vep->base.port) {
> > > > case ISP_OF_PHY_PARALLEL:
> > > > buscfg->interface = ISP_INTERFACE_PARALLEL;
> > > > buscfg->bus.parallel.data_lane_shift =
> > > > -   vep.bus.parallel.data_shift;
> > > > +   vep->bus.parallel.data_shift;
> > > > buscfg->bus.parallel.clk_pol =
> > > > -   !!(vep.bus.parallel.flags
> > > > +   !!(vep->bus.parallel.flags
> > > >& V4L2_MBUS_PCLK_SAMPLE_FALLING);
> > > > buscfg->bus.parallel.hs_pol =
> > > > -   !!(vep.bus.parallel.flags & 
> > > > V4L2_MBUS_VSYNC_ACTIVE_LOW);
> > > > +   !!(vep->bus.parallel.flags &
> > > > +  V4L2_MBUS_VSYNC_ACTIVE_LOW);
> > > > buscfg->bus.parallel.vs_pol =
> > > > -   !!(vep.bus.parallel.flags & 
> > > > V4L2_MBUS_HSYNC_ACTIVE_LOW);
> > > > +   !!(vep->bus.parallel.flags &
> > > > +  V4L2_MBUS_HSYNC_ACTIVE_LOW);
> > > > buscfg->bus.parallel.fld_pol =
> > > > -   !!(vep.bus.parallel.flags & 
> > > > V4L2_MBUS_FIELD_EVEN_LOW);
> > > > +   !!(vep->bus.parallel.flags & 
> > > > V4L2_MBUS_FIELD_EVEN_LOW);
> > > > buscfg->bus.parallel.data_pol =
> > > > -   !!(vep.bus.parallel.flags & 
> > > > V4L2_MBUS_DATA_ACTIVE_LOW);
> > > > +   !!(vep->bus.parallel.flags & 
> > > > V4L2_MBUS_DATA_ACTIVE_LOW);
> > > > break;
> > > >  
> > > > case ISP_OF_PHY_CSIPHY1:
> > > > case ISP_OF_PHY_CSIPHY2:
> > > > /* FIXME: always assume CSI-2 for now. */
> > > > -   switch (vep.base.port) {
> > > > +   switch (vep->base.port) {
> > > > case ISP_OF_PHY_CSIPHY1:
> > > > buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
> > > > break;
> > > > @@ -2052,18 +2051,19 @@ static int isp_fwnode_parse(struct device *dev, 
> > > > struct fwnode_handle *fwnode,
> > > > buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
> > > > break;
> > > > }
> > > > -   

Re: [PATCH 2/2] v4l: fwnode: Support generic parsing of graph endpoints in V4L2

2017-06-29 Thread Niklas Söderlund
On 2017-06-29 12:57:55 +0300, Sakari Ailus wrote:
> On Thu, Jun 29, 2017 at 11:02:03AM +0200, Niklas Söderlund wrote:
> > Hi Sakari,
> > 
> > Thanks for your patch.
> > 
> > On 2017-06-29 10:30:10 +0300, Sakari Ailus wrote:
> > > The current practice is that drivers iterate over their endpoints and
> > > parse each endpoint separately. This is very similar in a number of
> > > drivers, implement a generic function for the job. Driver specific matters
> > > can be taken into account in the driver specific callback.
> > > 
> > > Convert the omap3isp as an example.
> > > 
> > > Signed-off-by: Sakari Ailus 
> > > ---
> > >  drivers/media/platform/omap3isp/isp.c | 91 
> > > ++---
> > >  drivers/media/platform/omap3isp/isp.h |  3 --
> > >  drivers/media/v4l2-core/v4l2-fwnode.c | 94 
> > > +++
> > >  include/media/v4l2-async.h|  4 +-
> > >  include/media/v4l2-fwnode.h   |  9 
> > >  5 files changed, 132 insertions(+), 69 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/omap3isp/isp.c 
> > > b/drivers/media/platform/omap3isp/isp.c
> > > index 9df64c1..9ccf883 100644
> > > --- a/drivers/media/platform/omap3isp/isp.c
> > > +++ b/drivers/media/platform/omap3isp/isp.c
> > > @@ -2008,43 +2008,42 @@ enum isp_of_phy {
> > >   ISP_OF_PHY_CSIPHY2,
> > >  };
> > >  
> > > -static int isp_fwnode_parse(struct device *dev, struct fwnode_handle 
> > > *fwnode,
> > > - struct isp_async_subdev *isd)
> > > +static int isp_fwnode_parse(struct device *dev,
> > > + struct v4l2_fwnode_endpoint *vep,
> > > + struct v4l2_async_subdev *asd)
> > >  {
> > > + struct isp_async_subdev *isd =
> > > + container_of(asd, struct isp_async_subdev, asd);
> > >   struct isp_bus_cfg *buscfg = >bus;
> > > - struct v4l2_fwnode_endpoint vep;
> > >   unsigned int i;
> > > - int ret;
> > > -
> > > - ret = v4l2_fwnode_endpoint_parse(fwnode, );
> > > - if (ret)
> > > - return ret;
> > >  
> > >   dev_dbg(dev, "parsing endpoint %s, interface %u\n",
> > > - to_of_node(fwnode)->full_name, vep.base.port);
> > > + to_of_node(vep->base.local_fwnode)->full_name, vep->base.port);
> > >  
> > > - switch (vep.base.port) {
> > > + switch (vep->base.port) {
> > >   case ISP_OF_PHY_PARALLEL:
> > >   buscfg->interface = ISP_INTERFACE_PARALLEL;
> > >   buscfg->bus.parallel.data_lane_shift =
> > > - vep.bus.parallel.data_shift;
> > > + vep->bus.parallel.data_shift;
> > >   buscfg->bus.parallel.clk_pol =
> > > - !!(vep.bus.parallel.flags
> > > + !!(vep->bus.parallel.flags
> > >  & V4L2_MBUS_PCLK_SAMPLE_FALLING);
> > >   buscfg->bus.parallel.hs_pol =
> > > - !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
> > > + !!(vep->bus.parallel.flags &
> > > +V4L2_MBUS_VSYNC_ACTIVE_LOW);
> > >   buscfg->bus.parallel.vs_pol =
> > > - !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
> > > + !!(vep->bus.parallel.flags &
> > > +V4L2_MBUS_HSYNC_ACTIVE_LOW);
> > >   buscfg->bus.parallel.fld_pol =
> > > - !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> > > + !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> > >   buscfg->bus.parallel.data_pol =
> > > - !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> > > + !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> > >   break;
> > >  
> > >   case ISP_OF_PHY_CSIPHY1:
> > >   case ISP_OF_PHY_CSIPHY2:
> > >   /* FIXME: always assume CSI-2 for now. */
> > > - switch (vep.base.port) {
> > > + switch (vep->base.port) {
> > >   case ISP_OF_PHY_CSIPHY1:
> > >   buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
> > >   break;
> > > @@ -2052,18 +2051,19 @@ static int isp_fwnode_parse(struct device *dev, 
> > > struct fwnode_handle *fwnode,
> > >   buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
> > >   break;
> > >   }
> > > - buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
> > > + buscfg->bus.csi2.lanecfg.clk.pos =
> > > + vep->bus.mipi_csi2.clock_lane;
> > >   buscfg->bus.csi2.lanecfg.clk.pol =
> > > - vep.bus.mipi_csi2.lane_polarities[0];
> > > + vep->bus.mipi_csi2.lane_polarities[0];
> > >   dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> > >   buscfg->bus.csi2.lanecfg.clk.pol,
> > >   buscfg->bus.csi2.lanecfg.clk.pos);
> > >  
> > >   for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
> > >   buscfg->bus.csi2.lanecfg.data[i].pos 

Re: [PATCH 2/2] v4l: fwnode: Support generic parsing of graph endpoints in V4L2

2017-06-29 Thread Sakari Ailus
On Thu, Jun 29, 2017 at 11:02:03AM +0200, Niklas Söderlund wrote:
> Hi Sakari,
> 
> Thanks for your patch.
> 
> On 2017-06-29 10:30:10 +0300, Sakari Ailus wrote:
> > The current practice is that drivers iterate over their endpoints and
> > parse each endpoint separately. This is very similar in a number of
> > drivers, implement a generic function for the job. Driver specific matters
> > can be taken into account in the driver specific callback.
> > 
> > Convert the omap3isp as an example.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/platform/omap3isp/isp.c | 91 
> > ++---
> >  drivers/media/platform/omap3isp/isp.h |  3 --
> >  drivers/media/v4l2-core/v4l2-fwnode.c | 94 
> > +++
> >  include/media/v4l2-async.h|  4 +-
> >  include/media/v4l2-fwnode.h   |  9 
> >  5 files changed, 132 insertions(+), 69 deletions(-)
> > 
> > diff --git a/drivers/media/platform/omap3isp/isp.c 
> > b/drivers/media/platform/omap3isp/isp.c
> > index 9df64c1..9ccf883 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> > @@ -2008,43 +2008,42 @@ enum isp_of_phy {
> > ISP_OF_PHY_CSIPHY2,
> >  };
> >  
> > -static int isp_fwnode_parse(struct device *dev, struct fwnode_handle 
> > *fwnode,
> > -   struct isp_async_subdev *isd)
> > +static int isp_fwnode_parse(struct device *dev,
> > +   struct v4l2_fwnode_endpoint *vep,
> > +   struct v4l2_async_subdev *asd)
> >  {
> > +   struct isp_async_subdev *isd =
> > +   container_of(asd, struct isp_async_subdev, asd);
> > struct isp_bus_cfg *buscfg = >bus;
> > -   struct v4l2_fwnode_endpoint vep;
> > unsigned int i;
> > -   int ret;
> > -
> > -   ret = v4l2_fwnode_endpoint_parse(fwnode, );
> > -   if (ret)
> > -   return ret;
> >  
> > dev_dbg(dev, "parsing endpoint %s, interface %u\n",
> > -   to_of_node(fwnode)->full_name, vep.base.port);
> > +   to_of_node(vep->base.local_fwnode)->full_name, vep->base.port);
> >  
> > -   switch (vep.base.port) {
> > +   switch (vep->base.port) {
> > case ISP_OF_PHY_PARALLEL:
> > buscfg->interface = ISP_INTERFACE_PARALLEL;
> > buscfg->bus.parallel.data_lane_shift =
> > -   vep.bus.parallel.data_shift;
> > +   vep->bus.parallel.data_shift;
> > buscfg->bus.parallel.clk_pol =
> > -   !!(vep.bus.parallel.flags
> > +   !!(vep->bus.parallel.flags
> >& V4L2_MBUS_PCLK_SAMPLE_FALLING);
> > buscfg->bus.parallel.hs_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
> > +   !!(vep->bus.parallel.flags &
> > +  V4L2_MBUS_VSYNC_ACTIVE_LOW);
> > buscfg->bus.parallel.vs_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
> > +   !!(vep->bus.parallel.flags &
> > +  V4L2_MBUS_HSYNC_ACTIVE_LOW);
> > buscfg->bus.parallel.fld_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> > +   !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> > buscfg->bus.parallel.data_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> > +   !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> > break;
> >  
> > case ISP_OF_PHY_CSIPHY1:
> > case ISP_OF_PHY_CSIPHY2:
> > /* FIXME: always assume CSI-2 for now. */
> > -   switch (vep.base.port) {
> > +   switch (vep->base.port) {
> > case ISP_OF_PHY_CSIPHY1:
> > buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
> > break;
> > @@ -2052,18 +2051,19 @@ static int isp_fwnode_parse(struct device *dev, 
> > struct fwnode_handle *fwnode,
> > buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
> > break;
> > }
> > -   buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
> > +   buscfg->bus.csi2.lanecfg.clk.pos =
> > +   vep->bus.mipi_csi2.clock_lane;
> > buscfg->bus.csi2.lanecfg.clk.pol =
> > -   vep.bus.mipi_csi2.lane_polarities[0];
> > +   vep->bus.mipi_csi2.lane_polarities[0];
> > dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> > buscfg->bus.csi2.lanecfg.clk.pol,
> > buscfg->bus.csi2.lanecfg.clk.pos);
> >  
> > for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
> > buscfg->bus.csi2.lanecfg.data[i].pos =
> > -   vep.bus.mipi_csi2.data_lanes[i];
> > +   vep->bus.mipi_csi2.data_lanes[i];
> > 

[PATCH v6.1 1/3] media: adv748x: Add adv7481, adv7482 bindings

2017-06-29 Thread Kieran Bingham
From: Kieran Bingham 

Create device tree bindings documentation for the ADV748x.
The ADV748x supports both the ADV7481 and ADV7482 chips which
provide analogue decoding and HDMI receiving capabilities

Signed-off-by: Kieran Bingham 
Reviewed-by: Laurent Pinchart 
---
v6:
 - Clean up description and remove redundant text regarding optional
   nodes

v6.1:
 - Fix commit title

 Documentation/devicetree/bindings/media/i2c/adv748x.txt | 95 ++-
 1 file changed, 95 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/adv748x.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/adv748x.txt 
b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
new file mode 100644
index ..21ffb5ed8183
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
@@ -0,0 +1,95 @@
+* Analog Devices ADV748X video decoder with HDMI receiver
+
+The ADV7481 and ADV7482 are multi format video decoders with an integrated
+HDMI receiver. They can output CSI-2 on two independent outputs TXA and TXB
+from three input sources HDMI, analog and TTL.
+
+Required Properties:
+
+  - compatible: Must contain one of the following
+- "adi,adv7481" for the ADV7481
+- "adi,adv7482" for the ADV7482
+
+  - reg: I2C slave address
+
+Optional Properties:
+
+  - interrupt-names: Should specify the interrupts as "intrq1", "intrq2" and/or
+"intrq3". All interrupts are optional. The "intrq3" 
interrupt
+is only available on the adv7481
+  - interrupts: Specify the interrupt lines for the ADV748x
+
+The device node must contain one 'port' child node per device input and output
+port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt. The port nodes
+are numbered as follows.
+
+ Name  TypePort
+   ---
+ AIN0  sink0
+ AIN1  sink1
+ AIN2  sink2
+ AIN3  sink3
+ AIN4  sink4
+ AIN5  sink5
+ AIN6  sink6
+ AIN7  sink7
+ HDMI  sink8
+ TTL   sink9
+ TXA   source  10
+ TXB   source  11
+
+The digital output port nodes must contain at least one endpoint.
+
+Ports are optional if they are not connected to anything at the hardware level.
+
+Example:
+
+   video-receiver@70 {
+   compatible = "adi,adv7482";
+   reg = <0x70>;
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   interrupt-parent = <>;
+   interrupt-names = "intrq1", "intrq2";
+   interrupts = <30 IRQ_TYPE_LEVEL_LOW>,
+<31 IRQ_TYPE_LEVEL_LOW>;
+
+   port@7 {
+   reg = <7>;
+
+   adv7482_ain7: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+
+   port@8 {
+   reg = <8>;
+
+   adv7482_hdmi: endpoint {
+   remote-endpoint = <_in>;
+   };
+   };
+
+   port@10 {
+   reg = <10>;
+
+   adv7482_txa: endpoint {
+   clock-lanes = <0>;
+   data-lanes = <1 2 3 4>;
+   remote-endpoint = <_in>;
+   };
+   };
+
+   port@11 {
+   reg = <11>;
+
+   adv7482_txb: endpoint {
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   remote-endpoint = <_in>;
+   };
+   };
+   };
-- 
git-series 0.9.1


Re: [PATCH v6 1/3] media: adv748x: Add adv7181, adv7182 bindings

2017-06-29 Thread Laurent Pinchart
Hi Kieran,

Thank you for the patch.

On Tuesday 27 Jun 2017 16:03:32 Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> Create device tree bindings documentation for the ADV748x.
> The ADV748x supports both the ADV7481 and ADV7482 chips which
> provide analogue decoding and HDMI receiving capabilities
> 
> Signed-off-by: Kieran Bingham 

With the subject fixed,

Reviewed-by: Laurent Pinchart 

> ---
> v6:
>  - Clean up description and remove redundant text regarding optional
>nodes
> 
>  Documentation/devicetree/bindings/media/i2c/adv748x.txt | 95 ++-
>  1 file changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/adv748x.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/adv748x.txt
> b/Documentation/devicetree/bindings/media/i2c/adv748x.txt new file mode
> 100644
> index ..21ffb5ed8183
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/adv748x.txt
> @@ -0,0 +1,95 @@
> +* Analog Devices ADV748X video decoder with HDMI receiver
> +
> +The ADV7481 and ADV7482 are multi format video decoders with an integrated
> +HDMI receiver. They can output CSI-2 on two independent outputs TXA and TXB
> +from three input sources HDMI, analog and TTL.
> +
> +Required Properties:
> +
> +  - compatible: Must contain one of the following
> +- "adi,adv7481" for the ADV7481
> +- "adi,adv7482" for the ADV7482
> +
> +  - reg: I2C slave address
> +
> +Optional Properties:
> +
> +  - interrupt-names: Should specify the interrupts as "intrq1", "intrq2"
> and/or +   "intrq3". All interrupts are optional. The 
"intrq3"
> interrupt +is only available on the adv7481
> +  - interrupts: Specify the interrupt lines for the ADV748x
> +
> +The device node must contain one 'port' child node per device input and
> output +port, in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt. The port
> nodes +are numbered as follows.
> +
> +   Name  TypePort
> + ---
> +   AIN0  sink0
> +   AIN1  sink1
> +   AIN2  sink2
> +   AIN3  sink3
> +   AIN4  sink4
> +   AIN5  sink5
> +   AIN6  sink6
> +   AIN7  sink7
> +   HDMI  sink8
> +   TTL   sink9
> +   TXA   source  10
> +   TXB   source  11
> +
> +The digital output port nodes must contain at least one endpoint.
> +
> +Ports are optional if they are not connected to anything at the hardware
> level. +
> +Example:
> +
> + video-receiver@70 {
> + compatible = "adi,adv7482";
> + reg = <0x70>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + interrupt-parent = <>;
> + interrupt-names = "intrq1", "intrq2";
> + interrupts = <30 IRQ_TYPE_LEVEL_LOW>,
> +  <31 IRQ_TYPE_LEVEL_LOW>;
> +
> + port@7 {
> + reg = <7>;
> +
> + adv7482_ain7: endpoint {
> + remote-endpoint = <_in>;
> + };
> + };
> +
> + port@8 {
> + reg = <8>;
> +
> + adv7482_hdmi: endpoint {
> + remote-endpoint = <_in>;
> + };
> + };
> +
> + port@10 {
> + reg = <10>;
> +
> + adv7482_txa: endpoint {
> + clock-lanes = <0>;
> + data-lanes = <1 2 3 4>;
> + remote-endpoint = <_in>;
> + };
> + };
> +
> + port@11 {
> + reg = <11>;
> +
> + adv7482_txb: endpoint {
> + clock-lanes = <0>;
> + data-lanes = <1>;
> + remote-endpoint = <_in>;
> + };
> + };
> + };

-- 
Regards,

Laurent Pinchart



Re: [PATCH v6 3/3] MAINTAINERS: Add ADV748x driver

2017-06-29 Thread Laurent Pinchart
Hi Kieran,

Thank you for the patch.

On Tuesday 27 Jun 2017 16:03:34 Kieran Bingham wrote:
> From: Kieran Bingham 
> 
> The ADV7481 is an integrated video decoder and combined HDMI/MHL
> receiver.
> 
> Signed-off-by: Kieran Bingham 

Acked-by: Laurent Pinchart 

> ---
>  MAINTAINERS | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c4be6d4af7d2..741da59b133a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -770,6 +770,12 @@ W:   
> http://ez.analog.com/community/linux-device-drivers
>  S:   Supported
>  F:   drivers/media/i2c/adv7180.c
> 
> +ANALOG DEVICES INC ADV748X DRIVER
> +M:   Kieran Bingham 
> +L:   linux-media@vger.kernel.org
> +S:   Maintained
> +F:   drivers/media/i2c/adv748x/*
> +
>  ANALOG DEVICES INC ADV7511 DRIVER
>  M:   Hans Verkuil 
>  L:   linux-media@vger.kernel.org

-- 
Regards,

Laurent Pinchart



Re: [PATCH RESEND 0/7] Introduce MEDIA_VERSION to end KENREL_VERSION abuse in media

2017-06-29 Thread Johannes Thumshirn
On Sat, Jun 24, 2017 at 05:15:07PM -0300, Mauro Carvalho Chehab wrote:
> Sorry, but I can't see any advantage on it. On the downside, it
> includes the media controller header file (media.h) where it
> is not needed.

My reasoning was the differences in semantics. KERNEL_VERSION() is for
encoding the kernel's version triplet not a API or Hardware or whatever
version. Other subsystems do this as well, for instance in NVMe we have the
NVME_VS() macro which is used to encode the NVMe Spec compliance from a human
readable form to the hardware's u32. Also KERNEL_VERISON() shouldn't have
in-tree users IMHO. Yes there is _one_ other user of it in-tree which is EXT4
and I already talked to Jan Kara about it and we decided to leave it in until
4.20.

Byte,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCH v6 1/3] media: adv748x: Add adv7181, adv7182 bindings

2017-06-29 Thread Kieran Bingham


On 29/06/17 10:37, Geert Uytterhoeven wrote:
> On Tue, Jun 27, 2017 at 5:03 PM, Kieran Bingham  wrote:
>> From: Kieran Bingham 
>>
>> Create device tree bindings documentation for the ADV748x.
>> The ADV748x supports both the ADV7481 and ADV7482 chips which
>> provide analogue decoding and HDMI receiving capabilities
> 
> The subject says adv7*1*81, adv7*1*82.

Indeed it does. I'm not sure how I managed to typo that...
My usual typo is adv749x instead :)


Thanks for the spot!

--
Kieran

> 
> Gr{oetje,eeting}s,
> 
> Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- 
> ge...@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like 
> that.
> -- Linus Torvalds
> 

-- 
--
Kieran


Re: [PATCH v6 1/3] media: adv748x: Add adv7181, adv7182 bindings

2017-06-29 Thread Geert Uytterhoeven
On Tue, Jun 27, 2017 at 5:03 PM, Kieran Bingham  wrote:
> From: Kieran Bingham 
>
> Create device tree bindings documentation for the ADV748x.
> The ADV748x supports both the ADV7481 and ADV7482 chips which
> provide analogue decoding and HDMI receiving capabilities

The subject says adv7*1*81, adv7*1*82.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds


Re: [PATCH] [media] staging/imx: remove confusing IS_ERR_OR_NULL usage

2017-06-29 Thread Philipp Zabel
Hi Arnd,

thank you for the cleanup. I see two issues below:

On Wed, 2017-06-28 at 22:13 +0200, Arnd Bergmann wrote:
> While looking at a compiler warning, I noticed the use of
> IS_ERR_OR_NULL, which is generally a sign of a bad API design
> and should be avoided.
> 
> In this driver, this is fairly easy, we can simply stop storing
> error pointers in persistent structures, and change the one
> function that might return either a NULL pointer or an error
> code to consistently return error pointers when failing.
> 
> Fixes: e130291212df ("[media] media: Add i.MX media core driver")
> Signed-off-by: Arnd Bergmann 
> ---
> I can't reproduce the original warning any more, but this
> patch still makes sense by itself.
> ---
>  drivers/staging/media/imx/imx-ic-prpencvf.c | 41 
> -
>  drivers/staging/media/imx/imx-media-csi.c   | 29 +++-
>  drivers/staging/media/imx/imx-media-dev.c   |  2 +-
>  drivers/staging/media/imx/imx-media-of.c|  2 +-
>  drivers/staging/media/imx/imx-media-vdic.c  | 37 ++
>  5 files changed, 61 insertions(+), 50 deletions(-)
> 
[...]
> diff --git a/drivers/staging/media/imx/imx-media-csi.c 
> b/drivers/staging/media/imx/imx-media-csi.c
> index a2d26693912e..a4b3c305dcc8 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -122,11 +122,11 @@ static inline struct csi_priv *sd_to_dev(struct 
> v4l2_subdev *sdev)
>  
>  static void csi_idmac_put_ipu_resources(struct csi_priv *priv)
>  {
> - if (!IS_ERR_OR_NULL(priv->idmac_ch))
> + if (priv->idmac_ch)
>   ipu_idmac_put(priv->idmac_ch);
>   priv->idmac_ch = NULL;
>  
> - if (!IS_ERR_OR_NULL(priv->smfc))
> + if (priv->smfc)
>   ipu_smfc_put(priv->smfc);
>   priv->smfc = NULL;
>  }
> @@ -134,23 +134,26 @@ static void csi_idmac_put_ipu_resources(struct csi_priv 
> *priv)
>  static int csi_idmac_get_ipu_resources(struct csi_priv *priv)
>  {
>   int ch_num, ret;
> + struct ipu_smfc *smfc, *idmac_ch;

This should be

+   struct ipuv3_channel *idmac_ch;
+   struct ipu_smfc *smfc;

instead.

[...]
> diff --git a/drivers/staging/media/imx/imx-media-dev.c 
> b/drivers/staging/media/imx/imx-media-dev.c
> index 48cbc7716758..c58ff0831890 100644
> --- a/drivers/staging/media/imx/imx-media-dev.c
> +++ b/drivers/staging/media/imx/imx-media-dev.c
> @@ -91,7 +91,7 @@ imx_media_add_async_subdev(struct imx_media_dev *imxmd,
>   if (imx_media_find_async_subdev(imxmd, np, devname)) {
>   dev_dbg(imxmd->md.dev, "%s: already added %s\n",
>   __func__, np ? np->name : devname);
> - imxsd = NULL;
> + imxsd = ERR_PTR(-EEXIST);

And since this returns -EEXIST now, ...

>   goto out;
>   }
>  
> diff --git a/drivers/staging/media/imx/imx-media-of.c 
> b/drivers/staging/media/imx/imx-media-of.c
> index b026fe66467c..4aac42cb79a4 100644
> --- a/drivers/staging/media/imx/imx-media-of.c
> +++ b/drivers/staging/media/imx/imx-media-of.c
> @@ -115,7 +115,7 @@ of_parse_subdev(struct imx_media_dev *imxmd, struct 
> device_node *sd_np,
>  
>   /* register this subdev with async notifier */
>   imxsd = imx_media_add_async_subdev(imxmd, sd_np, NULL);
> - if (IS_ERR_OR_NULL(imxsd))
> + if (IS_ERR(imxsd))
>   return imxsd;

... this changes behaviour:

imx-media: imx_media_of_parse failed with -17
imx-media: probe of capture-subsystem failed with error -17

We must continue to return NULL here if imxsd == -EEXIST:

-   return imxsd;
+   return PTR_ERR(imxsd) == -EEXIST ? NULL : imxsd;

or change the code where of_parse_subdev is called (from
imx_media_of_parse, and recursively from of_parse_subdev) to not handle
the -EEXIST return value as an error.

With those fixed,

Reviewed-by: Philipp Zabel 
Tested-by: Philipp Zabel 

regards
Philipp



Re: [PATCH 2/2] v4l: fwnode: Support generic parsing of graph endpoints in V4L2

2017-06-29 Thread Niklas Söderlund
On 2017-06-29 11:02:03 +0200, Niklas Söderlund wrote:
> Hi Sakari,
> 
> Thanks for your patch.
> 
> On 2017-06-29 10:30:10 +0300, Sakari Ailus wrote:
> > The current practice is that drivers iterate over their endpoints and
> > parse each endpoint separately. This is very similar in a number of
> > drivers, implement a generic function for the job. Driver specific matters
> > can be taken into account in the driver specific callback.
> > 
> > Convert the omap3isp as an example.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/platform/omap3isp/isp.c | 91 
> > ++---
> >  drivers/media/platform/omap3isp/isp.h |  3 --
> >  drivers/media/v4l2-core/v4l2-fwnode.c | 94 
> > +++
> >  include/media/v4l2-async.h|  4 +-
> >  include/media/v4l2-fwnode.h   |  9 
> >  5 files changed, 132 insertions(+), 69 deletions(-)
> > 
> > diff --git a/drivers/media/platform/omap3isp/isp.c 
> > b/drivers/media/platform/omap3isp/isp.c
> > index 9df64c1..9ccf883 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> > @@ -2008,43 +2008,42 @@ enum isp_of_phy {
> > ISP_OF_PHY_CSIPHY2,
> >  };
> >  
> > -static int isp_fwnode_parse(struct device *dev, struct fwnode_handle 
> > *fwnode,
> > -   struct isp_async_subdev *isd)
> > +static int isp_fwnode_parse(struct device *dev,
> > +   struct v4l2_fwnode_endpoint *vep,
> > +   struct v4l2_async_subdev *asd)
> >  {
> > +   struct isp_async_subdev *isd =
> > +   container_of(asd, struct isp_async_subdev, asd);
> > struct isp_bus_cfg *buscfg = >bus;
> > -   struct v4l2_fwnode_endpoint vep;
> > unsigned int i;
> > -   int ret;
> > -
> > -   ret = v4l2_fwnode_endpoint_parse(fwnode, );
> > -   if (ret)
> > -   return ret;
> >  
> > dev_dbg(dev, "parsing endpoint %s, interface %u\n",
> > -   to_of_node(fwnode)->full_name, vep.base.port);
> > +   to_of_node(vep->base.local_fwnode)->full_name, vep->base.port);
> >  
> > -   switch (vep.base.port) {
> > +   switch (vep->base.port) {
> > case ISP_OF_PHY_PARALLEL:
> > buscfg->interface = ISP_INTERFACE_PARALLEL;
> > buscfg->bus.parallel.data_lane_shift =
> > -   vep.bus.parallel.data_shift;
> > +   vep->bus.parallel.data_shift;
> > buscfg->bus.parallel.clk_pol =
> > -   !!(vep.bus.parallel.flags
> > +   !!(vep->bus.parallel.flags
> >& V4L2_MBUS_PCLK_SAMPLE_FALLING);
> > buscfg->bus.parallel.hs_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
> > +   !!(vep->bus.parallel.flags &
> > +  V4L2_MBUS_VSYNC_ACTIVE_LOW);
> > buscfg->bus.parallel.vs_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
> > +   !!(vep->bus.parallel.flags &
> > +  V4L2_MBUS_HSYNC_ACTIVE_LOW);
> > buscfg->bus.parallel.fld_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> > +   !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> > buscfg->bus.parallel.data_pol =
> > -   !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> > +   !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> > break;
> >  
> > case ISP_OF_PHY_CSIPHY1:
> > case ISP_OF_PHY_CSIPHY2:
> > /* FIXME: always assume CSI-2 for now. */
> > -   switch (vep.base.port) {
> > +   switch (vep->base.port) {
> > case ISP_OF_PHY_CSIPHY1:
> > buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
> > break;
> > @@ -2052,18 +2051,19 @@ static int isp_fwnode_parse(struct device *dev, 
> > struct fwnode_handle *fwnode,
> > buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
> > break;
> > }
> > -   buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
> > +   buscfg->bus.csi2.lanecfg.clk.pos =
> > +   vep->bus.mipi_csi2.clock_lane;
> > buscfg->bus.csi2.lanecfg.clk.pol =
> > -   vep.bus.mipi_csi2.lane_polarities[0];
> > +   vep->bus.mipi_csi2.lane_polarities[0];
> > dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> > buscfg->bus.csi2.lanecfg.clk.pol,
> > buscfg->bus.csi2.lanecfg.clk.pos);
> >  
> > for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
> > buscfg->bus.csi2.lanecfg.data[i].pos =
> > -   vep.bus.mipi_csi2.data_lanes[i];
> > +   vep->bus.mipi_csi2.data_lanes[i];
> > 

Re: [PATCH 2/2] v4l: fwnode: Support generic parsing of graph endpoints in V4L2

2017-06-29 Thread Niklas Söderlund
Hi Sakari,

Thanks for your patch.

On 2017-06-29 10:30:10 +0300, Sakari Ailus wrote:
> The current practice is that drivers iterate over their endpoints and
> parse each endpoint separately. This is very similar in a number of
> drivers, implement a generic function for the job. Driver specific matters
> can be taken into account in the driver specific callback.
> 
> Convert the omap3isp as an example.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/platform/omap3isp/isp.c | 91 ++---
>  drivers/media/platform/omap3isp/isp.h |  3 --
>  drivers/media/v4l2-core/v4l2-fwnode.c | 94 
> +++
>  include/media/v4l2-async.h|  4 +-
>  include/media/v4l2-fwnode.h   |  9 
>  5 files changed, 132 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c 
> b/drivers/media/platform/omap3isp/isp.c
> index 9df64c1..9ccf883 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2008,43 +2008,42 @@ enum isp_of_phy {
>   ISP_OF_PHY_CSIPHY2,
>  };
>  
> -static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
> - struct isp_async_subdev *isd)
> +static int isp_fwnode_parse(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd)
>  {
> + struct isp_async_subdev *isd =
> + container_of(asd, struct isp_async_subdev, asd);
>   struct isp_bus_cfg *buscfg = >bus;
> - struct v4l2_fwnode_endpoint vep;
>   unsigned int i;
> - int ret;
> -
> - ret = v4l2_fwnode_endpoint_parse(fwnode, );
> - if (ret)
> - return ret;
>  
>   dev_dbg(dev, "parsing endpoint %s, interface %u\n",
> - to_of_node(fwnode)->full_name, vep.base.port);
> + to_of_node(vep->base.local_fwnode)->full_name, vep->base.port);
>  
> - switch (vep.base.port) {
> + switch (vep->base.port) {
>   case ISP_OF_PHY_PARALLEL:
>   buscfg->interface = ISP_INTERFACE_PARALLEL;
>   buscfg->bus.parallel.data_lane_shift =
> - vep.bus.parallel.data_shift;
> + vep->bus.parallel.data_shift;
>   buscfg->bus.parallel.clk_pol =
> - !!(vep.bus.parallel.flags
> + !!(vep->bus.parallel.flags
>  & V4L2_MBUS_PCLK_SAMPLE_FALLING);
>   buscfg->bus.parallel.hs_pol =
> - !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
> + !!(vep->bus.parallel.flags &
> +V4L2_MBUS_VSYNC_ACTIVE_LOW);
>   buscfg->bus.parallel.vs_pol =
> - !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
> + !!(vep->bus.parallel.flags &
> +V4L2_MBUS_HSYNC_ACTIVE_LOW);
>   buscfg->bus.parallel.fld_pol =
> - !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> + !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
>   buscfg->bus.parallel.data_pol =
> - !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> + !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
>   break;
>  
>   case ISP_OF_PHY_CSIPHY1:
>   case ISP_OF_PHY_CSIPHY2:
>   /* FIXME: always assume CSI-2 for now. */
> - switch (vep.base.port) {
> + switch (vep->base.port) {
>   case ISP_OF_PHY_CSIPHY1:
>   buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
>   break;
> @@ -2052,18 +2051,19 @@ static int isp_fwnode_parse(struct device *dev, 
> struct fwnode_handle *fwnode,
>   buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
>   break;
>   }
> - buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
> + buscfg->bus.csi2.lanecfg.clk.pos =
> + vep->bus.mipi_csi2.clock_lane;
>   buscfg->bus.csi2.lanecfg.clk.pol =
> - vep.bus.mipi_csi2.lane_polarities[0];
> + vep->bus.mipi_csi2.lane_polarities[0];
>   dev_dbg(dev, "clock lane polarity %u, pos %u\n",
>   buscfg->bus.csi2.lanecfg.clk.pol,
>   buscfg->bus.csi2.lanecfg.clk.pos);
>  
>   for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
>   buscfg->bus.csi2.lanecfg.data[i].pos =
> - vep.bus.mipi_csi2.data_lanes[i];
> + vep->bus.mipi_csi2.data_lanes[i];
>   buscfg->bus.csi2.lanecfg.data[i].pol =
> - vep.bus.mipi_csi2.lane_polarities[i + 1];
> +

[PATCH] [media] cx23885: add const to v4l2_file_operations structure

2017-06-29 Thread Bhumika Goyal
Declare v4l2_file_operations structure as const as it is only stored
in the fops field of video_device structure. This field is of type
const, so declare v4l2_file_operations structures with similar properties
as const.

Signed-off-by: Bhumika Goyal 
---
 drivers/media/pci/cx23885/cx23885-417.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/cx23885/cx23885-417.c 
b/drivers/media/pci/cx23885/cx23885-417.c
index 2ff1d1e..a71f3c7 100644
--- a/drivers/media/pci/cx23885/cx23885-417.c
+++ b/drivers/media/pci/cx23885/cx23885-417.c
@@ -1416,7 +1416,7 @@ static int vidioc_log_status(struct file *file, void 
*priv)
return 0;
 }
 
-static struct v4l2_file_operations mpeg_fops = {
+static const struct v4l2_file_operations mpeg_fops = {
.owner = THIS_MODULE,
.open   = v4l2_fh_open,
.release= vb2_fop_release,
-- 
2.7.4



[PATCH] [media] media/platform: add const to v4l2_file_operations structures

2017-06-29 Thread Bhumika Goyal
Declare v4l2_file_operations structures as const as they are only stored
in the fops field of video_device structures. This field is of type
const, so declare v4l2_file_operations structures with similar properties
as const.

Cross compiled bfin_capture.o for blackfin arch. vpbe_display.o file did
not cross compile for arm. Could not find any architecture matching the
configuraion symbol for fsl-viu.c file.

Signed-off-by: Bhumika Goyal 
---
 drivers/media/platform/blackfin/bfin_capture.c | 2 +-
 drivers/media/platform/davinci/vpbe_display.c  | 2 +-
 drivers/media/platform/davinci/vpif_capture.c  | 2 +-
 drivers/media/platform/fsl-viu.c   | 2 +-
 drivers/media/platform/soc_camera/soc_camera.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/blackfin/bfin_capture.c 
b/drivers/media/platform/blackfin/bfin_capture.c
index 1c5166d..294d696 100644
--- a/drivers/media/platform/blackfin/bfin_capture.c
+++ b/drivers/media/platform/blackfin/bfin_capture.c
@@ -769,7 +769,7 @@ static const struct v4l2_ioctl_ops bcap_ioctl_ops = {
.vidioc_log_status   = bcap_log_status,
 };
 
-static struct v4l2_file_operations bcap_fops = {
+static const struct v4l2_file_operations bcap_fops = {
.owner = THIS_MODULE,
.open = v4l2_fh_open,
.release = vb2_fop_release,
diff --git a/drivers/media/platform/davinci/vpbe_display.c 
b/drivers/media/platform/davinci/vpbe_display.c
index a9bc017..ca2adfa 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -1275,7 +1275,7 @@ static const struct v4l2_ioctl_ops vpbe_ioctl_ops = {
.vidioc_enum_dv_timings  = vpbe_display_enum_dv_timings,
 };
 
-static struct v4l2_file_operations vpbe_fops = {
+static const struct v4l2_file_operations vpbe_fops = {
.owner = THIS_MODULE,
.open = vpbe_display_open,
.release = vpbe_display_release,
diff --git a/drivers/media/platform/davinci/vpif_capture.c 
b/drivers/media/platform/davinci/vpif_capture.c
index d78580f..fbdc196 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -1344,7 +1344,7 @@ static const struct v4l2_ioctl_ops vpif_ioctl_ops = {
 };
 
 /* vpif file operations */
-static struct v4l2_file_operations vpif_fops = {
+static const struct v4l2_file_operations vpif_fops = {
.owner = THIS_MODULE,
.open = v4l2_fh_open,
.release = vb2_fop_release,
diff --git a/drivers/media/platform/fsl-viu.c b/drivers/media/platform/fsl-viu.c
index 97e164b..f7b88e5 100644
--- a/drivers/media/platform/fsl-viu.c
+++ b/drivers/media/platform/fsl-viu.c
@@ -1340,7 +1340,7 @@ static int viu_mmap(struct file *file, struct 
vm_area_struct *vma)
return ret;
 }
 
-static struct v4l2_file_operations viu_fops = {
+static const struct v4l2_file_operations viu_fops = {
.owner  = THIS_MODULE,
.open   = viu_open,
.release= viu_release,
diff --git a/drivers/media/platform/soc_camera/soc_camera.c 
b/drivers/media/platform/soc_camera/soc_camera.c
index 45a0429..dd349ef 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -820,7 +820,7 @@ static unsigned int soc_camera_poll(struct file *file, 
poll_table *pt)
return res;
 }
 
-static struct v4l2_file_operations soc_camera_fops = {
+static const struct v4l2_file_operations soc_camera_fops = {
.owner  = THIS_MODULE,
.open   = soc_camera_open,
.release= soc_camera_close,
-- 
2.7.4



Re: [PATCH 1/2] v4l: fwnode: link_frequency is an optional property

2017-06-29 Thread Niklas Söderlund
Hi Sakari,

Thanks for your patch.

On 2017-06-29 10:30:09 +0300, Sakari Ailus wrote:
> v4l2_fwnode_endpoint_alloc_parse() is intended as a replacement for
> v4l2_fwnode_endpoint_parse(). It parses the "link-frequency" property and
> if the property isn't found, it returns an error. However,
> "link-frequency" is an optional property and if it does not exist is not
> an error. Instead, the number of link frequencies is simply zero in that
> case.
> 
> Signed-off-by: Sakari Ailus 

Reviewed-by: Niklas Söderlund 

> ---
>  drivers/media/v4l2-core/v4l2-fwnode.c | 30 +++---
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 153c53c..0ec6c14 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -247,23 +247,23 @@ struct v4l2_fwnode_endpoint 
> *v4l2_fwnode_endpoint_alloc_parse(
>  
>   rval = fwnode_property_read_u64_array(fwnode, "link-frequencies",
> NULL, 0);
> - if (rval < 0)
> - goto out_err;
> -
> - vep->link_frequencies =
> - kmalloc_array(rval, sizeof(*vep->link_frequencies), GFP_KERNEL);
> - if (!vep->link_frequencies) {
> - rval = -ENOMEM;
> - goto out_err;
> - }
> + if (rval > 0) {
> + vep->link_frequencies =
> + kmalloc_array(rval, sizeof(*vep->link_frequencies),
> +   GFP_KERNEL);
> + if (!vep->link_frequencies) {
> + rval = -ENOMEM;
> + goto out_err;
> + }
>  
> - vep->nr_of_link_frequencies = rval;
> + vep->nr_of_link_frequencies = rval;
>  
> - rval = fwnode_property_read_u64_array(fwnode, "link-frequencies",
> -   vep->link_frequencies,
> -   vep->nr_of_link_frequencies);
> - if (rval < 0)
> - goto out_err;
> + rval = fwnode_property_read_u64_array(
> + fwnode, "link-frequencies", vep->link_frequencies,
> + vep->nr_of_link_frequencies);
> + if (rval < 0)
> + goto out_err;
> + }
>  
>   return vep;
>  
> -- 
> 2.1.4
> 
> 

-- 
Regards,
Niklas Söderlund


[PATCH] media: exynos4-is: fimc-is-i2c: constify dev_pm_ops structures.

2017-06-29 Thread Arvind Yadav
dev_pm_ops are not supposed to change at runtime. All functions
working with dev_pm_ops provided by  work with const
dev_pm_ops. So mark the non-const structs as const.

File size before:
   textdata bss dec hex filename
   1195 376   01571 623 
drivers/media/platform/exynos4-is/fimc-is-i2c.o

File size After adding 'const':
   textdata bss dec hex filename
   1403 176   01579 62b 
drivers/media/platform/exynos4-is/fimc-is-i2c.o

Signed-off-by: Arvind Yadav 
---
 drivers/media/platform/exynos4-is/fimc-is-i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-is-i2c.c 
b/drivers/media/platform/exynos4-is/fimc-is-i2c.c
index 2f55966..70dd485 100644
--- a/drivers/media/platform/exynos4-is/fimc-is-i2c.c
+++ b/drivers/media/platform/exynos4-is/fimc-is-i2c.c
@@ -130,7 +130,7 @@ static int fimc_is_i2c_resume(struct device *dev)
 }
 #endif
 
-static struct dev_pm_ops fimc_is_i2c_pm_ops = {
+static const struct dev_pm_ops fimc_is_i2c_pm_ops = {
SET_RUNTIME_PM_OPS(fimc_is_i2c_runtime_suspend,
fimc_is_i2c_runtime_resume, NULL)
SET_SYSTEM_SLEEP_PM_OPS(fimc_is_i2c_suspend, fimc_is_i2c_resume)
-- 
1.9.1



Re: [PATCH 1/1] docs-rst: media: Document s_stream() core op usage

2017-06-29 Thread Sakari Ailus
Hejssan!

On Thu, Jun 29, 2017 at 09:21:53AM +0200, Niklas Söderlund wrote:
> Hi Sakari,
> 
> Thanks for your patch.
> 
> On 2017-06-29 09:38:53 +0300, Sakari Ailus wrote:
> > As we begin to add support for systems with media pipelines controlled by
> > more than one device driver, it is essential that we precisely define the
> > responsibilities of each component in the stream control and common
> > practices.
> > 
> > Specifically, this patch documents two things:
> > 
> > 1) streaming control is done per sub-device and sub-device drivers
> > themselves are responsible for streaming setup in upstream sub-devices and
> > 
> > 2) broken frames should be tolerated at streaming stop. It is the
> > responsibility of the sub-device driver to stop the transmitter after
> > itself if it cannot handle broken frames (and it should be probably be
> > done anyway).
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  Documentation/media/kapi/v4l2-subdev.rst | 36 
> > 
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/Documentation/media/kapi/v4l2-subdev.rst 
> > b/Documentation/media/kapi/v4l2-subdev.rst
> > index e1f0b72..27e371e 100644
> > --- a/Documentation/media/kapi/v4l2-subdev.rst
> > +++ b/Documentation/media/kapi/v4l2-subdev.rst
> > @@ -262,6 +262,42 @@ is called. After all subdevices have been located the 
> > .complete() callback is
> >  called. When a subdevice is removed from the system the .unbind() method is
> >  called. All three callbacks are optional.
> >  
> > +Streaming control
> > +^
> > +
> > +Starting and stopping the stream are somewhat complex operations that
> > +often require walking the media graph to enable streaming on
> > +sub-devices which the pipeline consists of. This involves interaction
> > +between multiple drivers, sometimes more than two.
> > +
> > +The ``.s_stream()`` core op is responsible for starting and stopping
> 
> .s_stream() is a video or audio op not a core op right? But at the same 
> time it's all part of the v4l2 core :-) Apart from this nit-pick which 
> I'm fine to leave as is at your leisure.
> 
> Acked-by: Niklas Söderlund 

Good point. The audio s_stream() op appears to be much less used and not by
MC enabled drivers.

I'll fix that, as well as add a proper reference to struct
v4l2_subdev_video_ops.

-- 
Hälsningar,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk


[PATCH 2/2] v4l: fwnode: Support generic parsing of graph endpoints in V4L2

2017-06-29 Thread Sakari Ailus
The current practice is that drivers iterate over their endpoints and
parse each endpoint separately. This is very similar in a number of
drivers, implement a generic function for the job. Driver specific matters
can be taken into account in the driver specific callback.

Convert the omap3isp as an example.

Signed-off-by: Sakari Ailus 
---
 drivers/media/platform/omap3isp/isp.c | 91 ++---
 drivers/media/platform/omap3isp/isp.h |  3 --
 drivers/media/v4l2-core/v4l2-fwnode.c | 94 +++
 include/media/v4l2-async.h|  4 +-
 include/media/v4l2-fwnode.h   |  9 
 5 files changed, 132 insertions(+), 69 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c 
b/drivers/media/platform/omap3isp/isp.c
index 9df64c1..9ccf883 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2008,43 +2008,42 @@ enum isp_of_phy {
ISP_OF_PHY_CSIPHY2,
 };
 
-static int isp_fwnode_parse(struct device *dev, struct fwnode_handle *fwnode,
-   struct isp_async_subdev *isd)
+static int isp_fwnode_parse(struct device *dev,
+   struct v4l2_fwnode_endpoint *vep,
+   struct v4l2_async_subdev *asd)
 {
+   struct isp_async_subdev *isd =
+   container_of(asd, struct isp_async_subdev, asd);
struct isp_bus_cfg *buscfg = >bus;
-   struct v4l2_fwnode_endpoint vep;
unsigned int i;
-   int ret;
-
-   ret = v4l2_fwnode_endpoint_parse(fwnode, );
-   if (ret)
-   return ret;
 
dev_dbg(dev, "parsing endpoint %s, interface %u\n",
-   to_of_node(fwnode)->full_name, vep.base.port);
+   to_of_node(vep->base.local_fwnode)->full_name, vep->base.port);
 
-   switch (vep.base.port) {
+   switch (vep->base.port) {
case ISP_OF_PHY_PARALLEL:
buscfg->interface = ISP_INTERFACE_PARALLEL;
buscfg->bus.parallel.data_lane_shift =
-   vep.bus.parallel.data_shift;
+   vep->bus.parallel.data_shift;
buscfg->bus.parallel.clk_pol =
-   !!(vep.bus.parallel.flags
+   !!(vep->bus.parallel.flags
   & V4L2_MBUS_PCLK_SAMPLE_FALLING);
buscfg->bus.parallel.hs_pol =
-   !!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
+   !!(vep->bus.parallel.flags &
+  V4L2_MBUS_VSYNC_ACTIVE_LOW);
buscfg->bus.parallel.vs_pol =
-   !!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
+   !!(vep->bus.parallel.flags &
+  V4L2_MBUS_HSYNC_ACTIVE_LOW);
buscfg->bus.parallel.fld_pol =
-   !!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
+   !!(vep->bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
buscfg->bus.parallel.data_pol =
-   !!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
+   !!(vep->bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
break;
 
case ISP_OF_PHY_CSIPHY1:
case ISP_OF_PHY_CSIPHY2:
/* FIXME: always assume CSI-2 for now. */
-   switch (vep.base.port) {
+   switch (vep->base.port) {
case ISP_OF_PHY_CSIPHY1:
buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
break;
@@ -2052,18 +2051,19 @@ static int isp_fwnode_parse(struct device *dev, struct 
fwnode_handle *fwnode,
buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
break;
}
-   buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
+   buscfg->bus.csi2.lanecfg.clk.pos =
+   vep->bus.mipi_csi2.clock_lane;
buscfg->bus.csi2.lanecfg.clk.pol =
-   vep.bus.mipi_csi2.lane_polarities[0];
+   vep->bus.mipi_csi2.lane_polarities[0];
dev_dbg(dev, "clock lane polarity %u, pos %u\n",
buscfg->bus.csi2.lanecfg.clk.pol,
buscfg->bus.csi2.lanecfg.clk.pos);
 
for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
buscfg->bus.csi2.lanecfg.data[i].pos =
-   vep.bus.mipi_csi2.data_lanes[i];
+   vep->bus.mipi_csi2.data_lanes[i];
buscfg->bus.csi2.lanecfg.data[i].pol =
-   vep.bus.mipi_csi2.lane_polarities[i + 1];
+   vep->bus.mipi_csi2.lane_polarities[i + 1];
dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
   

[PATCH 0/2] Unified fwnode endpoint parser

2017-06-29 Thread Sakari Ailus
Hi folks,

We have a large influx of new, unmerged, drivers that are now parsing
fwnode endpoints and each one of them is doing this a little bit
differently. The needs are still exactly the same for the graph data
structure is device independent. This is still a non-trivial task and the
majority of the driver implementations are buggy, just buggy in different
ways.

Facilitate parsing endpoints by adding a convenience function for parsing
the endpoints, and make the omap3isp driver use it as an example.

The parser succeeds an essential bugfix in the set.

Sakari Ailus (2):
  v4l: fwnode: link_frequency is an optional property
  v4l: fwnode: Support generic parsing of graph endpoints in V4L2

 drivers/media/platform/omap3isp/isp.c |  91 +++--
 drivers/media/platform/omap3isp/isp.h |   3 -
 drivers/media/v4l2-core/v4l2-fwnode.c | 124 ++
 include/media/v4l2-async.h|   4 +-
 include/media/v4l2-fwnode.h   |   9 +++
 5 files changed, 147 insertions(+), 84 deletions(-)

-- 
2.1.4



[PATCH 1/2] v4l: fwnode: link_frequency is an optional property

2017-06-29 Thread Sakari Ailus
v4l2_fwnode_endpoint_alloc_parse() is intended as a replacement for
v4l2_fwnode_endpoint_parse(). It parses the "link-frequency" property and
if the property isn't found, it returns an error. However,
"link-frequency" is an optional property and if it does not exist is not
an error. Instead, the number of link frequencies is simply zero in that
case.

Signed-off-by: Sakari Ailus 
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
b/drivers/media/v4l2-core/v4l2-fwnode.c
index 153c53c..0ec6c14 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -247,23 +247,23 @@ struct v4l2_fwnode_endpoint 
*v4l2_fwnode_endpoint_alloc_parse(
 
rval = fwnode_property_read_u64_array(fwnode, "link-frequencies",
  NULL, 0);
-   if (rval < 0)
-   goto out_err;
-
-   vep->link_frequencies =
-   kmalloc_array(rval, sizeof(*vep->link_frequencies), GFP_KERNEL);
-   if (!vep->link_frequencies) {
-   rval = -ENOMEM;
-   goto out_err;
-   }
+   if (rval > 0) {
+   vep->link_frequencies =
+   kmalloc_array(rval, sizeof(*vep->link_frequencies),
+ GFP_KERNEL);
+   if (!vep->link_frequencies) {
+   rval = -ENOMEM;
+   goto out_err;
+   }
 
-   vep->nr_of_link_frequencies = rval;
+   vep->nr_of_link_frequencies = rval;
 
-   rval = fwnode_property_read_u64_array(fwnode, "link-frequencies",
- vep->link_frequencies,
- vep->nr_of_link_frequencies);
-   if (rval < 0)
-   goto out_err;
+   rval = fwnode_property_read_u64_array(
+   fwnode, "link-frequencies", vep->link_frequencies,
+   vep->nr_of_link_frequencies);
+   if (rval < 0)
+   goto out_err;
+   }
 
return vep;
 
-- 
2.1.4



Re: [PATCH 1/1] docs-rst: media: Document s_stream() core op usage

2017-06-29 Thread Niklas Söderlund
Hi Sakari,

Thanks for your patch.

On 2017-06-29 09:38:53 +0300, Sakari Ailus wrote:
> As we begin to add support for systems with media pipelines controlled by
> more than one device driver, it is essential that we precisely define the
> responsibilities of each component in the stream control and common
> practices.
> 
> Specifically, this patch documents two things:
> 
> 1) streaming control is done per sub-device and sub-device drivers
> themselves are responsible for streaming setup in upstream sub-devices and
> 
> 2) broken frames should be tolerated at streaming stop. It is the
> responsibility of the sub-device driver to stop the transmitter after
> itself if it cannot handle broken frames (and it should be probably be
> done anyway).
> 
> Signed-off-by: Sakari Ailus 
> ---
>  Documentation/media/kapi/v4l2-subdev.rst | 36 
> 
>  1 file changed, 36 insertions(+)
> 
> diff --git a/Documentation/media/kapi/v4l2-subdev.rst 
> b/Documentation/media/kapi/v4l2-subdev.rst
> index e1f0b72..27e371e 100644
> --- a/Documentation/media/kapi/v4l2-subdev.rst
> +++ b/Documentation/media/kapi/v4l2-subdev.rst
> @@ -262,6 +262,42 @@ is called. After all subdevices have been located the 
> .complete() callback is
>  called. When a subdevice is removed from the system the .unbind() method is
>  called. All three callbacks are optional.
>  
> +Streaming control
> +^
> +
> +Starting and stopping the stream are somewhat complex operations that
> +often require walking the media graph to enable streaming on
> +sub-devices which the pipeline consists of. This involves interaction
> +between multiple drivers, sometimes more than two.
> +
> +The ``.s_stream()`` core op is responsible for starting and stopping

.s_stream() is a video or audio op not a core op right? But at the same 
time it's all part of the v4l2 core :-) Apart from this nit-pick which 
I'm fine to leave as is at your leisure.

Acked-by: Niklas Söderlund 

> +the stream on the sub-device it is called on. Additionally, if there
> +are sub-devices further up in the pipeline, i.e. connected to that
> +sub-device's sink pads through enabled links, the sub-device driver
> +must call the ``.s_stream()`` core op of all such sub-devices. The
> +sub-device driver is thus in control of whether the upstream
> +sub-devices start (or stop) streaming before or after the sub-device
> +itself is set up for streaming.
> +
> +.. note::
> +
> +   As the ``.s_stream()`` callback is called recursively through the
> +   sub-devices along the pipeline, it is important to keep the
> +   recursion as short as possible. To this end, drivers are encouraged
> +   not to internally call ``.s_stream()`` recursively in order to make
> +   only a single additional recursion per driver in the pipeline. This
> +   greatly reduces stack usage.
> +
> +Stopping the transmitter
> +
> +
> +A transmitter stops sending the stream of images as a result of
> +calling the ``.s_stream()`` callback. Some transmitters may stop the
> +stream at a frame boundary whereas others stop immediately,
> +effectively leaving the current frame unfinished. The receiver driver
> +should not make assumptions either way, but function properly in both
> +cases.
> +
>  V4L2 sub-device userspace API
>  -
>  
> -- 
> 2.7.4
> 

-- 
Regards,
Niklas Söderlund


[PATCH] [media]: usb: add const to v4l2_file_operations structures

2017-06-29 Thread Bhumika Goyal
Declare v4l2_file_operations structures as const as they are only stored
in the fops field of video_device structures. This field is of type
const, so declare v4l2_file_operations structures with similar properties
as const.

Signed-off-by: Bhumika Goyal 
---
 drivers/media/usb/au0828/au0828-video.c  | 2 +-
 drivers/media/usb/cx231xx/cx231xx-417.c  | 2 +-
 drivers/media/usb/go7007/go7007-v4l2.c   | 2 +-
 drivers/media/usb/gspca/gspca.c  | 2 +-
 drivers/media/usb/stk1160/stk1160-v4l.c  | 2 +-
 drivers/media/usb/stkwebcam/stk-webcam.c | 2 +-
 drivers/media/usb/tm6000/tm6000-video.c  | 2 +-
 7 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c 
b/drivers/media/usb/au0828/au0828-video.c
index 2a255bd..9342402 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -1740,7 +1740,7 @@ void au0828_v4l2_resume(struct au0828_dev *dev)
}
 }
 
-static struct v4l2_file_operations au0828_v4l_fops = {
+static const struct v4l2_file_operations au0828_v4l_fops = {
.owner  = THIS_MODULE,
.open   = au0828_v4l2_open,
.release= au0828_v4l2_close,
diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c 
b/drivers/media/usb/cx231xx/cx231xx-417.c
index 509d971..8d5eb99 100644
--- a/drivers/media/usb/cx231xx/cx231xx-417.c
+++ b/drivers/media/usb/cx231xx/cx231xx-417.c
@@ -1843,7 +1843,7 @@ static int mpeg_mmap(struct file *file, struct 
vm_area_struct *vma)
return videobuf_mmap_mapper(>vidq, vma);
 }
 
-static struct v4l2_file_operations mpeg_fops = {
+static const struct v4l2_file_operations mpeg_fops = {
.owner = THIS_MODULE,
.open  = mpeg_open,
.release   = mpeg_release,
diff --git a/drivers/media/usb/go7007/go7007-v4l2.c 
b/drivers/media/usb/go7007/go7007-v4l2.c
index ed5ec97..445f17b 100644
--- a/drivers/media/usb/go7007/go7007-v4l2.c
+++ b/drivers/media/usb/go7007/go7007-v4l2.c
@@ -857,7 +857,7 @@ static int go7007_s_ctrl(struct v4l2_ctrl *ctrl)
return 0;
 }
 
-static struct v4l2_file_operations go7007_fops = {
+static const struct v4l2_file_operations go7007_fops = {
.owner  = THIS_MODULE,
.open   = v4l2_fh_open,
.release= vb2_fop_release,
diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 16bc1dd..0f14176 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -1964,7 +1964,7 @@ static ssize_t dev_read(struct file *file, char __user 
*data,
return ret;
 }
 
-static struct v4l2_file_operations dev_fops = {
+static const struct v4l2_file_operations dev_fops = {
.owner = THIS_MODULE,
.open = dev_open,
.release = dev_close,
diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c 
b/drivers/media/usb/stk1160/stk1160-v4l.c
index a005d26..a132faa 100644
--- a/drivers/media/usb/stk1160/stk1160-v4l.c
+++ b/drivers/media/usb/stk1160/stk1160-v4l.c
@@ -326,7 +326,7 @@ static int stk1160_stop_streaming(struct stk1160 *dev)
return 0;
 }
 
-static struct v4l2_file_operations stk1160_fops = {
+static const struct v4l2_file_operations stk1160_fops = {
.owner = THIS_MODULE,
.open = v4l2_fh_open,
.release = vb2_fop_release,
diff --git a/drivers/media/usb/stkwebcam/stk-webcam.c 
b/drivers/media/usb/stkwebcam/stk-webcam.c
index 90d4a08..93330be 100644
--- a/drivers/media/usb/stkwebcam/stk-webcam.c
+++ b/drivers/media/usb/stkwebcam/stk-webcam.c
@@ -1202,7 +1202,7 @@ static const struct v4l2_ctrl_ops stk_ctrl_ops = {
.s_ctrl = stk_s_ctrl,
 };
 
-static struct v4l2_file_operations v4l_stk_fops = {
+static const struct v4l2_file_operations v4l_stk_fops = {
.owner = THIS_MODULE,
.open = v4l_stk_open,
.release = v4l_stk_release,
diff --git a/drivers/media/usb/tm6000/tm6000-video.c 
b/drivers/media/usb/tm6000/tm6000-video.c
index 7e960d0..cec1321 100644
--- a/drivers/media/usb/tm6000/tm6000-video.c
+++ b/drivers/media/usb/tm6000/tm6000-video.c
@@ -1532,7 +1532,7 @@ static int tm6000_mmap(struct file *file, struct 
vm_area_struct * vma)
return res;
 }
 
-static struct v4l2_file_operations tm6000_fops = {
+static const struct v4l2_file_operations tm6000_fops = {
.owner = THIS_MODULE,
.open = tm6000_open,
.release = tm6000_release,
-- 
2.7.4



[PATCH 1/1] docs-rst: media: Document s_stream() core op usage

2017-06-29 Thread Sakari Ailus
As we begin to add support for systems with media pipelines controlled by
more than one device driver, it is essential that we precisely define the
responsibilities of each component in the stream control and common
practices.

Specifically, this patch documents two things:

1) streaming control is done per sub-device and sub-device drivers
themselves are responsible for streaming setup in upstream sub-devices and

2) broken frames should be tolerated at streaming stop. It is the
responsibility of the sub-device driver to stop the transmitter after
itself if it cannot handle broken frames (and it should be probably be
done anyway).

Signed-off-by: Sakari Ailus 
---
 Documentation/media/kapi/v4l2-subdev.rst | 36 
 1 file changed, 36 insertions(+)

diff --git a/Documentation/media/kapi/v4l2-subdev.rst 
b/Documentation/media/kapi/v4l2-subdev.rst
index e1f0b72..27e371e 100644
--- a/Documentation/media/kapi/v4l2-subdev.rst
+++ b/Documentation/media/kapi/v4l2-subdev.rst
@@ -262,6 +262,42 @@ is called. After all subdevices have been located the 
.complete() callback is
 called. When a subdevice is removed from the system the .unbind() method is
 called. All three callbacks are optional.
 
+Streaming control
+^
+
+Starting and stopping the stream are somewhat complex operations that
+often require walking the media graph to enable streaming on
+sub-devices which the pipeline consists of. This involves interaction
+between multiple drivers, sometimes more than two.
+
+The ``.s_stream()`` core op is responsible for starting and stopping
+the stream on the sub-device it is called on. Additionally, if there
+are sub-devices further up in the pipeline, i.e. connected to that
+sub-device's sink pads through enabled links, the sub-device driver
+must call the ``.s_stream()`` core op of all such sub-devices. The
+sub-device driver is thus in control of whether the upstream
+sub-devices start (or stop) streaming before or after the sub-device
+itself is set up for streaming.
+
+.. note::
+
+   As the ``.s_stream()`` callback is called recursively through the
+   sub-devices along the pipeline, it is important to keep the
+   recursion as short as possible. To this end, drivers are encouraged
+   not to internally call ``.s_stream()`` recursively in order to make
+   only a single additional recursion per driver in the pipeline. This
+   greatly reduces stack usage.
+
+Stopping the transmitter
+
+
+A transmitter stops sending the stream of images as a result of
+calling the ``.s_stream()`` callback. Some transmitters may stop the
+stream at a frame boundary whereas others stop immediately,
+effectively leaving the current frame unfinished. The receiver driver
+should not make assumptions either way, but function properly in both
+cases.
+
 V4L2 sub-device userspace API
 -
 
-- 
2.7.4



Re: [PATCH v2 09/19] media: camms: Add core files

2017-06-29 Thread Sakari Ailus
Hi Todor,

On Mon, Jun 19, 2017 at 05:48:29PM +0300, Todor Tomov wrote:
> These files implement the platform driver code.
> 
> Signed-off-by: Todor Tomov 
> ---
>  drivers/media/platform/qcom/camss-8x16/camss.c | 630 
> +
>  drivers/media/platform/qcom/camss-8x16/camss.h |  96 
>  2 files changed, 726 insertions(+)
>  create mode 100644 drivers/media/platform/qcom/camss-8x16/camss.c
>  create mode 100644 drivers/media/platform/qcom/camss-8x16/camss.h
> 
> diff --git a/drivers/media/platform/qcom/camss-8x16/camss.c 
> b/drivers/media/platform/qcom/camss-8x16/camss.c
> new file mode 100644
> index 000..a8798d1
> --- /dev/null
> +++ b/drivers/media/platform/qcom/camss-8x16/camss.c
> @@ -0,0 +1,630 @@
> +/*
> + * camss.c
> + *
> + * Qualcomm MSM Camera Subsystem - Core
> + *
> + * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2015-2016 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only 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 
> +#include 
> +#include 
> +#include 
> +
> +#include "camss.h"
> +
> +static struct resources csiphy_res[] = {
> + /* CSIPHY0 */
> + {
> + .regulator = { NULL },
> + .clock = { "camss_top_ahb_clk", "ispif_ahb_clk",
> +"camss_ahb_clk", "csiphy0_timer_clk" },
> + .clock_rate = { 0, 0, 0, 2 },
> + .reg = { "csiphy0", "csiphy0_clk_mux" },
> + .interrupt = { "csiphy0" }
> + },
> +
> + /* CSIPHY1 */
> + {
> + .regulator = { NULL },
> + .clock = { "camss_top_ahb_clk", "ispif_ahb_clk",
> +"camss_ahb_clk", "csiphy1_timer_clk" },
> + .clock_rate = { 0, 0, 0, 2 },
> + .reg = { "csiphy1", "csiphy1_clk_mux" },
> + .interrupt = { "csiphy1" }
> + }
> +};
> +
> +static struct resources csid_res[] = {
> + /* CSID0 */
> + {
> + .regulator = { "vdda" },
> + .clock = { "camss_top_ahb_clk", "ispif_ahb_clk",
> +"csi0_ahb_clk", "camss_ahb_clk",
> +"csi0_clk", "csi0_phy_clk",
> +"csi0_pix_clk", "csi0_rdi_clk" },
> + .clock_rate = { 0, 0, 0, 0, 2, 0, 0, 0 },
> + .reg = { "csid0" },
> + .interrupt = { "csid0" }
> + },
> +
> + /* CSID1 */
> + {
> + .regulator = { "vdda" },
> + .clock = { "camss_top_ahb_clk", "ispif_ahb_clk",
> +"csi1_ahb_clk", "camss_ahb_clk",
> +"csi1_clk", "csi1_phy_clk",
> +"csi1_pix_clk", "csi1_rdi_clk" },
> + .clock_rate = { 0, 0, 0, 0, 2, 0, 0, 0 },
> + .reg = { "csid1" },
> + .interrupt = { "csid1" }
> + },
> +};
> +
> +static struct resources_ispif ispif_res = {
> + /* ISPIF */
> + .clock = { "camss_top_ahb_clk", "camss_ahb_clk", "ispif_ahb_clk",
> +"csi0_clk", "csi0_pix_clk", "csi0_rdi_clk",
> +"csi1_clk", "csi1_pix_clk", "csi1_rdi_clk" },
> + .clock_for_reset = { "camss_vfe_vfe_clk", "camss_csi_vfe_clk" },
> + .reg = { "ispif", "csi_clk_mux" },
> + .interrupt = "ispif"
> +
> +};
> +
> +static struct resources vfe_res = {
> + /* VFE0 */
> + .regulator = { NULL },
> + .clock = { "camss_top_ahb_clk", "camss_vfe_vfe_clk",
> +"camss_csi_vfe_clk", "iface_clk",
> +"bus_clk", "camss_ahb_clk" },
> + .clock_rate = { 0, 32000, 0, 0, 0, 0, 0, 0 },
> + .reg = { "vfe0" },
> + .interrupt = { "vfe0" }
> +};

Could these be const?

> +
> +/*
> + * camss_enable_clocks - Enable multiple clocks
> + * @nclocks: Number of clocks in clock array
> + * @clock: Clock array
> + * @dev: Device
> + *
> + * Return 0 on success or a negative error code otherwise
> + */
> +int camss_enable_clocks(int nclocks, struct clk **clock, struct device *dev)
> +{
> + int ret;
> + int i;
> +
> + for (i = 0; i < nclocks; i++) {
> + ret = clk_prepare_enable(clock[i]);
> + if (ret) {
> + dev_err(dev, "clock enable failed\n");
> + goto error;
> + }
> + }
> +
> + return 0;
> +
> +error:
> + for (i--; i >= 0; i--)
> + clk_disable_unprepare(clock[i]);
> +
> + return ret;
> +}
> +
> +/*
> + *