Re: [PATCH v4 5/5] [media] MAINTAINERS: add entries for OV2685/OV5695 sensor drivers

[Sakari Ailus]

On Fri, Jan 12, 2018 at 09:52:13AM +0800, Shunqian Zheng wrote:
> Hi Sakari,
> 
> 
> On 2018年01月10日 06:33, Sakari Ailus wrote:
> > On Tue, Jan 09, 2018 at 10:48:24PM +0800, Shunqian Zheng wrote:
> > > Add maintainer entries for the OV2685 and OV5695 V4L2 sensor drivers.
> > > 
> > > Signed-off-by: Shunqian Zheng 
> > Same patch with the driver, please.
> > 
> > Other than that seems good to me.
> Does that mean I can add your Reviewed-by in the new patches?

Presumably you'll have one patch less in v5. I'll still check the rest of
the patches.

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

[no subject]

[Al Thomas]

Salutations 


https://goo.gl/Pbzrv4



https://goo.gl/csrJzP




Al

Re: [PATCH v4 1/9] dt-bindings: media: Add Renesas CEU bindings

[Simon Horman]

On Fri, Jan 12, 2018 at 12:50:41AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> Thank you for the patch.
> 
> On Tuesday, 9 January 2018 18:25:23 EET Jacopo Mondi wrote:
> > Add bindings documentation for Renesas Capture Engine Unit (CEU).
> > 
> > Signed-off-by: Jacopo Mondi 
> 
> Reviewed-by: Laurent Pinchart 

Hi,

I see that these bindings have now been reviewed. What is their (likely)
path to upstream from here? I'd like to accept the related DTS changes
once there is a clear path for the bindings to land in upstream.

cron job: media_tree daily build: ERRORS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Fri Jan 12 05:00:15 CET 2018
media-tree git hash:e3ee691dbf24096ea51b3200946b11d68ce75361
media_build git hash:   46c9dc0a08499791cedfc7ee0df387e475f075a2
v4l-utils git hash: 351eb68ac235f37f749a1c5d6ed2fae80e9dffe3
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3911-g6f737e1f
smatch version: v0.5.0-3911-g6f737e1f
host hardware:  x86_64
host os:4.13.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: WARNINGS
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: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9.26-i686: ERRORS
linux-4.10.14-i686: WARNINGS
linux-4.11-i686: WARNINGS
linux-4.12.1-i686: WARNINGS
linux-4.13-i686: WARNINGS
linux-4.14-i686: WARNINGS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: 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: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.26-x86_64: ERRORS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: WARNINGS
linux-4.14-x86_64: WARNINGS
apps: OK
spec-git: OK
smatch: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support

[Yong]

Hi Maxime,

On Thu, 11 Jan 2018 13:40:18 +0100
Maxime Ripard  wrote:

> Hi Yong,
> 
> On Thu, Jan 11, 2018 at 09:15:08AM +0800, Yong wrote:
> > > On Mon, Jan 08, 2018 at 05:13:39PM +, Hugues FRUCHET wrote:
> > > > I'm using a ST board with OV5640 wired in parallel bus output in order 
> > > > to interface to my STM32 DCMI parallel interface.
> > > > Perhaps could you describe your setup so I could help on understanding 
> > > > the problem on your side. From my past experience with this sensor 
> > > > module, you can first check hsync/vsync polarities, the datasheet is 
> > > > buggy on VSYNC polarity as documented in patch 4/5.
> > > 
> > > It turns out that it was indeed a polarity issue.
> > > 
> > > It looks like that in order to operate properly, I need to setup the
> > > opposite polarity on HSYNC and VSYNC on the interface. I looked at the
> > > signals under a scope, and VSYNC is obviously inversed as you
> > > described. HSYNC, I'm not so sure since the HBLANK period seems very
> > > long, almost a line.
> > > 
> > > Since VSYNC at least looks correct, I'd be inclined to think that the
> > > polarity is inversed on at least the SoC I'm using it on.
> > > 
> > > Yong, did you test the V3S CSI driver with a parallel interface? With
> > > what sensor driver? Have you found some polarities issues like this?
> > 
> > Did you try it with Allwinner SoCs?
> 
> Yes, on an H3. Looking at all the Allwinner datasheet I could get my
> hands on, they are all documented in the same way. However, I really
> start to wonder whether the polarity shouldn't be reversed.
> 
> At least the fact that VSYNC is clearly active low on the
> oscilloscope, while I have to set it active high in the controller
> seems like a strong hint :)

The BSP code of Allwinner also treat V4L2_MBUS_VSYNC_ACTIVE_HIGH as
they documented 'positive'.
Maybe there need some more tests to confirm if the datasheet and BSP
code are both wrong.

> 
> > No. I only tested with a BT1120 signal generated by FPGA or ADV7611. HSYNC
> > and VSYNC are not used.
> 
> Ok, that's good to know :)
> 
> > For V3s CSI driver, I will add the following to dt-bindings:
> > Endpoint node properties for CSI1
> > -
> > 
> > - remote-endpoint  : (required) a phandle to the bus receiver's endpoint
> >   node
> > - bus-width:   : (required) must be 8, 10, 12 or 16
> > - pclk-sample  : (optional) (default: sample on falling edge)
> > - hsync-active : (only required for parallel)
> > - vsync-active : (only required for parallel)
> > 
> > You could try diffrent hsync-active/vsync-active values here.
> 
> I did already, and the only combination that works is the one that is
> the inversed polarity on HSYNC and VSYNC than what the sensor setup.
> 
> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com


Thanks,
Yong

Re: [PATCH v4 5/5] [media] MAINTAINERS: add entries for OV2685/OV5695 sensor drivers

[Shunqian Zheng]

Hi Sakari,


On 2018年01月10日 06:33, Sakari Ailus wrote:

On Tue, Jan 09, 2018 at 10:48:24PM +0800, Shunqian Zheng wrote:

Add maintainer entries for the OV2685 and OV5695 V4L2 sensor drivers.

Signed-off-by: Shunqian Zheng 

Same patch with the driver, please.

Other than that seems good to me.

Does that mean I can add your Reviewed-by in the new patches?

Thank you very much~

Re: [linux-sunxi] Re: [PATCH v5 2/2] media: V3s: Add support for Allwinner CSI.

[Yong]

Hi Maxime,

On Thu, 11 Jan 2018 14:28:44 +0100
Maxime Ripard  wrote:

> Hi Yong,
> 
> On Thu, Jan 11, 2018 at 11:06:06AM +0800, Yong Deng wrote:
> > Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
> > interface and CSI1 is used for parallel interface. This is not
> > documented in datasheet but by test and guess.
> > 
> > This patch implement a v4l2 framework driver for it.
> > 
> > Currently, the driver only support the parallel interface. MIPI-CSI2,
> > ISP's support are not included in this patch.
> > 
> > Signed-off-by: Yong Deng 
> 
> I've needed this patch in order to fix a NULL pointer dereference:
> http://code.bulix.org/oz6gmb-257359?raw
> 
> This is needed because while it's ok to have a NULL pointer to
> v4l2_subdev_pad_config when you call the subdev set_fmt with
> V4L2_SUBDEV_FORMAT_ACTIVE, it's not with V4L2_SUBDEV_FORMAT_TRY, and
> sensors will assume taht it's a valid pointer.
> 
> Otherwise,
> Tested-by: Maxime Ripard 

I revisit some code of subdevs and you are right.

Squash your patch into my driver patch and add your Tested-by in
commit. Is it right?

> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-sunxi+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.


Thanks,
Yong

Re: [PATCH v2 00/19] prevent bounds-check bypass via speculative execution

[Dan Williams]

On Thu, Jan 11, 2018 at 5:19 PM, Linus Torvalds
 wrote:
> On Thu, Jan 11, 2018 at 4:46 PM, Dan Williams  
> wrote:
>>
>> This series incorporates Mark Rutland's latest ARM changes and adds
>> the x86 specific implementation of 'ifence_array_ptr'. That ifence
>> based approach is provided as an opt-in fallback, but the default
>> mitigation, '__array_ptr', uses a 'mask' approach that removes
>> conditional branches instructions, and otherwise aims to redirect
>> speculation to use a NULL pointer rather than a user controlled value.
>
> Do you have any performance numbers and perhaps example code
> generation? Is this noticeable? Are there any microbenchmarks showing
> the difference between lfence use and the masking model?

I don't have performance numbers, but here's a sample code generation
from __fcheck_files, where the 'and; lea; and' sequence is portion of
array_ptr() after the mask generation with 'sbb'.

fdp = array_ptr(fdt->fd, fd, fdt->max_fds);
 8e7:   8b 02   mov(%rdx),%eax
 8e9:   48 39 c7cmp%rax,%rdi
 8ec:   48 19 c9sbb%rcx,%rcx
 8ef:   48 8b 42 08 mov0x8(%rdx),%rax
 8f3:   48 89 femov%rdi,%rsi
 8f6:   48 21 ceand%rcx,%rsi
 8f9:   48 8d 04 f0 lea(%rax,%rsi,8),%rax
 8fd:   48 21 c8and%rcx,%rax


> Having both seems good for testing, but wouldn't we want to pick one in the 
> end?

I was thinking we'd keep it as a 'just in case' sort of thing, at
least until the 'probably safe' assumption of the 'mask' approach has
more time to settle out.

>
> Also, I do think that there is one particular array load that would
> seem to be pretty obvious: the system call function pointer array.
>
> Yes, yes, the actual call is now behind a retpoline, but that protects
> against a speculative BTB access, it's not obvious that it  protects
> against the mispredict of the __NR_syscall_max comparison in
> arch/x86/entry/entry_64.S.
>
> The act of fetching code is a kind of read too. And retpoline protects
> against BTB stuffing etc, but what if the _actual_ system call
> function address is wrong (due to mis-prediction of the system call
> index check)?
>
> Should the array access in entry_SYSCALL_64_fastpath be made to use
> the masking approach?

I'll take a look. I'm firmly in the 'patch first / worry later' stance
on these investigations.

Re: [PATCH v2 00/19] prevent bounds-check bypass via speculative execution

[Linus Torvalds]

On Thu, Jan 11, 2018 at 4:46 PM, Dan Williams  wrote:
>
> This series incorporates Mark Rutland's latest ARM changes and adds
> the x86 specific implementation of 'ifence_array_ptr'. That ifence
> based approach is provided as an opt-in fallback, but the default
> mitigation, '__array_ptr', uses a 'mask' approach that removes
> conditional branches instructions, and otherwise aims to redirect
> speculation to use a NULL pointer rather than a user controlled value.

Do you have any performance numbers and perhaps example code
generation? Is this noticeable? Are there any microbenchmarks showing
the difference between lfence use and the masking model?

Having both seems good for testing, but wouldn't we want to pick one in the end?

Also, I do think that there is one particular array load that would
seem to be pretty obvious: the system call function pointer array.

Yes, yes, the actual call is now behind a retpoline, but that protects
against a speculative BTB access, it's not obvious that it  protects
against the mispredict of the __NR_syscall_max comparison in
arch/x86/entry/entry_64.S.

The act of fetching code is a kind of read too. And retpoline protects
against BTB stuffing etc, but what if the _actual_ system call
function address is wrong (due to mis-prediction of the system call
index check)?

Should the array access in entry_SYSCALL_64_fastpath be made to use
the masking approach?

Linus

[PATCH v2 00/19] prevent bounds-check bypass via speculative execution

[Dan Williams]

Changes since v1 [1]:
* fixup the ifence definition to use alternative_2 per recent AMD
  changes in tip/x86/pti (Tom)

* drop 'nospec_ptr' (Linus, Mark)

* rename 'nospec_array_ptr' to 'array_ptr' (Alexei)

* rename 'nospec_barrier' to 'ifence' (Peter, Ingo)

* clean up occasions of 'variable assignment in if()' (Sergei, Stephen)

* make 'array_ptr' use a mask instead of an architectural ifence by
  default (Linus, Alexei)

* provide a command line and compile-time opt-in to the ifence
  mechanism, if an architecture provides 'ifence_array_ptr'.

* provide an optimized mask generation helper, 'array_ptr_mask', for
  x86 (Linus)

* move 'get_user' hardening from '__range_not_ok' to '__uaccess_begin'
  (Linus)

* drop "Thermal/int340x: prevent bounds-check..." since userspace does
  not have arbitrary control over the 'trip' index (Srinivas)

* update the changelog of "net: mpls: prevent bounds-check..." and keep
  it in the series to continue the debate about Spectre hygiene patches.
  (Eric).

* record a reviewed-by from Laurent on "[media] uvcvideo: prevent
  bounds-check..."

* update the cover letter

[1]: https://lwn.net/Articles/743376/

---

Quoting Mark's original RFC:

"Recently, Google Project Zero discovered several classes of attack
against speculative execution. One of these, known as variant-1, allows
explicit bounds checks to be bypassed under speculation, providing an
arbitrary read gadget. Further details can be found on the GPZ blog [2]
and the Documentation patch in this series."

This series incorporates Mark Rutland's latest ARM changes and adds
the x86 specific implementation of 'ifence_array_ptr'. That ifence
based approach is provided as an opt-in fallback, but the default
mitigation, '__array_ptr', uses a 'mask' approach that removes
conditional branches instructions, and otherwise aims to redirect
speculation to use a NULL pointer rather than a user controlled value.

The mask is generated by the following from Alexei, and Linus:

mask = ~(long)(_i | (_s - 1 - _i)) >> (BITS_PER_LONG - 1);

...and Linus provided an optimized mask generation helper for x86:

asm ("cmpq %1,%2; sbbq %0,%0;"
:"=r" (mask)
:"r"(sz),"r" (idx)
:"cc");

The 'array_ptr' mechanism can be switched between 'mask' and 'ifence'
via the spectre_v1={mask,ifence} command line option, and the
compile-time default is set by selecting either CONFIG_SPECTRE1_MASK or
CONFIG_SPECTRE1_IFENCE.

The 'array_ptr' infrastructure is the primary focus this patch set. The
individual patches that perform 'array_ptr' conversions are a point in
time (i.e. earlier kernel, early analysis tooling, x86 only etc...)
start at finding some of these gadgets.

Another consideration for reviewing these patches is the 'hygiene'
argument. When a patch refers to hygiene it is concerned with stopping
speculation on an unconstrained or insufficiently constrained pointer
value under userspace control. That by itself is not sufficient for
attack (per current understanding) [3], but it is a necessary
pre-condition.  So 'hygiene' refers to cleaning up those suspect
pointers regardless of whether they are usable as a gadget.

These patches are also be available via the 'nospec-v2' git branch
here:

git://git.kernel.org/pub/scm/linux/kernel/git/djbw/linux nospec-v2

Note that the BPF fix for Spectre variant1 is merged in the bpf.git
tree [4], and is not included in this branch.

[2]: 
https://googleprojectzero.blogspot.co.uk/2018/01/reading-privileged-memory-with-side.html
[3]: https://spectreattack.com/spectre.pdf
[4]: 
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=b2157399cc98

---

Dan Williams (16):
  x86: implement ifence()
  x86: implement ifence_array_ptr() and array_ptr_mask()
  asm-generic/barrier: mask speculative execution flows
  x86: introduce __uaccess_begin_nospec and ASM_IFENCE
  x86: use __uaccess_begin_nospec and ASM_IFENCE in get_user paths
  ipv6: prevent bounds-check bypass via speculative execution
  ipv4: prevent bounds-check bypass via speculative execution
  vfs, fdtable: prevent bounds-check bypass via speculative execution
  userns: prevent bounds-check bypass via speculative execution
  udf: prevent bounds-check bypass via speculative execution
  [media] uvcvideo: prevent bounds-check bypass via speculative execution
  carl9170: prevent bounds-check bypass via speculative execution
  p54: prevent bounds-check bypass via speculative execution
  qla2xxx: prevent bounds-check bypass via speculative execution
  cw1200: prevent bounds-check bypass via speculative execution
  net: mpls: prevent bounds-check bypass via speculative execution

Mark Rutland (3):
  Documentation: document array_ptr
  arm64: implement ifence_array_ptr()
  arm: implement ifence_array_ptr()

 Documentation/speculation.txt|  142 ++
 arch/arm/Kconfig

[PATCH v2 14/19] [media] uvcvideo: prevent bounds-check bypass via speculative execution

[Dan Williams]

Static analysis reports that 'index' may be a user controlled value that
is used as a data dependency to read 'pin' from the
'selector->baSourceID' array. In order to avoid potential leaks of
kernel memory values, block speculative execution of the instruction
stream that could issue reads based on an invalid value of 'pin'.

Based on an original patch by Elena Reshetova.

Laurent notes:

"...as this is nowhere close to being a fast path, I think we can close
this potential hole as proposed in the patch"

Cc: Mauro Carvalho Chehab 
Cc: linux-media@vger.kernel.org
Reviewed-by: Laurent Pinchart 
Signed-off-by: Elena Reshetova 
Signed-off-by: Dan Williams 
---
 drivers/media/usb/uvc/uvc_v4l2.c |9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 3e7e283a44a8..30ee200206ee 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -809,8 +810,12 @@ static int uvc_ioctl_enum_input(struct file *file, void 
*fh,
const struct uvc_entity *selector = chain->selector;
struct uvc_entity *iterm = NULL;
u32 index = input->index;
+   __u8 *elem = NULL;
int pin = 0;
 
+   if (selector)
+   elem = array_ptr(selector->baSourceID, index,
+   selector->bNrInPins);
if (selector == NULL ||
(chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
if (index != 0)
@@ -820,8 +825,8 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
break;
}
pin = iterm->id;
-   } else if (index < selector->bNrInPins) {
-   pin = selector->baSourceID[index];
+   } else if (elem) {
+   pin = *elem;
list_for_each_entry(iterm, >entities, chain) {
if (!UVC_ENTITY_IS_ITERM(iterm))
continue;

MT9M131 on I.MX6DL CSI color issue

[Florian Boor]

Hello all,

I have a Phytec VM-009 camera based on MT9M131 connected to CSI0 of a I.MX6DL
based board running mainline 4.13.0 + custom devicetree. Its using the parallel
interface, 8 bit bus width on pins 12 to 19.

Basically it works pretty well apart from the really strange colors. I guess its
some YUV vs. RGB issue or similar. Here [1] is an example generated with the
following command.

gst-launch v4l2src device=/dev/video4 num-buffers=1 ! jpegenc ! filesink
location=capture1.jpeg

Apart from the colors everything is fine.
I'm pretty sure I have not seen such an effect before - what might be wrong 
here?

The current setup looks like this:

IF=UYVY2X8
GEOM="1280x1024"
media-ctl -l "'mt9m111 2-0048':0 -> 'ipu1_csi0_mux':4[1]"
media-ctl -l "'ipu1_csi0_mux':5 -> 'ipu1_csi0':0[1]"
media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"

media-ctl -d /dev/media0 -v -V "'ipu1_csi0':2 [fmt:${IF}/${GEOM} field:none]"
media-ctl -d /dev/media0 -v -V "'ipu1_csi0 capture':0 [fmt:${IF}/${GEOM}
field:none]"
media-ctl -d /dev/media0 -v -V "'ipu1_csi0_mux':4 [fmt:${IF}/${GEOM} field: 
none]"
media-ctl -d /dev/media0 -v -V "'ipu1_csi0_mux':5 [fmt:${IF}/${GEOM} field: 
none]"
media-ctl -d /dev/media0 -v -V "'mt9m111 2-0048':0 [fmt:${IF}/${GEOM} field: 
none]"


Greetings

Florian

[1] http://www.kernelconcepts.de/~florian/capture1.jpeg

-- 
The dream of yesterday  Florian Boor
is the hope of todayTel: +49 271-771091-15
and the reality of tomorrow.Fax: +49 271-338857-29
[Robert Hutchings Goddard, 1904]florian.b...@kernelconcepts.de
http://www.kernelconcepts.de/en

kernel concepts GmbH
Hauptstraße 16
D-57074 Siegen
Geschäftsführer: Ole Reinhardt
HR Siegen, HR B 9613

Re: [PATCH v4 3/9] v4l: platform: Add Renesas CEU driver

[Laurent Pinchart]

On Tuesday, 9 January 2018 18:25:25 EET Jacopo Mondi wrote:
> Add driver for Renesas Capture Engine Unit (CEU).
> 
> The CEU interface supports capturing 'data' (YUV422) and 'images'
> (NV[12|21|16|61]).
> 
> This driver aims to replace the soc_camera-based sh_mobile_ceu one.
> 
> Tested with ov7670 camera sensor, providing YUYV_2X8 data on Renesas RZ
> platform GR-Peach.
> 
> Tested with ov7725 camera sensor on SH4 platform Migo-R.
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/platform/Kconfig   |9 +
>  drivers/media/platform/Makefile  |1 +
>  drivers/media/platform/renesas-ceu.c | 1648
> ++ 3 files changed, 1658 insertions(+)
>  create mode 100644 drivers/media/platform/renesas-ceu.c

[snip]

> diff --git a/drivers/media/platform/renesas-ceu.c
> b/drivers/media/platform/renesas-ceu.c new file mode 100644
> index 000..d261704
> --- /dev/null
> +++ b/drivers/media/platform/renesas-ceu.c
> @@ -0,0 +1,1648 @@
> +// SPDX-License-Identifier: GPL-2.0

It was recently brought to my attention that SPDX headers should use either 
GPL-2.0-only or GPL-2.0-or-later, no the ambiguous GPL-2.0. Could you please 
update all patches in this series ?

[snip]

> +/*
> + * struct ceu_data - Platform specific CEU data
> + * @irq_mask: CETCR mask with all interrupt sources enabled. The mask
> differs
> + * between SH4 and RZ platforms.
> + */
> +struct ceu_data {
> + const u32 irq_mask;
> +};
> +
> +const struct ceu_data ceu_data_rz = {
> + .irq_mask = CEU_CETCR_ALL_IRQS_RZ,
> +};
> +
> +const struct ceu_data ceu_data_sh4 = {
> + .irq_mask = CEU_CETCR_ALL_IRQS_SH4,
> +};

I meant static and not const in my last review (as in static const struct 
ceu_data ...). Adding a const keyword in front of the u32 irq_mask field 
definition isn't very useful.

With that fixed,

Reviewed-by: Laurent Pinchart 

-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 1/9] dt-bindings: media: Add Renesas CEU bindings

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tuesday, 9 January 2018 18:25:23 EET Jacopo Mondi wrote:
> Add bindings documentation for Renesas Capture Engine Unit (CEU).
> 
> Signed-off-by: Jacopo Mondi 

Reviewed-by: Laurent Pinchart 

> ---
>  .../devicetree/bindings/media/renesas,ceu.txt  | 81
> ++ 1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt
> b/Documentation/devicetree/bindings/media/renesas,ceu.txt new file mode
> 100644
> index 000..590ee27
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
> @@ -0,0 +1,81 @@
> +Renesas Capture Engine Unit (CEU)
> +--
> +
> +The Capture Engine Unit is the image capture interface found in the Renesas
> +SH Mobile and RZ SoCs.
> +
> +The interface supports a single parallel input with data bus width of 8 or
> 16 +bits.
> +
> +Required properties:
> +- compatible: Shall be "renesas,r7s72100-ceu" for CEU units found in
> RZ/A1-H +  and RZ/A1-M SoCs.
> +- reg: Registers address base and size.
> +- interrupts: The interrupt specifier.
> +
> +The CEU supports a single parallel input and should contain a single 'port'
> +subnode with a single 'endpoint'. Connection to input devices are modeled
> +according to the video interfaces OF bindings specified in:
> +Documentation/devicetree/bindings/media/video-interfaces.txt
> +
> +Optional endpoint properties applicable to parallel input bus described in
> +the above mentioned "video-interfaces.txt" file are supported.
> +
> +- hsync-active: Active state of the HSYNC signal, 0/1 for LOW/HIGH
> respectively. +  If property is not present, default is active high.
> +- vsync-active: Active state of the VSYNC signal, 0/1 for LOW/HIGH
> respectively. +  If property is not present, default is active high.
> +
> +Example:
> +
> +The example describes the connection between the Capture Engine Unit and an
> +OV7670 image sensor connected to i2c1 interface.
> +
> +ceu: ceu@e821 {
> + reg = <0xe821 0x209c>;
> + compatible = "renesas,r7s72100-ceu";
> + interrupts = ;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins>;
> +
> + status = "okay";
> +
> + port {
> + ceu_in: endpoint {
> + remote-endpoint = <_out>;
> +
> + hsync-active = <1>;
> + vsync-active = <0>;
> + };
> + };
> +};
> +
> +i2c1: i2c@fcfee400 {
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins>;
> +
> + status = "okay";
> +
> + clock-frequency = <10>;
> +
> + ov7670: camera@21 {
> + compatible = "ovti,ov7670";
> + reg = <0x21>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins>;
> +
> + reset-gpios = < 11 GPIO_ACTIVE_LOW>;
> + powerdown-gpios = < 12 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + ov7670_out: endpoint {
> + remote-endpoint = <_in>;
> +
> + hsync-active = <1>;
> + vsync-active = <0>;
> + };
> + };
> + };
> +};


-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] media: v4l2-core: v4l2-mc: Add SPDX license identifier

[Laurent Pinchart]

Hi Shuah,

Thank you for the patch.

On Friday, 12 January 2018 00:26:22 EET Shuah Khan wrote:
> Replace GPL license statement with SPDX GPL-2.0 license identifier.
> 
> Signed-off-by: Shuah Khan 

Reviewed-by: Laurent Pinchart 

> ---
> Changes since v1:
> - Fixed SPDX comment format
> - Fixed SPDX license text to eliminate change in license. It now
>   reads GPL-2.0-or-later to maintain the original.
> 
>  drivers/media/v4l2-core/v4l2-mc.c | 12 ++--
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mc.c
> b/drivers/media/v4l2-core/v4l2-mc.c index 303980b71aae..8b61ccf3df81 100644
> --- a/drivers/media/v4l2-core/v4l2-mc.c
> +++ b/drivers/media/v4l2-core/v4l2-mc.c
> @@ -1,3 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
>  /*
>   * Media Controller ancillary functions
>   *
> @@ -5,16 +7,6 @@
>   * Copyright (C) 2016 Shuah Khan 
>   * Copyright (C) 2006-2010 Nokia Corporation
>   * Copyright (c) 2016 Intel Corporation.
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; either version 2 of the License, or
> - *  (at your option) any later version.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
>   */
> 
>  #include 


-- 
Regards,

Laurent Pinchart

Re: [PATCH v4 1/9] dt-bindings: media: Add Renesas CEU bindings

[Rob Herring]

On Tue, Jan 09, 2018 at 05:25:23PM +0100, Jacopo Mondi wrote:
> Add bindings documentation for Renesas Capture Engine Unit (CEU).
> 
> Signed-off-by: Jacopo Mondi 
> ---
>  .../devicetree/bindings/media/renesas,ceu.txt  | 81 
> ++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt

Reviewed-by: Rob Herring 

[PATCH v2] media: v4l2-core: v4l2-mc: Add SPDX license identifier

[Shuah Khan]

Replace GPL license statement with SPDX GPL-2.0 license identifier.

Signed-off-by: Shuah Khan 
---
Changes since v1:
- Fixed SPDX comment format
- Fixed SPDX license text to eliminate change in license. It now
  reads GPL-2.0-or-later to maintain the original.

 drivers/media/v4l2-core/v4l2-mc.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-mc.c 
b/drivers/media/v4l2-core/v4l2-mc.c
index 303980b71aae..8b61ccf3df81 100644
--- a/drivers/media/v4l2-core/v4l2-mc.c
+++ b/drivers/media/v4l2-core/v4l2-mc.c
@@ -1,3 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
 /*
  * Media Controller ancillary functions
  *
@@ -5,16 +7,6 @@
  * Copyright (C) 2016 Shuah Khan 
  * Copyright (C) 2006-2010 Nokia Corporation
  * Copyright (c) 2016 Intel Corporation.
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
  */
 
 #include 
-- 
2.14.1

Re: [PATCH] media: v4l2-core: v4l2-mc: Add SPDX license identifier

[Shuah Khan]

On 01/11/2018 02:33 PM, Laurent Pinchart wrote:
> Hi Shuah,
> 
> On Thursday, 11 January 2018 22:44:08 EET Shuah Khan wrote:
>> On 01/11/2018 11:42 AM, Laurent Pinchart wrote:
>>> On Thursday, 11 January 2018 17:45:15 EET Shuah Khan wrote:
 On 01/11/2018 05:55 AM, Laurent Pinchart wrote:
> On Wednesday, 10 January 2018 18:35:36 EET Shuah Khan wrote:
>> Replace GPL license statement with SPDX GPL-2.0 license identifier.
>>
>> Signed-off-by: Shuah Khan 
>> ---
>>
>>  drivers/media/v4l2-core/v4l2-mc.c | 11 +--
>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-mc.c
>> b/drivers/media/v4l2-core/v4l2-mc.c index 303980b71aae..1297132acd4e
>> 100644
>> --- a/drivers/media/v4l2-core/v4l2-mc.c
>> +++ b/drivers/media/v4l2-core/v4l2-mc.c
>> @@ -1,3 +1,4 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>
> The header doesn't match the existing license.

 When I added the file, I must have cut and pasted the license statement
 from another file. More on this below the deleted license lines.

> Furthermore, unless I'm mistaken, the standard comment style for SPDX
> headers in the kernel is //, not /* ... */

 Looks like we have 3 conventions for SPDX comment style.
 /* ... */ for headers and # ... for shell scripts and
 // for .c files.

 I can update it it and send v2 provided we think the change is inline
 with the original license.
>>>
>>> Personally I prefer the /* ... */ comment style, but I noticed that Greg
>>> used // in his large patch the adds SPDX license headers, so I think we
>>> should follow the established practice. I'll let you investigate to find
>>> what is preferred :)
>>
>> Yeah /*...*/ is my preferred as well. Hence  the autopilot change I made
>> in the first place. I redid a couple of patches already to follow the
>> // convention and I can do the same here.
>>
>>  /*
>>  
>>   * Media Controller ancillary functions
>>   *
>>
>> @@ -5,16 +6,6 @@
>>
>>   * Copyright (C) 2016 Shuah Khan 
>>   * Copyright (C) 2006-2010 Nokia Corporation
>>   * Copyright (c) 2016 Intel Corporation.
>>
>> - *
>> - *  This program is free software; you can redistribute it and/or
>> modify
>> - *  it under the terms of the GNU General Public License as published
>> by
>> - *  the Free Software Foundation; either version 2 of the License, or
>> - *  (at your option) any later version.

 Are you concerned about the "or (at your option) any later version." part
 that it doesn't match?
>>>
>>> Yes, that's my concern. I'm personally fine with GPL-2.0-only, but you'll
>>> have a hard time contacting all the other copyright holders if you want
>>> to relicense this. Good luck getting hold of the appropriate legal
>>> department at Nokia :-)
>>
>> Yeah. I don't think it is beneficial to continue this effort. I am going to
>> not pursue the patch at this file. Thanks for the review.
> 
> How about just using
> 
> // SPDX-License-Identifier: GPL-2.0-or-later
> 
> ? That is equivalent to the current license text.
> 

Hmm. Yes GPL-2.0-or-later would maintain the intent and doesn't change
the license. I can send v2 with that and see anybody objects to it.

thanks,
-- Shuah

Re: [PATCH v2 1/1] media: entity: Add a nop variant of media_entity_cleanup

[Arnd Bergmann]

On Thu, Jan 11, 2018 at 11:19 AM, Sakari Ailus
 wrote:
> Add nop variant of media_entity_cleanup. This allows calling
> media_entity_cleanup whether or not Media controller is enabled,
> simplifying driver code.
>
> Also drop #ifdefs on a few drivers around media_entity_cleanup().
>
> Signed-off-by: Sakari Ailus 

Thanks for addressing this,

Reviewed-by: Arnd Bergmann 

Re: [PATCH] media: v4l2-core: v4l2-mc: Add SPDX license identifier

[Laurent Pinchart]

Hi Shuah,

On Thursday, 11 January 2018 22:44:08 EET Shuah Khan wrote:
> On 01/11/2018 11:42 AM, Laurent Pinchart wrote:
> > On Thursday, 11 January 2018 17:45:15 EET Shuah Khan wrote:
> >> On 01/11/2018 05:55 AM, Laurent Pinchart wrote:
> >>> On Wednesday, 10 January 2018 18:35:36 EET Shuah Khan wrote:
>  Replace GPL license statement with SPDX GPL-2.0 license identifier.
>  
>  Signed-off-by: Shuah Khan 
>  ---
>  
>   drivers/media/v4l2-core/v4l2-mc.c | 11 +--
>   1 file changed, 1 insertion(+), 10 deletions(-)
>  
>  diff --git a/drivers/media/v4l2-core/v4l2-mc.c
>  b/drivers/media/v4l2-core/v4l2-mc.c index 303980b71aae..1297132acd4e
>  100644
>  --- a/drivers/media/v4l2-core/v4l2-mc.c
>  +++ b/drivers/media/v4l2-core/v4l2-mc.c
>  @@ -1,3 +1,4 @@
>  +/* SPDX-License-Identifier: GPL-2.0 */
> >>> 
> >>> The header doesn't match the existing license.
> >> 
> >> When I added the file, I must have cut and pasted the license statement
> >> from another file. More on this below the deleted license lines.
> >> 
> >>> Furthermore, unless I'm mistaken, the standard comment style for SPDX
> >>> headers in the kernel is //, not /* ... */
> >> 
> >> Looks like we have 3 conventions for SPDX comment style.
> >> /* ... */ for headers and # ... for shell scripts and
> >> // for .c files.
> >> 
> >> I can update it it and send v2 provided we think the change is inline
> >> with the original license.
> > 
> > Personally I prefer the /* ... */ comment style, but I noticed that Greg
> > used // in his large patch the adds SPDX license headers, so I think we
> > should follow the established practice. I'll let you investigate to find
> > what is preferred :)
> 
> Yeah /*...*/ is my preferred as well. Hence  the autopilot change I made
> in the first place. I redid a couple of patches already to follow the
> // convention and I can do the same here.
> 
>   /*
>   
>    * Media Controller ancillary functions
>    *
>  
>  @@ -5,16 +6,6 @@
>  
>    * Copyright (C) 2016 Shuah Khan 
>    * Copyright (C) 2006-2010 Nokia Corporation
>    * Copyright (c) 2016 Intel Corporation.
>  
>  - *
>  - *  This program is free software; you can redistribute it and/or
>  modify
>  - *  it under the terms of the GNU General Public License as published
>  by
>  - *  the Free Software Foundation; either version 2 of the License, or
>  - *  (at your option) any later version.
> >> 
> >> Are you concerned about the "or (at your option) any later version." part
> >> that it doesn't match?
> > 
> > Yes, that's my concern. I'm personally fine with GPL-2.0-only, but you'll
> > have a hard time contacting all the other copyright holders if you want
> > to relicense this. Good luck getting hold of the appropriate legal
> > department at Nokia :-)
> 
> Yeah. I don't think it is beneficial to continue this effort. I am going to
> not pursue the patch at this file. Thanks for the review.

How about just using

// SPDX-License-Identifier: GPL-2.0-or-later

? That is equivalent to the current license text.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: imx258: Add imx258 camera sensor driver

[Sakari Ailus]

Hi Andy,

Thanks for the patch. Please see my comments below.

On Thu, Jan 11, 2018 at 10:47:39PM +0800, Andy Yeh wrote:
> Add a V4L2 sub-device driver for the Sony IMX258 image sensor.
> This is a camera sensor using the I2C bus for control and the
> CSI-2 bus for data.
> 
> Signed-off-by: Andy Yeh 
> ---
>  MAINTAINERS|7 +
>  drivers/media/i2c/Kconfig  |   11 +
>  drivers/media/i2c/Makefile |1 +
>  drivers/media/i2c/imx258.c | 1184 
> 
>  4 files changed, 1203 insertions(+)
>  create mode 100644 drivers/media/i2c/imx258.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d76af75..806aa46 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12640,6 +12640,13 @@ S:   Maintained
>  F:   drivers/ssb/
>  F:   include/linux/ssb/
>  
> +SONY IMX258 SENSOR DRIVER
> +M:   Sakari Ailus 
> +L:   linux-media@vger.kernel.org
> +T:   git git://linuxtv.org/media_tree.git
> +S:   Maintained
> +F:   drivers/media/i2c/imx258.c
> +
>  SONY IMX274 SENSOR DRIVER
>  M:   Leon Luo 
>  L:   linux-media@vger.kernel.org
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index cb5d7ff..cabde37 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -555,6 +555,17 @@ config VIDEO_APTINA_PLL
>  config VIDEO_SMIAPP_PLL
>   tristate
>  
> +config VIDEO_IMX258
> + tristate "Sony IMX258 sensor support"
> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + depends on MEDIA_CAMERA_SUPPORT
> + ---help---
> +   This is a Video4Linux2 sensor-level driver for the Sony
> +   IMX258 camera.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called imx258.
> +
>  config VIDEO_IMX274
>   tristate "Sony IMX274 sensor support"
>   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 548a9ef..cf1e0f1 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -93,6 +93,7 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
>  obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o
>  obj-$(CONFIG_VIDEO_OV2659)   += ov2659.o
>  obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> +obj-$(CONFIG_VIDEO_IMX258)   += imx258.o
>  obj-$(CONFIG_VIDEO_IMX274)   += imx274.o
>  
>  obj-$(CONFIG_SDR_MAX2175) += max2175.o
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> new file mode 100644
> index 000..e4a44d6
> --- /dev/null
> +++ b/drivers/media/i2c/imx258.c
> @@ -0,0 +1,1184 @@
> +// Copyright (C) 2018 Intel Corporation
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define IMX258_REG_VALUE_08BIT   1
> +#define IMX258_REG_VALUE_16BIT   2
> +#define IMX258_REG_VALUE_24BIT   3
> +
> +#define IMX258_REG_MODE_SELECT   0x0100
> +#define IMX258_MODE_STANDBY  0x00
> +#define IMX258_MODE_STREAMING0x01
> +
> +/* Chip ID */
> +#define IMX258_REG_CHIP_ID   0x0016
> +#define IMX258_CHIP_ID   0x0258
> +
> +/* V_TIMING internal */
> +#define IMX258_REG_VTS   0x0340
> +#define IMX258_VTS_30FPS 0x0c98
> +#define IMX258_VTS_MAX   0x7fff
> +
> +/* HBLANK control - read only */
> +#define IMX258_PPL_650MHZ5352
> +#define IMX258_PPL_325MHZ2676
> +
> +/* Exposure control */
> +#define IMX258_REG_EXPOSURE  0x0202
> +#define IMX258_EXPOSURE_MIN  4
> +#define IMX258_EXPOSURE_STEP 1
> +#define IMX258_EXPOSURE_DEFAULT  0x640
> +
> +/* Analog gain control */
> +#define IMX258_REG_ANALOG_GAIN   0x0204
> +#define IMX258_ANA_GAIN_MIN  0
> +#define IMX258_ANA_GAIN_MAX  0x1fff
> +#define IMX258_ANA_GAIN_STEP 1
> +#define IMX258_ANA_GAIN_DEFAULT  0x0
> +
> +/* Orientation */
> +#define REG_MIRROR_FLIP_CONTROL  0x0101
> +#define REG_CONFIG_MIRROR_FLIP   0x03
> +
> +struct imx258_reg {
> + u16 address;
> + u8 val;
> +};
> +
> +struct imx258_reg_list {
> + u32 num_of_regs;
> + const struct imx258_reg *regs;
> +};
> +
> +/* Link frequency config */
> +struct imx258_link_freq_config {
> + u32 pixels_per_line;
> +
> + /* PLL registers for this link frequency */
> + struct imx258_reg_list reg_list;
> +};
> +
> +/* Mode : resolution and related config */
> +struct imx258_mode {
> + /* Frame width */
> + u32 width;
> + /* Frame height */
> + u32 height;
> +
> + /* V-timing */
> + u32 vts_def;
> + u32 vts_min;
> +
> + /* Index of Link frequency config to be used */
> + u32 link_freq_index;
> + /* Default register values */
> + struct imx258_reg_list reg_list;
> +};
> +
> +/* 4208x3118 needs 

Re: [PATCH] media: v4l2-core: v4l2-mc: Add SPDX license identifier

[Shuah Khan]

On 01/11/2018 11:42 AM, Laurent Pinchart wrote:
> Hi Shuah,
> 
> On Thursday, 11 January 2018 17:45:15 EET Shuah Khan wrote:
>> On 01/11/2018 05:55 AM, Laurent Pinchart wrote:
>>> On Wednesday, 10 January 2018 18:35:36 EET Shuah Khan wrote:
 Replace GPL license statement with SPDX GPL-2.0 license identifier.

 Signed-off-by: Shuah Khan 
 ---

  drivers/media/v4l2-core/v4l2-mc.c | 11 +--
  1 file changed, 1 insertion(+), 10 deletions(-)

 diff --git a/drivers/media/v4l2-core/v4l2-mc.c
 b/drivers/media/v4l2-core/v4l2-mc.c index 303980b71aae..1297132acd4e
 100644
 --- a/drivers/media/v4l2-core/v4l2-mc.c
 +++ b/drivers/media/v4l2-core/v4l2-mc.c
 @@ -1,3 +1,4 @@
 +/* SPDX-License-Identifier: GPL-2.0 */
>>>
>>> The header doesn't match the existing license.
>>
>> When I added the file, I must have cut and pasted the license statement
>> from another file. More on this below the deleted license lines.
>>
>>> Furthermore, unless I'm mistaken, the standard comment style for SPDX
>>> headers in the kernel is //, not /* ... */
>>
>> Looks like we have 3 conventions for SPDX comment style.
>> /* ... */ for headers and # ... for shell scripts and
>> // for .c files.
>>
>> I can update it it and send v2 provided we think the change is inline
>> with the original license.
> 
> Personally I prefer the /* ... */ comment style, but I noticed that Greg used 
> // in his large patch the adds SPDX license headers, so I think we should 
> follow the established practice. I'll let you investigate to find what is 
> preferred :)

Yeah /*...*/ is my preferred as well. Hence  the autopilot change I made
in the first place. I redid a couple of patches already to follow the
// convention and I can do the same here.

> 
  /*
  
   * Media Controller ancillary functions
   *

 @@ -5,16 +6,6 @@

   * Copyright (C) 2016 Shuah Khan 
   * Copyright (C) 2006-2010 Nokia Corporation
   * Copyright (c) 2016 Intel Corporation.

 - *
 - *  This program is free software; you can redistribute it and/or modify
 - *  it under the terms of the GNU General Public License as published by
 - *  the Free Software Foundation; either version 2 of the License, or
 - *  (at your option) any later version.
>>
>> Are you concerned about the "or (at your option) any later version." part
>> that it doesn't match?
> 
> Yes, that's my concern. I'm personally fine with GPL-2.0-only, but you'll 
> have 
> a hard time contacting all the other copyright holders if you want to 
> relicense this. Good luck getting hold of the appropriate legal department at 
> Nokia :-)

Yeah. I don't think it is beneficial to continue this effort. I am going to not
pursue the patch at this file. Thanks for the review.

thanks,
-- Shuah

Re: [PATCH v2 2/2] media: ov9650: add device tree binding

[Rob Herring]

On Mon, Jan 08, 2018 at 01:54:24AM +0900, Akinobu Mita wrote:
> Now the ov9650 driver supports device tree probing.  So this adds a
> device tree binding documentation.
> 
> Cc: Jacopo Mondi 
> Cc: H. Nikolaus Schaller 
> Cc: Hugues Fruchet 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Cc: Rob Herring 
> Signed-off-by: Akinobu Mita 
> ---
> * Changelog v2
> - Split binding documentation, suggested by Rob Herring and Jacopo Mondi
> - Improve the wording for compatible property in the binding documentation,
>   suggested by Jacopo Mondi
> - Improve the description for the device node in the binding documentation,
>   suggested by Sakari Ailus
> 
>  .../devicetree/bindings/media/i2c/ov9650.txt   | 36 
> ++
>  1 file changed, 36 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt

Reviewed-by: Rob Herring 

Re: [PATCH] media: v4l2-core: v4l2-mc: Add SPDX license identifier

[Laurent Pinchart]

Hi Shuah,

On Thursday, 11 January 2018 17:45:15 EET Shuah Khan wrote:
> On 01/11/2018 05:55 AM, Laurent Pinchart wrote:
> > On Wednesday, 10 January 2018 18:35:36 EET Shuah Khan wrote:
> >> Replace GPL license statement with SPDX GPL-2.0 license identifier.
> >> 
> >> Signed-off-by: Shuah Khan 
> >> ---
> >> 
> >>  drivers/media/v4l2-core/v4l2-mc.c | 11 +--
> >>  1 file changed, 1 insertion(+), 10 deletions(-)
> >> 
> >> diff --git a/drivers/media/v4l2-core/v4l2-mc.c
> >> b/drivers/media/v4l2-core/v4l2-mc.c index 303980b71aae..1297132acd4e
> >> 100644
> >> --- a/drivers/media/v4l2-core/v4l2-mc.c
> >> +++ b/drivers/media/v4l2-core/v4l2-mc.c
> >> @@ -1,3 +1,4 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> > 
> > The header doesn't match the existing license.
> 
> When I added the file, I must have cut and pasted the license statement
> from another file. More on this below the deleted license lines.
> 
> > Furthermore, unless I'm mistaken, the standard comment style for SPDX
> > headers in the kernel is //, not /* ... */
> 
> Looks like we have 3 conventions for SPDX comment style.
> /* ... */ for headers and # ... for shell scripts and
> // for .c files.
> 
> I can update it it and send v2 provided we think the change is inline
> with the original license.

Personally I prefer the /* ... */ comment style, but I noticed that Greg used 
// in his large patch the adds SPDX license headers, so I think we should 
follow the established practice. I'll let you investigate to find what is 
preferred :)

> >>  /*
> >>  
> >>   * Media Controller ancillary functions
> >>   *
> >> 
> >> @@ -5,16 +6,6 @@
> >> 
> >>   * Copyright (C) 2016 Shuah Khan 
> >>   * Copyright (C) 2006-2010 Nokia Corporation
> >>   * Copyright (c) 2016 Intel Corporation.
> >> 
> >> - *
> >> - *  This program is free software; you can redistribute it and/or modify
> >> - *  it under the terms of the GNU General Public License as published by
> >> - *  the Free Software Foundation; either version 2 of the License, or
> >> - *  (at your option) any later version.
> 
> Are you concerned about the "or (at your option) any later version." part
> that it doesn't match?

Yes, that's my concern. I'm personally fine with GPL-2.0-only, but you'll have 
a hard time contacting all the other copyright holders if you want to 
relicense this. Good luck getting hold of the appropriate legal department at 
Nokia :-)

On a related note, I nowadays encourage developers to keep their copyright on 
code they wrote when possible, and to at least negotiate that with their 
employers.

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

-- 
Regards,

Laurent Pinchart

Re: [RFT PATCH v2 6/6] uvcvideo: Move decode processing to process context

[Kieran Bingham]

Hi Kieran !

Thanking you for the patch might be rather self serving here :D

On 09/01/18 13:09, Kieran Bingham wrote:
> Newer high definition cameras, and cameras with multiple lenses such as
> the range of stereo-vision cameras now available have ever increasing
> data rates.
> 
> The inclusion of a variable length packet header in URB packets mean
> that we must memcpy the frame data out to our destination 'manually'.
> This can result in data rates of up to 2 gigabits per second being
> processed.
> 
> To improve efficiency, and maximise throughput, handle the URB decode
> processing through a work queue to move it from interrupt context, and
> allow multiple processors to work on URBs in parallel.
> 
> Signed-off-by: Kieran Bingham 
> 
> ---
> v2:
>  - Lock full critical section of usb_submit_urb()
> 
>  drivers/media/usb/uvc/uvc_queue.c |  12 +++-
>  drivers/media/usb/uvc/uvc_video.c | 111 +--
>  drivers/media/usb/uvc/uvcvideo.h  |  24 +++-
>  3 files changed, 129 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_queue.c 
> b/drivers/media/usb/uvc/uvc_queue.c
> index 5a9987e547d3..598087eeb5c2 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -179,10 +179,22 @@ static void uvc_stop_streaming(struct vb2_queue *vq)
>   struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
>   struct uvc_streaming *stream = uvc_queue_to_stream(queue);
>  
> + /* Prevent new buffers coming in. */
> + spin_lock_irq(>irqlock);
> + queue->flags |= UVC_QUEUE_STOPPING;
> + spin_unlock_irq(>irqlock);
> +
> + /*
> +  * All pending work should be completed before disabling the stream, as
> +  * all URBs will be free'd during uvc_video_enable(s, 0).
> +  */
> + flush_workqueue(stream->async_wq);
> +
>   uvc_video_enable(stream, 0);
>  
>   spin_lock_irq(>irqlock);
>   uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
> + queue->flags &= ~UVC_QUEUE_STOPPING;
>   spin_unlock_irq(>irqlock);
>  }
>  
> diff --git a/drivers/media/usb/uvc/uvc_video.c 
> b/drivers/media/usb/uvc/uvc_video.c
> index 3878bec3276e..a9ddc4f27012 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1058,21 +1058,67 @@ static int uvc_video_decode_start(struct 
> uvc_streaming *stream,
>   return data[0];
>  }
>  
> -static void uvc_video_decode_data(struct uvc_streaming *stream,
> - struct uvc_buffer *buf, const __u8 *data, int len)
> +/*
> + * uvc_video_decode_data_work: Asynchronous memcpy processing
> + *
> + * Perform memcpy tasks in process context, with completion handlers
> + * to return the URB, and buffer handles.
> + *
> + * The work submitter must pre-determine that the work is safe
> + */
> +static void uvc_video_decode_data_work(struct work_struct *work)

This handles asynchronous memory copy work - which could equally be encode work
- so _decode_ might not be an appropriate name here.

>  {
> - unsigned int maxlen, nbytes;
> - void *mem;
> + struct uvc_urb *uvc_urb = container_of(work, struct uvc_urb, work);
> + struct uvc_streaming *stream = uvc_urb->stream;
> + struct uvc_video_queue *queue = >queue;
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < uvc_urb->packets; i++) {
> + struct uvc_decode_op *op = _urb->decodes[i];
> +
> + memcpy(op->dst, op->src, op->len);
> +
> + /* Release reference taken on this buffer */
> + uvc_queue_buffer_release(op->buf);
> + }
> +
> + /*
> +  * Prevent resubmitting URBs when shutting down to ensure that no new
> +  * work item will be scheduled after uvc_stop_streaming() flushes the
> +  * work queue.
> +  */
> + spin_lock_irq(>irqlock);
> + if (!(queue->flags & UVC_QUEUE_STOPPING)) {
> + ret = usb_submit_urb(uvc_urb->urb, GFP_ATOMIC);
> + if (ret < 0)
> + uvc_printk(KERN_ERR,
> +"Failed to resubmit video URB (%d).\n",
> +ret);
> + }
> + spin_unlock_irq(>irqlock);
> +}
> +
> +static void uvc_video_decode_data(struct uvc_decode_op *decode,
> + struct uvc_urb *uvc_urb, struct uvc_buffer *buf,
> + const __u8 *data, int len)
> +{
> + unsigned int maxlen;
>  
>   if (len <= 0)
>   return;
>  
> - /* Copy the video data to the buffer. */
>   maxlen = buf->length - buf->bytesused;
> - mem = buf->mem + buf->bytesused;
> - nbytes = min((unsigned int)len, maxlen);
> - memcpy(mem, data, nbytes);
> - buf->bytesused += nbytes;
> +
> + /* Take a buffer reference for async work */
> + kref_get(>ref);
> +
> + decode->buf = buf;
> + decode->src = data;
> + decode->dst = buf->mem + buf->bytesused;
> + decode->len = min_t(unsigned int, len, maxlen);

Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution

[Daniel Borkmann]

On 01/11/2018 04:58 PM, Dan Williams wrote:
> On Thu, Jan 11, 2018 at 1:54 AM, Jiri Kosina  wrote:
>> On Tue, 9 Jan 2018, Josh Poimboeuf wrote:
>>> On Tue, Jan 09, 2018 at 11:44:05AM -0800, Dan Williams wrote:
 On Tue, Jan 9, 2018 at 11:34 AM, Jiri Kosina  wrote:
> On Fri, 5 Jan 2018, Dan Williams wrote:
>
> [ ... snip ... ]
>> Andi Kleen (1):
>>   x86, barrier: stop speculation for failed access_ok
>>
>> Dan Williams (13):
>>   x86: implement nospec_barrier()
>>   [media] uvcvideo: prevent bounds-check bypass via speculative 
>> execution
>>   carl9170: prevent bounds-check bypass via speculative execution
>>   p54: prevent bounds-check bypass via speculative execution
>>   qla2xxx: prevent bounds-check bypass via speculative execution
>>   cw1200: prevent bounds-check bypass via speculative execution
>>   Thermal/int340x: prevent bounds-check bypass via speculative 
>> execution
>>   ipv6: prevent bounds-check bypass via speculative execution
>>   ipv4: prevent bounds-check bypass via speculative execution
>>   vfs, fdtable: prevent bounds-check bypass via speculative execution
>>   net: mpls: prevent bounds-check bypass via speculative execution
>>   udf: prevent bounds-check bypass via speculative execution
>>   userns: prevent bounds-check bypass via speculative execution
>>
>> Mark Rutland (4):
>>   asm-generic/barrier: add generic nospec helpers
>>   Documentation: document nospec helpers
>>   arm64: implement nospec_ptr()
>>   arm: implement nospec_ptr()
>
> So considering the recent publication of [1], how come we all of a sudden
> don't need the barriers in ___bpf_prog_run(), namely for LD_IMM_DW and
> LDX_MEM_##SIZEOP, and something comparable for eBPF JIT?
>
> Is this going to be handled in eBPF in some other way?
>
> Without that in place, and considering Jann Horn's paper, it would seem
> like PTI doesn't really lock it down fully, right?

 Here is the latest (v3) bpf fix:

 https://patchwork.ozlabs.org/patch/856645/

 I currently have v2 on my 'nospec' branch and will move that to v3 for
 the next update, unless it goes upstream before then.
>>
>> Daniel, I guess you're planning to send this still for 4.15?
> 
> It's pending in the bpf.git tree:
> 
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=b2157399cc9

Sorry for the delay, just noticed the question now since not on Cc either:
It made it into in DaveM's tree already and part of his latest pull-req
to Linus.

>>> That patch seems specific to CONFIG_BPF_SYSCALL.  Is the bpf() syscall
>>> the only attack vector?  Or are there other ways to run bpf programs
>>> that we should be worried about?
>>
>> Seems like Alexei is probably the only person in the whole universe who
>> isn't CCed here ... let's fix that.
> 
> He will be cc'd on v2 of this series which will be available later today.

[PATCH v2] MAINTAINERS: mtd/nand: update Microchip nand entry

[Nicolas Ferre]

Update Wenyou Yang email address.
Take advantage of this update to move this entry to the MICROCHIP / ATMEL
location and add the DT binding documentation link.

Signed-off-by: Nicolas Ferre 
Acked-by: Wenyou Yang 
---
v2: - patch agains 09ec417b0ea8 ("mtd: nand: samsung: Disable subpage
  writes on E-die NAND") of 
  http://git.infradead.org/linux-mtd.git/shortlog/refs/heads/nand/next
- Ack by Wenyou added


Hi,

v1 patch was part of a series because it was conflicting with the previous one
named:
"[PATCH 1/2] MAINTAINERS: linux-media: update Microchip ISI and ISC entries"
Boris asked me to rebase it so that they are independent.
So, if this first one goes upstream through another tree, conflicts will have
to be resolved at one point.

Best regards,
  Nicolas


 MAINTAINERS | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index aa71ab52fd76..37ee5ae4bae2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2382,13 +2382,6 @@ F:   
Documentation/devicetree/bindings/input/atmel,maxtouch.txt
 F: drivers/input/touchscreen/atmel_mxt_ts.c
 F: include/linux/platform_data/atmel_mxt_ts.h
 
-ATMEL NAND DRIVER
-M: Wenyou Yang 
-M: Josh Wu 
-L: linux-...@lists.infradead.org
-S: Supported
-F: drivers/mtd/nand/atmel/*
-
 ATMEL SAMA5D2 ADC DRIVER
 M: Ludovic Desroches 
 L: linux-...@vger.kernel.org
@@ -9045,6 +9038,14 @@ F:   drivers/media/platform/atmel/atmel-isc.c
 F: drivers/media/platform/atmel/atmel-isc-regs.h
 F: devicetree/bindings/media/atmel-isc.txt
 
+MICROCHIP / ATMEL NAND DRIVER
+M: Wenyou Yang 
+M: Josh Wu 
+L: linux-...@lists.infradead.org
+S: Supported
+F: drivers/mtd/nand/atmel/*
+F: Documentation/devicetree/bindings/mtd/atmel-nand.txt
+
 MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
 M: Woojung Huh 
 M: Microchip Linux Driver Support 
-- 
2.9.0

Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution

[Dan Williams]

On Thu, Jan 11, 2018 at 1:54 AM, Jiri Kosina  wrote:
> On Tue, 9 Jan 2018, Josh Poimboeuf wrote:
>
>> On Tue, Jan 09, 2018 at 11:44:05AM -0800, Dan Williams wrote:
>> > On Tue, Jan 9, 2018 at 11:34 AM, Jiri Kosina  wrote:
>> > > On Fri, 5 Jan 2018, Dan Williams wrote:
>> > >
>> > > [ ... snip ... ]
>> > >> Andi Kleen (1):
>> > >>   x86, barrier: stop speculation for failed access_ok
>> > >>
>> > >> Dan Williams (13):
>> > >>   x86: implement nospec_barrier()
>> > >>   [media] uvcvideo: prevent bounds-check bypass via speculative 
>> > >> execution
>> > >>   carl9170: prevent bounds-check bypass via speculative execution
>> > >>   p54: prevent bounds-check bypass via speculative execution
>> > >>   qla2xxx: prevent bounds-check bypass via speculative execution
>> > >>   cw1200: prevent bounds-check bypass via speculative execution
>> > >>   Thermal/int340x: prevent bounds-check bypass via speculative 
>> > >> execution
>> > >>   ipv6: prevent bounds-check bypass via speculative execution
>> > >>   ipv4: prevent bounds-check bypass via speculative execution
>> > >>   vfs, fdtable: prevent bounds-check bypass via speculative 
>> > >> execution
>> > >>   net: mpls: prevent bounds-check bypass via speculative execution
>> > >>   udf: prevent bounds-check bypass via speculative execution
>> > >>   userns: prevent bounds-check bypass via speculative execution
>> > >>
>> > >> Mark Rutland (4):
>> > >>   asm-generic/barrier: add generic nospec helpers
>> > >>   Documentation: document nospec helpers
>> > >>   arm64: implement nospec_ptr()
>> > >>   arm: implement nospec_ptr()
>> > >
>> > > So considering the recent publication of [1], how come we all of a sudden
>> > > don't need the barriers in ___bpf_prog_run(), namely for LD_IMM_DW and
>> > > LDX_MEM_##SIZEOP, and something comparable for eBPF JIT?
>> > >
>> > > Is this going to be handled in eBPF in some other way?
>> > >
>> > > Without that in place, and considering Jann Horn's paper, it would seem
>> > > like PTI doesn't really lock it down fully, right?
>> >
>> > Here is the latest (v3) bpf fix:
>> >
>> > https://patchwork.ozlabs.org/patch/856645/
>> >
>> > I currently have v2 on my 'nospec' branch and will move that to v3 for
>> > the next update, unless it goes upstream before then.
>
> Daniel, I guess you're planning to send this still for 4.15?

It's pending in the bpf.git tree:


https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=b2157399cc9

>> That patch seems specific to CONFIG_BPF_SYSCALL.  Is the bpf() syscall
>> the only attack vector?  Or are there other ways to run bpf programs
>> that we should be worried about?
>
> Seems like Alexei is probably the only person in the whole universe who
> isn't CCed here ... let's fix that.

He will be cc'd on v2 of this series which will be available later today.

Re: [PATCH] media: v4l2-core: v4l2-mc: Add SPDX license identifier

[Shuah Khan]

On 01/11/2018 05:55 AM, Laurent Pinchart wrote:
> Hi Shuah,
> 
> Thank you for the patch.
> 
> On Wednesday, 10 January 2018 18:35:36 EET Shuah Khan wrote:
>> Replace GPL license statement with SPDX GPL-2.0 license identifier.
>>
>> Signed-off-by: Shuah Khan 
>> ---
>>  drivers/media/v4l2-core/v4l2-mc.c | 11 +--
>>  1 file changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-mc.c
>> b/drivers/media/v4l2-core/v4l2-mc.c index 303980b71aae..1297132acd4e 100644
>> --- a/drivers/media/v4l2-core/v4l2-mc.c
>> +++ b/drivers/media/v4l2-core/v4l2-mc.c
>> @@ -1,3 +1,4 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
> 
> The header doesn't match the existing license.

When I added the file, I must have cut and pasted the license statement
from another file. More on this below the deleted license lines.

> 
> Furthermore, unless I'm mistaken, the standard comment style for SPDX headers 
> in the kernel is //, not /* ... */

Looks like we have 3 conventions for SPDX comment style.
/* ... */ for headers and # ... for shell scripts and
// for .c files.

I can update it it and send v2 provided we think the change is inline
with the original license.

> 
>>  /*
>>   * Media Controller ancillary functions
>>   *
>> @@ -5,16 +6,6 @@
>>   * Copyright (C) 2016 Shuah Khan 
>>   * Copyright (C) 2006-2010 Nokia Corporation
>>   * Copyright (c) 2016 Intel Corporation.
>> - *
>> - *  This program is free software; you can redistribute it and/or modify
>> - *  it under the terms of the GNU General Public License as published by
>> - *  the Free Software Foundation; either version 2 of the License, or
>> - *  (at your option) any later version.

Are you concerned about the "or (at your option) any later version." part
that it doesn't match?

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

thanks,
-- Shuah

Re: [PATCH v2 2/2] media: dt-bindings: Add OF properties to ov7670

[Rob Herring]

On Tue, Jan 9, 2018 at 2:01 AM, jacopo mondi  wrote:
> Hi Rob,
>thanks for comments
>
> On Mon, Jan 08, 2018 at 09:35:55PM -0600, Rob Herring wrote:
>> On Thu, Jan 04, 2018 at 10:52:33AM +0100, Jacopo Mondi wrote:
>> > Describe newly introduced OF properties for ov7670 image sensor.
>> > The driver supports two standard properties to configure synchronism
>> > signals polarities and two custom properties already supported as
>> > platform data options by the driver.
>>
>> Missing S-o-b.
>>
>
> Ups, that was trivial, sorry about that
>
>> > ---
>> >  Documentation/devicetree/bindings/media/i2c/ov7670.txt | 14 ++
>> >  1 file changed, 14 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov7670.txt 
>> > b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
>> > index 826b656..57ded18 100644
>> > --- a/Documentation/devicetree/bindings/media/i2c/ov7670.txt
>> > +++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
>> > @@ -9,11 +9,22 @@ Required Properties:
>> >  - clocks: reference to the xclk input clock.
>> >  - clock-names: should be "xclk".
>> >
>> > +The following properties, as defined by video interfaces OF bindings
>> > +"Documentation/devicetree/bindings/media/video-interfaces.txt" are 
>> > supported:
>> > +
>> > +- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH 
>> > respectively.
>> > +- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH 
>> > respectively.
>>
>> Don't these go in the endpoint? Not sure offhand.
>>
>
> Yes they do. I will list them as "Optional endpoint properties", and
>
>> > +
>> > +Default is high active state for both vsync and hsync signals.
>> > +
>> >  Optional Properties:
>> >  - reset-gpios: reference to the GPIO connected to the resetb pin, if any.
>> >Active is low.
>> >  - powerdown-gpios: reference to the GPIO connected to the pwdn pin, if 
>> > any.
>> >Active is high.
>> > +- ov7670,pll-bypass: set to 1 to bypass PLL for pixel clock generation.
>>
>> Boolean instead?
>>
>
> Do we have booleans? I had a look at device tree specs and grep for
> "true"/"false" in arch/arm*/boot/dts, and didn't find that much.
> Seems like they're actually strings, am I wrong?

Properties with no value are boolean. Present is true, absent is
false. "foo = <0>" is also treated as true, but not recommended.

Rob

[PATCH] media: imx258: Add imx258 camera sensor driver

[Andy Yeh]

Add a V4L2 sub-device driver for the Sony IMX258 image sensor.
This is a camera sensor using the I2C bus for control and the
CSI-2 bus for data.

Signed-off-by: Andy Yeh 
---
 MAINTAINERS|7 +
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/imx258.c | 1184 
 4 files changed, 1203 insertions(+)
 create mode 100644 drivers/media/i2c/imx258.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d76af75..806aa46 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12640,6 +12640,13 @@ S: Maintained
 F: drivers/ssb/
 F: include/linux/ssb/
 
+SONY IMX258 SENSOR DRIVER
+M: Sakari Ailus 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/imx258.c
+
 SONY IMX274 SENSOR DRIVER
 M: Leon Luo 
 L: linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cb5d7ff..cabde37 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -555,6 +555,17 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
+config VIDEO_IMX258
+   tristate "Sony IMX258 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the Sony
+ IMX258 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called imx258.
+
 config VIDEO_IMX274
tristate "Sony IMX274 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 548a9ef..cf1e0f1 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
 obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
+obj-$(CONFIG_VIDEO_IMX258) += imx258.o
 obj-$(CONFIG_VIDEO_IMX274) += imx274.o
 
 obj-$(CONFIG_SDR_MAX2175) += max2175.o
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
new file mode 100644
index 000..e4a44d6
--- /dev/null
+++ b/drivers/media/i2c/imx258.c
@@ -0,0 +1,1184 @@
+// Copyright (C) 2018 Intel Corporation
+// SPDX-License-Identifier: GPL-2.0
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IMX258_REG_VALUE_08BIT 1
+#define IMX258_REG_VALUE_16BIT 2
+#define IMX258_REG_VALUE_24BIT 3
+
+#define IMX258_REG_MODE_SELECT 0x0100
+#define IMX258_MODE_STANDBY0x00
+#define IMX258_MODE_STREAMING  0x01
+
+/* Chip ID */
+#define IMX258_REG_CHIP_ID 0x0016
+#define IMX258_CHIP_ID 0x0258
+
+/* V_TIMING internal */
+#define IMX258_REG_VTS 0x0340
+#define IMX258_VTS_30FPS   0x0c98
+#define IMX258_VTS_MAX 0x7fff
+
+/* HBLANK control - read only */
+#define IMX258_PPL_650MHZ  5352
+#define IMX258_PPL_325MHZ  2676
+
+/* Exposure control */
+#define IMX258_REG_EXPOSURE0x0202
+#define IMX258_EXPOSURE_MIN4
+#define IMX258_EXPOSURE_STEP   1
+#define IMX258_EXPOSURE_DEFAULT0x640
+
+/* Analog gain control */
+#define IMX258_REG_ANALOG_GAIN 0x0204
+#define IMX258_ANA_GAIN_MIN0
+#define IMX258_ANA_GAIN_MAX0x1fff
+#define IMX258_ANA_GAIN_STEP   1
+#define IMX258_ANA_GAIN_DEFAULT0x0
+
+/* Orientation */
+#define REG_MIRROR_FLIP_CONTROL0x0101
+#define REG_CONFIG_MIRROR_FLIP 0x03
+
+struct imx258_reg {
+   u16 address;
+   u8 val;
+};
+
+struct imx258_reg_list {
+   u32 num_of_regs;
+   const struct imx258_reg *regs;
+};
+
+/* Link frequency config */
+struct imx258_link_freq_config {
+   u32 pixels_per_line;
+
+   /* PLL registers for this link frequency */
+   struct imx258_reg_list reg_list;
+};
+
+/* Mode : resolution and related config */
+struct imx258_mode {
+   /* Frame width */
+   u32 width;
+   /* Frame height */
+   u32 height;
+
+   /* V-timing */
+   u32 vts_def;
+   u32 vts_min;
+
+   /* Index of Link frequency config to be used */
+   u32 link_freq_index;
+   /* Default register values */
+   struct imx258_reg_list reg_list;
+};
+
+/* 4208x3118 needs 1300Mbps/lane, 4 lanes */
+static const struct imx258_reg mipi_data_rate_1300mbps[] = {
+   {0x0301, 0x05},
+   {0x0303, 0x02},
+   {0x0305, 0x03},
+   {0x0306, 0x00},
+   {0x0307, 0xCB},
+   {0x0309, 0x0A},
+   {0x030B, 0x01},
+   {0x030D, 0x02},
+   {0x030E, 0x00},
+   {0x030F, 0xD8},
+   {0x0310, 0x00},
+   {0x0820, 0x14},
+   {0x0821, 

[PATCH] media: imx258: Add imx258 camera sensor driver

[Andy Yeh]

Add a V4L2 sub-device driver for the Sony IMX258 image sensor.
This is a camera sensor using the I2C bus for control and the
CSI-2 bus for data.

Signed-off-by: Andy Yeh 
---
 MAINTAINERS|7 +
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/imx258.c | 1184 
 4 files changed, 1203 insertions(+)
 create mode 100644 drivers/media/i2c/imx258.c

diff --git a/MAINTAINERS b/MAINTAINERS
index b46c9ce..f288320 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12641,6 +12641,13 @@ S: Maintained
 F: drivers/ssb/
 F: include/linux/ssb/
 
+SONY IMX258 SENSOR DRIVER
+M: Sakari Ailus 
+L: linux-media@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/imx258.c
+
 SONY IMX274 SENSOR DRIVER
 M: Leon Luo 
 L: linux-media@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cb5d7ff..cabde37 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -555,6 +555,17 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
+config VIDEO_IMX258
+   tristate "Sony IMX258 sensor support"
+   depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the Sony
+ IMX258 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called imx258.
+
 config VIDEO_IMX274
tristate "Sony IMX274 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 548a9ef..cf1e0f1 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
 obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
 obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
+obj-$(CONFIG_VIDEO_IMX258) += imx258.o
 obj-$(CONFIG_VIDEO_IMX274) += imx274.o
 
 obj-$(CONFIG_SDR_MAX2175) += max2175.o
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
new file mode 100644
index 000..e2b96f4
--- /dev/null
+++ b/drivers/media/i2c/imx258.c
@@ -0,0 +1,1184 @@
+// Copyright (C) 2018 Intel Corporation
+// SPDX-License-Identifier: GPL-2.0
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IMX258_REG_VALUE_08BIT 1
+#define IMX258_REG_VALUE_16BIT 2
+#define IMX258_REG_VALUE_24BIT 3
+
+#define IMX258_REG_MODE_SELECT 0x0100
+#define IMX258_MODE_STANDBY0x00
+#define IMX258_MODE_STREAMING  0x01
+
+/* Chip ID */
+#define IMX258_REG_CHIP_ID 0x0016
+#define IMX258_CHIP_ID 0x0258
+
+/* V_TIMING internal */
+#define IMX258_REG_VTS 0x0340
+#define IMX258_VTS_30FPS   0x0c98
+#define IMX258_VTS_MAX 0x7fff
+
+/* HBLANK control - read only */
+#define IMX258_PPL_650MHZ  5352
+#define IMX258_PPL_325MHZ  2676
+
+/* Exposure control */
+#define IMX258_REG_EXPOSURE0x0202
+#define IMX258_EXPOSURE_MIN4
+#define IMX258_EXPOSURE_STEP   1
+#define IMX258_EXPOSURE_DEFAULT0x640
+
+/* Analog gain control */
+#define IMX258_REG_ANALOG_GAIN 0x0204
+#define IMX258_ANA_GAIN_MIN0
+#define IMX258_ANA_GAIN_MAX0x1fff
+#define IMX258_ANA_GAIN_STEP   1
+#define IMX258_ANA_GAIN_DEFAULT0x0
+
+/* Orientation */
+#define REG_MIRROR_FLIP_CONTROL0x0101
+#define REG_CONFIG_MIRROR_FLIP 0x03
+
+struct imx258_reg {
+   u16 address;
+   u8 val;
+};
+
+struct imx258_reg_list {
+   u32 num_of_regs;
+   const struct imx258_reg *regs;
+};
+
+/* Link frequency config */
+struct imx258_link_freq_config {
+   u32 pixels_per_line;
+
+   /* PLL registers for this link frequency */
+   struct imx258_reg_list reg_list;
+};
+
+/* Mode : resolution and related config */
+struct imx258_mode {
+   /* Frame width */
+   u32 width;
+   /* Frame height */
+   u32 height;
+
+   /* V-timing */
+   u32 vts_def;
+   u32 vts_min;
+
+   /* Index of Link frequency config to be used */
+   u32 link_freq_index;
+   /* Default register values */
+   struct imx258_reg_list reg_list;
+};
+
+/* 4208x3118 needs 1300Mbps/lane, 4 lanes */
+static const struct imx258_reg mipi_data_rate_1300mbps[] = {
+   {0x0301, 0x05},
+   {0x0303, 0x02},
+   {0x0305, 0x03},
+   {0x0306, 0x00},
+   {0x0307, 0xCB},
+   {0x0309, 0x0A},
+   {0x030B, 0x01},
+   {0x030D, 0x02},
+   {0x030E, 0x00},
+   {0x030F, 0xD8},
+   {0x0310, 0x00},
+   {0x0820, 0x14},
+   {0x0821, 

Re: [PATCH v5 2/2] media: V3s: Add support for Allwinner CSI.

[Maxime Ripard]

Hi Yong,

On Thu, Jan 11, 2018 at 11:06:06AM +0800, Yong Deng wrote:
> Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
> interface and CSI1 is used for parallel interface. This is not
> documented in datasheet but by test and guess.
> 
> This patch implement a v4l2 framework driver for it.
> 
> Currently, the driver only support the parallel interface. MIPI-CSI2,
> ISP's support are not included in this patch.
> 
> Signed-off-by: Yong Deng 

I've needed this patch in order to fix a NULL pointer dereference:
http://code.bulix.org/oz6gmb-257359?raw

This is needed because while it's ok to have a NULL pointer to
v4l2_subdev_pad_config when you call the subdev set_fmt with
V4L2_SUBDEV_FORMAT_ACTIVE, it's not with V4L2_SUBDEV_FORMAT_TRY, and
sensors will assume taht it's a valid pointer.

Otherwise,
Tested-by: Maxime Ripard 

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH] media: v4l2-core: v4l2-mc: Add SPDX license identifier

[Laurent Pinchart]

Hi Shuah,

Thank you for the patch.

On Wednesday, 10 January 2018 18:35:36 EET Shuah Khan wrote:
> Replace GPL license statement with SPDX GPL-2.0 license identifier.
> 
> Signed-off-by: Shuah Khan 
> ---
>  drivers/media/v4l2-core/v4l2-mc.c | 11 +--
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mc.c
> b/drivers/media/v4l2-core/v4l2-mc.c index 303980b71aae..1297132acd4e 100644
> --- a/drivers/media/v4l2-core/v4l2-mc.c
> +++ b/drivers/media/v4l2-core/v4l2-mc.c
> @@ -1,3 +1,4 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

The header doesn't match the existing license.

Furthermore, unless I'm mistaken, the standard comment style for SPDX headers 
in the kernel is //, not /* ... */

>  /*
>   * Media Controller ancillary functions
>   *
> @@ -5,16 +6,6 @@
>   * Copyright (C) 2016 Shuah Khan 
>   * Copyright (C) 2006-2010 Nokia Corporation
>   * Copyright (c) 2016 Intel Corporation.
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; either version 2 of the License, or
> - *  (at your option) any later version.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
>   */
> 
>  #include 

-- 
Regards,

Laurent Pinchart

[GIT PULL for 4.16] Print fwnode parsing results, add nop media_entity_cleanup

[Sakari Ailus]

Hi Mauro,

This pull request adds printing fwnode parsing debug information and a nop
variant for media_entity_cleanup, which makes a little easier to write
drivers that support operation with and without MC.

Please pull.

The following changes since commit e3ee691dbf24096ea51b3200946b11d68ce75361:

  media: ov5640: add support of RGB565 and YUYV formats (2018-01-05 12:54:14 
-0500)

are available in the git repository at:

  ssh://linuxtv.org/git/sailus/media_tree.git for-4.16-3

for you to fetch changes up to 5a7d72afcc70c9842a3738becec2d76bfbe2eeb4:

  media: entity: Add a nop variant of media_entity_cleanup (2018-01-11 14:14:27 
+0200)


Sakari Ailus (2):
  v4l: fwnode: Add debug prints for V4L2 endpoint property parsing
  media: entity: Add a nop variant of media_entity_cleanup

 drivers/media/i2c/mt9m111.c   |   2 -
 drivers/media/i2c/ov2640.c|   4 --
 drivers/media/i2c/ov2659.c|   4 --
 drivers/media/i2c/ov7670.c|   4 --
 drivers/media/i2c/ov7740.c|   2 -
 drivers/media/i2c/tvp514x.c   |   4 --
 drivers/media/v4l2-core/v4l2-fwnode.c | 101 +++---
 include/media/media-entity.h  |   6 +-
 8 files changed, 85 insertions(+), 42 deletions(-)

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

Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support

[Maxime Ripard]

Hi Yong,

On Thu, Jan 11, 2018 at 09:15:08AM +0800, Yong wrote:
> > On Mon, Jan 08, 2018 at 05:13:39PM +, Hugues FRUCHET wrote:
> > > I'm using a ST board with OV5640 wired in parallel bus output in order 
> > > to interface to my STM32 DCMI parallel interface.
> > > Perhaps could you describe your setup so I could help on understanding 
> > > the problem on your side. From my past experience with this sensor 
> > > module, you can first check hsync/vsync polarities, the datasheet is 
> > > buggy on VSYNC polarity as documented in patch 4/5.
> > 
> > It turns out that it was indeed a polarity issue.
> > 
> > It looks like that in order to operate properly, I need to setup the
> > opposite polarity on HSYNC and VSYNC on the interface. I looked at the
> > signals under a scope, and VSYNC is obviously inversed as you
> > described. HSYNC, I'm not so sure since the HBLANK period seems very
> > long, almost a line.
> > 
> > Since VSYNC at least looks correct, I'd be inclined to think that the
> > polarity is inversed on at least the SoC I'm using it on.
> > 
> > Yong, did you test the V3S CSI driver with a parallel interface? With
> > what sensor driver? Have you found some polarities issues like this?
> 
> Did you try it with Allwinner SoCs?

Yes, on an H3. Looking at all the Allwinner datasheet I could get my
hands on, they are all documented in the same way. However, I really
start to wonder whether the polarity shouldn't be reversed.

At least the fact that VSYNC is clearly active low on the
oscilloscope, while I have to set it active high in the controller
seems like a strong hint :)

> No. I only tested with a BT1120 signal generated by FPGA or ADV7611. HSYNC
> and VSYNC are not used.

Ok, that's good to know :)

> For V3s CSI driver, I will add the following to dt-bindings:
> Endpoint node properties for CSI1
> -
> 
> - remote-endpoint  : (required) a phandle to the bus receiver's endpoint
>   node
> - bus-width:   : (required) must be 8, 10, 12 or 16
> - pclk-sample  : (optional) (default: sample on falling edge)
> - hsync-active : (only required for parallel)
> - vsync-active : (only required for parallel)
> 
> You could try diffrent hsync-active/vsync-active values here.

I did already, and the only combination that works is the one that is
the inversed polarity on HSYNC and VSYNC than what the sensor setup.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support

[Maxime Ripard]

Hi Hugues,

On Wed, Jan 10, 2018 at 03:51:07PM +, Hugues FRUCHET wrote:
> Good news Maxime !
> 
> Have you seen that you can adapt the polarities through devicetree ?
> 
> +   /* Parallel bus endpoint */
> +   ov5640_to_parallel: endpoint {
> [...]
> +   hsync-active = <0>;
> +   vsync-active = <0>;
> +   pclk-sample = <1>;
> +   };

It's what I did, with the polarity infos on both side of the
endpoints.
Here it is:
http://code.bulix.org/4vgchd-257344?raw

You can see that the polarity are inverted on both sides of the link,
which seems weird :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v2 2/2] media: dt-bindings: Add OF properties to ov7670

[Sakari Ailus]

Hi Jacopo,

On Thu, Jan 04, 2018 at 10:52:33AM +0100, Jacopo Mondi wrote:
> Describe newly introduced OF properties for ov7670 image sensor.
> The driver supports two standard properties to configure synchronism
> signals polarities and two custom properties already supported as
> platform data options by the driver.
> ---
>  Documentation/devicetree/bindings/media/i2c/ov7670.txt | 14 ++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov7670.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
> index 826b656..57ded18 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ov7670.txt
> +++ b/Documentation/devicetree/bindings/media/i2c/ov7670.txt
> @@ -9,11 +9,22 @@ Required Properties:
>  - clocks: reference to the xclk input clock.
>  - clock-names: should be "xclk".
>  
> +The following properties, as defined by video interfaces OF bindings
> +"Documentation/devicetree/bindings/media/video-interfaces.txt" are supported:
> +
> +- hsync-active: active state of the HSYNC signal, 0/1 for LOW/HIGH 
> respectively.
> +- vsync-active: active state of the VSYNC signal, 0/1 for LOW/HIGH 
> respectively.

Shouldn't these be mandatory?

If you make them optional, the V4L2 fwnode functions will give you Bt.656
as nothing tells that the signals are there --- which is probably not what
you want. The sensor also supports that, though, if you wish to add support
for it later on.

> +
> +Default is high active state for both vsync and hsync signals.
> +
>  Optional Properties:
>  - reset-gpios: reference to the GPIO connected to the resetb pin, if any.
>Active is low.
>  - powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
>Active is high.
> +- ov7670,pll-bypass: set to 1 to bypass PLL for pixel clock generation.
> +- ov7670,pclk-hb-disable: set to 1 to suppress pixel clock output signal 
> during
> +  horizontal blankings.
>  
>  The device node must contain one 'port' child node for its digital output
>  video port, in accordance with the video interface bindings defined in
> @@ -34,6 +45,9 @@ Example:
>   assigned-clocks = <>;
>   assigned-clock-rates = <2500>;
>  
> + vsync-active = <0>;
> + ov7670,pclk-hb-disable = <1>;
> +
>   port {
>   ov7670_0: endpoint {
>   remote-endpoint = <_0>;
> -- 
> 2.7.4
> 

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

Re: [PATCH v2 0/2] media: ov7670: Implement mbus configuration

[Sakari Ailus]

Hi Jacopo,

On Thu, Jan 04, 2018 at 10:52:31AM +0100, Jacopo Mondi wrote:
> Hello,
>this series adds mbus configuration properties to ov7670 sensor driver.
> 
> I have sent v1 a few days ago and forgot to cc device tree people. Doing it
> now with bindings description and implementation split in 2 separate patches.
> 
> I have fixed Sakari's comment on v1, and I'm sending v2 out with support for
> "pll-bypass" custom property as it was in v1. If we decide it is not worth
> to make an OF property out of it, I will drop it in v3. Technically it is not
> even an mbus configuration option, so I'm fine dropping it eventually.
> 
> Thanks
>   j
> 
> v1->v2:
> - Split bindings description and implementation
> - Addressed Sakari's comments on v1
> - Check for return values of register writes in set_fmt()
> - TODO: decide if "pll-bypass" should be an OF property.

The register lists in the sensor driver likely assume certain external clock
frequency, and the pll-bypass bit can be used to avoid dividing this
frequency by four.

As the driver should be aware of the actual clock frequency as well as the
desired clock frequency, it should be possible for the driver to determine
whether to divide the external clock frequency by four or not.

> 
> Jacopo Mondi (2):
>   v4l2: i2c: ov7670: Implement OF mbus configuration
>   media: dt-bindings: Add OF properties to ov7670
> 
>  .../devicetree/bindings/media/i2c/ov7670.txt   |  14 +++
>  drivers/media/i2c/ov7670.c | 124 
> ++---
>  2 files changed, 124 insertions(+), 14 deletions(-)
> 
> --
> 2.7.4
> 

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

Re: [Linaro-mm-sig] [PATCH] dma-buf: add some lockdep asserts to the reservation object implementation

[Christian König]

Yeah, somehow missed that one.

The patch looks mostly good, except for reservation_object_get_excl().

For that one an RCU protection is usually sufficient, so annotating it 
with reservation_object_assert_held() sounds incorrect to me.


Regards,
Christian.

Am 11.01.2018 um 11:43 schrieb Lucas Stach:

Did this fall through the cracks over the holidays? It really has made
my work much easier while reworking some of the reservation object
handling in etnaviv and I think it might benefit others.

Regards,
Lucas

Am Freitag, den 01.12.2017, 12:12 +0100 schrieb Lucas Stach:

This adds lockdep asserts to the reservation functions which state in their
documentation that obj->lock must be held. Allows builds with PROVE_LOCKING
enabled to check that the locking requirements are met.


Signed-off-by: Lucas Stach 

---
  drivers/dma-buf/reservation.c | 8 
  include/linux/reservation.h   | 2 ++
  2 files changed, 10 insertions(+)

diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index b44d9d7db347..accd398e2ea6 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -71,6 +71,8 @@ int reservation_object_reserve_shared(struct 
reservation_object *obj)

    struct reservation_object_list *fobj, *old;
    u32 max;
  

+   reservation_object_assert_held(obj);

+

    old = reservation_object_get_list(obj);
  

    if (old && old->shared_max) {

@@ -211,6 +213,8 @@ void reservation_object_add_shared_fence(struct 
reservation_object *obj,
  {

    struct reservation_object_list *old, *fobj = obj->staged;
  

+   reservation_object_assert_held(obj);

+

    old = reservation_object_get_list(obj);
    obj->staged = NULL;
  
@@ -236,6 +240,8 @@ void reservation_object_add_excl_fence(struct reservation_object *obj,

    struct reservation_object_list *old;
    u32 i = 0;
  

+   reservation_object_assert_held(obj);

+

    old = reservation_object_get_list(obj);
    if (old)
    i = old->shared_count;

@@ -276,6 +282,8 @@ int reservation_object_copy_fences(struct 
reservation_object *dst,

    size_t size;
    unsigned i;
  

+   reservation_object_assert_held(dst);

+

    rcu_read_lock();
    src_list = rcu_dereference(src->fence);
  
diff --git a/include/linux/reservation.h b/include/linux/reservation.h

index 21fc84d82d41..55e7318800fd 100644
--- a/include/linux/reservation.h
+++ b/include/linux/reservation.h
@@ -212,6 +212,8 @@ reservation_object_unlock(struct reservation_object *obj)
  static inline struct dma_fence *
  reservation_object_get_excl(struct reservation_object *obj)
  {

+   reservation_object_assert_held(obj);

+

    return rcu_dereference_protected(obj->fence_excl,
     reservation_object_held(obj));

  }

___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[linux-next:master 6051/9035] drivers/media/common/videobuf/videobuf2-core.o:(__jump_table+0x10): undefined reference to `__tracepoint_vb2_buf_queue'

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git 
master
head:   b4464bcab38d3f7fe995a7cb960eeac6889bec08
commit: 03fbdb2fc2b8bb27b0ee0534fd3e9c57cdc3854a [6051/9035] media: move 
videobuf2 to drivers/media/common
config: x86_64-randconfig-s5-01110339
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
git checkout 03fbdb2fc2b8bb27b0ee0534fd3e9c57cdc3854a
make ARCH=x86_64  randconfig
make ARCH=x86_64 

All errors (new ones prefixed by >>):

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

[PATCH v2 1/1] media: entity: Add a nop variant of media_entity_cleanup

[Sakari Ailus]

Add nop variant of media_entity_cleanup. This allows calling
media_entity_cleanup whether or not Media controller is enabled,
simplifying driver code.

Also drop #ifdefs on a few drivers around media_entity_cleanup().

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

- Remove "r" at the end of the subject line

 drivers/media/i2c/mt9m111.c  | 2 --
 drivers/media/i2c/ov2640.c   | 4 
 drivers/media/i2c/ov2659.c   | 4 
 drivers/media/i2c/ov7670.c   | 4 
 drivers/media/i2c/ov7740.c   | 2 --
 drivers/media/i2c/tvp514x.c  | 4 
 include/media/media-entity.h | 6 +-
 7 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index d74f254db661..efda1aa95ca0 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -1046,9 +1046,7 @@ static int mt9m111_remove(struct i2c_client *client)
struct mt9m111 *mt9m111 = to_mt9m111(client);
 
v4l2_async_unregister_subdev(>subdev);
-#ifdef CONFIG_MEDIA_CONTROLLER
media_entity_cleanup(>subdev.entity);
-#endif
v4l2_clk_put(mt9m111->clk);
v4l2_ctrl_handler_free(>hdl);
 
diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
index 518868388d65..4c3b92763243 100644
--- a/drivers/media/i2c/ov2640.c
+++ b/drivers/media/i2c/ov2640.c
@@ -1147,9 +1147,7 @@ static int ov2640_probe(struct i2c_client *client,
return 0;
 
 err_videoprobe:
-#if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>subdev.entity);
-#endif
 err_hdl:
v4l2_ctrl_handler_free(>hdl);
 err_clk:
@@ -1163,9 +1161,7 @@ static int ov2640_remove(struct i2c_client *client)
 
v4l2_async_unregister_subdev(>subdev);
v4l2_ctrl_handler_free(>hdl);
-#if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>subdev.entity);
-#endif
v4l2_device_unregister_subdev(>subdev);
clk_disable_unprepare(priv->clk);
return 0;
diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
index 122dd6c5eb38..4715edc8ca33 100644
--- a/drivers/media/i2c/ov2659.c
+++ b/drivers/media/i2c/ov2659.c
@@ -1474,9 +1474,7 @@ static int ov2659_probe(struct i2c_client *client,
 
 error:
v4l2_ctrl_handler_free(>ctrls);
-#if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>entity);
-#endif
mutex_destroy(>lock);
return ret;
 }
@@ -1488,9 +1486,7 @@ static int ov2659_remove(struct i2c_client *client)
 
v4l2_ctrl_handler_free(>ctrls);
v4l2_async_unregister_subdev(sd);
-#if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>entity);
-#endif
mutex_destroy(>lock);
 
return 0;
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index fd229bc8a0e5..28571de1c2f6 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1846,9 +1846,7 @@ static int ov7670_probe(struct i2c_client *client,
return 0;
 
 entity_cleanup:
-#if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>sd.entity);
-#endif
 hdl_free:
v4l2_ctrl_handler_free(>hdl);
 power_off:
@@ -1867,9 +1865,7 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_async_unregister_subdev(sd);
v4l2_ctrl_handler_free(>hdl);
clk_disable_unprepare(info->clk);
-#if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>sd.entity);
-#endif
ov7670_s_power(sd, 0);
return 0;
 }
diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
index 0308ba437bbb..576ce0640297 100644
--- a/drivers/media/i2c/ov7740.c
+++ b/drivers/media/i2c/ov7740.c
@@ -1148,9 +1148,7 @@ static int ov7740_remove(struct i2c_client *client)
 
mutex_destroy(>mutex);
v4l2_ctrl_handler_free(ov7740->subdev.ctrl_handler);
-#if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>subdev.entity);
-#endif
v4l2_async_unregister_subdev(sd);
ov7740_free_controls(ov7740);
 
diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
index d575b3e7e835..8b0aa9297bde 100644
--- a/drivers/media/i2c/tvp514x.c
+++ b/drivers/media/i2c/tvp514x.c
@@ -1131,9 +1131,7 @@ tvp514x_probe(struct i2c_client *client, const struct 
i2c_device_id *id)
 done:
if (ret < 0) {
v4l2_ctrl_handler_free(>hdl);
-#if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>sd.entity);
-#endif
}
return ret;
 }
@@ -1151,9 +1149,7 @@ static int tvp514x_remove(struct i2c_client *client)
struct tvp514x_decoder *decoder = to_decoder(sd);
 
v4l2_async_unregister_subdev(>sd);
-#if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>sd.entity);
-#endif
v4l2_ctrl_handler_free(>hdl);
return 0;
 }
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index d7a669058b5e..a732af1dbba0 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -634,7 +634,11 

Re: [PATCH 00/18] prevent bounds-check bypass via speculative execution

[Jiri Kosina]

On Tue, 9 Jan 2018, Josh Poimboeuf wrote:

> On Tue, Jan 09, 2018 at 11:44:05AM -0800, Dan Williams wrote:
> > On Tue, Jan 9, 2018 at 11:34 AM, Jiri Kosina  wrote:
> > > On Fri, 5 Jan 2018, Dan Williams wrote:
> > >
> > > [ ... snip ... ]
> > >> Andi Kleen (1):
> > >>   x86, barrier: stop speculation for failed access_ok
> > >>
> > >> Dan Williams (13):
> > >>   x86: implement nospec_barrier()
> > >>   [media] uvcvideo: prevent bounds-check bypass via speculative 
> > >> execution
> > >>   carl9170: prevent bounds-check bypass via speculative execution
> > >>   p54: prevent bounds-check bypass via speculative execution
> > >>   qla2xxx: prevent bounds-check bypass via speculative execution
> > >>   cw1200: prevent bounds-check bypass via speculative execution
> > >>   Thermal/int340x: prevent bounds-check bypass via speculative 
> > >> execution
> > >>   ipv6: prevent bounds-check bypass via speculative execution
> > >>   ipv4: prevent bounds-check bypass via speculative execution
> > >>   vfs, fdtable: prevent bounds-check bypass via speculative execution
> > >>   net: mpls: prevent bounds-check bypass via speculative execution
> > >>   udf: prevent bounds-check bypass via speculative execution
> > >>   userns: prevent bounds-check bypass via speculative execution
> > >>
> > >> Mark Rutland (4):
> > >>   asm-generic/barrier: add generic nospec helpers
> > >>   Documentation: document nospec helpers
> > >>   arm64: implement nospec_ptr()
> > >>   arm: implement nospec_ptr()
> > >
> > > So considering the recent publication of [1], how come we all of a sudden
> > > don't need the barriers in ___bpf_prog_run(), namely for LD_IMM_DW and
> > > LDX_MEM_##SIZEOP, and something comparable for eBPF JIT?
> > >
> > > Is this going to be handled in eBPF in some other way?
> > >
> > > Without that in place, and considering Jann Horn's paper, it would seem
> > > like PTI doesn't really lock it down fully, right?
> > 
> > Here is the latest (v3) bpf fix:
> > 
> > https://patchwork.ozlabs.org/patch/856645/
> > 
> > I currently have v2 on my 'nospec' branch and will move that to v3 for
> > the next update, unless it goes upstream before then.

Daniel, I guess you're planning to send this still for 4.15?

> That patch seems specific to CONFIG_BPF_SYSCALL.  Is the bpf() syscall
> the only attack vector?  Or are there other ways to run bpf programs
> that we should be worried about?

Seems like Alexei is probably the only person in the whole universe who 
isn't CCed here ... let's fix that.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support

[Sakari Ailus]

On Thu, Jan 11, 2018 at 08:25:41AM +, Hugues FRUCHET wrote:
> On 01/11/2018 09:19 AM, Sakari Ailus wrote:
> > On Thu, Jan 11, 2018 at 08:12:11AM +, Hugues FRUCHET wrote:
> >> Hi Sakari,
> >>
> >> On 01/10/2018 11:25 PM, Sakari Ailus wrote:
> >>> Hi Hugues,
> >>>
> >>> On Wed, Jan 10, 2018 at 03:51:07PM +, Hugues FRUCHET wrote:
>  Good news Maxime !
> 
>  Have you seen that you can adapt the polarities through devicetree ?
> 
>  +   /* Parallel bus endpoint */
>  +   ov5640_to_parallel: endpoint {
>  [...]
>  +   hsync-active = <0>;
>  +   vsync-active = <0>;
>  +   pclk-sample = <1>;
>  +   };
> 
>  Doing so you can adapt to your SoC/board setup easily.
> 
>  If you don't put those lines in devicetree, the ov5640 default init
>  sequence is used which set the polarity as defined in below comment:
>  ov5640_set_stream_dvp()
>  [...]
>  +* Control lines polarity can be configured through
>  +* devicetree endpoint control lines properties.
>  +* If no endpoint control lines properties are set,
>  +* polarity will be as below:
>  +* - VSYNC: active high
>  +* - HREF:  active low
>  +* - PCLK:  active low
>  +*/
>  [...]
> >>>
> >>> The properties are at the moment documented as mandatory in DT binding
> >>> documentation.
> >>>
> >> of course, it was just to ask Maxime to check the devicetree on its
> >> side, the symptom observed by Maxime with hsync/vsync inversed is the
> >> same than the one observed if we stick to just default init sequence.
> > 
> > I wonder if the driver should be changed to require hsync and vsync. These
> > signals won't be there at all in Bt.656 mode.
> > 
> I will revisit this when pushing Bt.656 mode.

That may lead to a situation where DT source which currently intends to use
parallel bus with sync signals will automatically switch to Bt.656. That
could be an issue for the receiver.

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

Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support

[Hugues FRUCHET]

On 01/11/2018 09:19 AM, Sakari Ailus wrote:
> On Thu, Jan 11, 2018 at 08:12:11AM +, Hugues FRUCHET wrote:
>> Hi Sakari,
>>
>> On 01/10/2018 11:25 PM, Sakari Ailus wrote:
>>> Hi Hugues,
>>>
>>> On Wed, Jan 10, 2018 at 03:51:07PM +, Hugues FRUCHET wrote:
 Good news Maxime !

 Have you seen that you can adapt the polarities through devicetree ?

 +   /* Parallel bus endpoint */
 +   ov5640_to_parallel: endpoint {
 [...]
 +   hsync-active = <0>;
 +   vsync-active = <0>;
 +   pclk-sample = <1>;
 +   };

 Doing so you can adapt to your SoC/board setup easily.

 If you don't put those lines in devicetree, the ov5640 default init
 sequence is used which set the polarity as defined in below comment:
 ov5640_set_stream_dvp()
 [...]
 +* Control lines polarity can be configured through
 +* devicetree endpoint control lines properties.
 +* If no endpoint control lines properties are set,
 +* polarity will be as below:
 +* - VSYNC: active high
 +* - HREF:  active low
 +* - PCLK:  active low
 +*/
 [...]
>>>
>>> The properties are at the moment documented as mandatory in DT binding
>>> documentation.
>>>
>> of course, it was just to ask Maxime to check the devicetree on its
>> side, the symptom observed by Maxime with hsync/vsync inversed is the
>> same than the one observed if we stick to just default init sequence.
> 
> I wonder if the driver should be changed to require hsync and vsync. These
> signals won't be there at all in Bt.656 mode.
> 
I will revisit this when pushing Bt.656 mode.

Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support

[Sakari Ailus]

On Thu, Jan 11, 2018 at 08:12:11AM +, Hugues FRUCHET wrote:
> Hi Sakari,
> 
> On 01/10/2018 11:25 PM, Sakari Ailus wrote:
> > Hi Hugues,
> > 
> > On Wed, Jan 10, 2018 at 03:51:07PM +, Hugues FRUCHET wrote:
> >> Good news Maxime !
> >>
> >> Have you seen that you can adapt the polarities through devicetree ?
> >>
> >> +   /* Parallel bus endpoint */
> >> +   ov5640_to_parallel: endpoint {
> >> [...]
> >> +   hsync-active = <0>;
> >> +   vsync-active = <0>;
> >> +   pclk-sample = <1>;
> >> +   };
> >>
> >> Doing so you can adapt to your SoC/board setup easily.
> >>
> >> If you don't put those lines in devicetree, the ov5640 default init
> >> sequence is used which set the polarity as defined in below comment:
> >> ov5640_set_stream_dvp()
> >> [...]
> >> +* Control lines polarity can be configured through
> >> +* devicetree endpoint control lines properties.
> >> +* If no endpoint control lines properties are set,
> >> +* polarity will be as below:
> >> +* - VSYNC: active high
> >> +* - HREF:  active low
> >> +* - PCLK:  active low
> >> +*/
> >> [...]
> > 
> > The properties are at the moment documented as mandatory in DT binding
> > documentation.
> > 
> of course, it was just to ask Maxime to check the devicetree on its 
> side, the symptom observed by Maxime with hsync/vsync inversed is the 
> same than the one observed if we stick to just default init sequence.

I wonder if the driver should be changed to require hsync and vsync. These
signals won't be there at all in Bt.656 mode.

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

Re: [PATCH v5 0/5] Add OV5640 parallel interface and RGB565/YUYV support

[Hugues FRUCHET]

Hi Sakari,

On 01/10/2018 11:25 PM, Sakari Ailus wrote:
> Hi Hugues,
> 
> On Wed, Jan 10, 2018 at 03:51:07PM +, Hugues FRUCHET wrote:
>> Good news Maxime !
>>
>> Have you seen that you can adapt the polarities through devicetree ?
>>
>> +   /* Parallel bus endpoint */
>> +   ov5640_to_parallel: endpoint {
>> [...]
>> +   hsync-active = <0>;
>> +   vsync-active = <0>;
>> +   pclk-sample = <1>;
>> +   };
>>
>> Doing so you can adapt to your SoC/board setup easily.
>>
>> If you don't put those lines in devicetree, the ov5640 default init
>> sequence is used which set the polarity as defined in below comment:
>> ov5640_set_stream_dvp()
>> [...]
>> +* Control lines polarity can be configured through
>> +* devicetree endpoint control lines properties.
>> +* If no endpoint control lines properties are set,
>> +* polarity will be as below:
>> +* - VSYNC: active high
>> +* - HREF:  active low
>> +* - PCLK:  active low
>> +*/
>> [...]
> 
> The properties are at the moment documented as mandatory in DT binding
> documentation.
> 
of course, it was just to ask Maxime to check the devicetree on its 
side, the symptom observed by Maxime with hsync/vsync inversed is the 
same than the one observed if we stick to just default init sequence.

BR,
Hugues.