Re: [PATCH v2 00/12] media: ov5640: Misc cleanup and improvements

[Samuel Bobrowicz]

Hi Maxime,

I applied your patches, and they are a big improvement for what I am
trying to do, but things still aren't working right on my platform.

How confident are you that the MIPI mode will work with this version
of the driver? I am having issues that I believe are due to incorrect
clock generation. Our engineers did some reverse engineering of the
clock tree themselves, and came up with a slightly different model.
I've captured their model in a spreadsheet here:
https://tinyurl.com/pll-calc . Just modify the register and xclk
values to see the clocks change. Do your tests disagree with this
potential model?

I'm not sure which model is more correct, but my tests suggest the
high speed MIPI clock is generated directly off the PLL. This means
the PLL multiplier you are generating in your algorithm is not high
enough to satisfy the bandwidth. If this is the case, MIPI mode will
require a different set of parameters that enable some of the
downstream dividers, so that the PLL multiplier can be higher while
the PCLK value still matches the needed rate calculated from the
resolution.

Any thoughts on this before I dive in and start tweaking the algorithm
in mipi mode?

Sam
---
Sam Bobrowicz
Elite Embedded Consulting LLC
elite-embedded.com


On Tue, Apr 17, 2018 at 9:01 AM, Maxime Ripard
 wrote:
> On Mon, Apr 16, 2018 at 04:22:39PM -0700, Samuel Bobrowicz wrote:
>> I've been digging around the ov5640.c code for a few weeks now, these
>> look like some solid improvements. I'll give them a shot and let you
>> know how they work.
>
> Great, thanks!
>
>> On that note, I'm bringing up a module that uses dual lane MIPI with a
>> 12MHz fixed oscillator for xclk (Digilent's Pcam 5c). The mainline
>> version of the driver seems to only support xclk of 22MHz (or maybe
>> 24MHz), despite allowing xclk values from 6-24MHz. Will any of these
>> patches add support for a 12MHz xclk while in MIPI mode?
>
> My setup has a 24MHz crystal, and work with a parallel bus so I
> haven't been able to test yours. However, yeah, I guess my patches
> will improve your situation a lot.
>
> Maxime
>
> --
> Maxime Ripard, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com

Re: [PATCH 09/13] imx274: get rid of mode_index

[kbuild test robot]

Hi Luca,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.17-rc1 next-20180418]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Luca-Ceresoli/imx274-add-cropping-and-misc-improvements/20180413-234459
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-h1-04190052 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/dynamic_debug.h:6:0,
from include/linux/printk.h:329,
from include/linux/kernel.h:14,
from include/linux/clk.h:16,
from drivers/media//i2c/imx274.c:22:
   drivers/media//i2c/imx274.c: In function 'imx274_s_stream':
>> include/linux/jump_label.h:407:59: warning: format '%lu' expects argument of 
>> type 'long unsigned int', but argument 6 has type 'int' [-Wformat=]
 else if (__builtin_types_compatible_p(typeof(*x), struct 
static_key_false)) \
  ^
   include/linux/dynamic_debug.h:103:2: note: in expansion of macro 
'static_branch_unlikely'
 static_branch_unlikely(_key_false)
 ^
   include/linux/dynamic_debug.h:134:6: note: in expansion of macro 
'DYNAMIC_DEBUG_BRANCH'
 if (DYNAMIC_DEBUG_BRANCH(descriptor))   \
 ^
   include/linux/device.h:1364:2: note: in expansion of macro 'dynamic_dev_dbg'
 dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
 ^
   drivers/media//i2c/imx274.c:1027:2: note: in expansion of macro 'dev_dbg'
 dev_dbg(>client->dev, "%s : %s, mode index %lu\n", __func__,
 ^
--
   In file included from include/linux/dynamic_debug.h:6:0,
from include/linux/printk.h:329,
from include/linux/kernel.h:14,
from include/linux/clk.h:16,
from drivers/media/i2c/imx274.c:22:
   drivers/media/i2c/imx274.c: In function 'imx274_s_stream':
>> include/linux/jump_label.h:407:59: warning: format '%lu' expects argument of 
>> type 'long unsigned int', but argument 6 has type 'int' [-Wformat=]
 else if (__builtin_types_compatible_p(typeof(*x), struct 
static_key_false)) \
  ^
   include/linux/dynamic_debug.h:103:2: note: in expansion of macro 
'static_branch_unlikely'
 static_branch_unlikely(_key_false)
 ^
   include/linux/dynamic_debug.h:134:6: note: in expansion of macro 
'DYNAMIC_DEBUG_BRANCH'
 if (DYNAMIC_DEBUG_BRANCH(descriptor))   \
 ^
   include/linux/device.h:1364:2: note: in expansion of macro 'dynamic_dev_dbg'
 dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
 ^
   drivers/media/i2c/imx274.c:1027:2: note: in expansion of macro 'dev_dbg'
 dev_dbg(>client->dev, "%s : %s, mode index %lu\n", __func__,
 ^

vim +407 include/linux/jump_label.h

11276d53 Peter Zijlstra 2015-07-24  401  
11276d53 Peter Zijlstra 2015-07-24  402  #define static_branch_unlikely(x)  
\
11276d53 Peter Zijlstra 2015-07-24  403  ({ 
\
11276d53 Peter Zijlstra 2015-07-24  404 bool branch;
\
11276d53 Peter Zijlstra 2015-07-24  405 if 
(__builtin_types_compatible_p(typeof(*x), struct static_key_true))   \
11276d53 Peter Zijlstra 2015-07-24  406 branch = 
arch_static_branch_jump(&(x)->key, false); \
11276d53 Peter Zijlstra 2015-07-24 @407 else if 
(__builtin_types_compatible_p(typeof(*x), struct static_key_false)) \
11276d53 Peter Zijlstra 2015-07-24  408 branch = 
arch_static_branch(&(x)->key, false);  \
11276d53 Peter Zijlstra 2015-07-24  409 else
\
11276d53 Peter Zijlstra 2015-07-24  410 branch = 
wrong_branch_error();  \
81dcf89f Peter Zijlstra 2018-01-18  411 unlikely(branch);   
\
11276d53 Peter Zijlstra 2015-07-24  412  })
11276d53 Peter Zijlstra 2015-07-24  413  

:: The code at line 407 was first introduced by commit
:: 11276d5306b8e5b438a36bbff855fe792d7eaa61 locking/static_keys: Add a new 
static_key interface

:: TO: Peter Zijlstra <pet...@infradead.org>
:: CC: Ingo Molnar <mi...@kernel.org>

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


.config.gz
Description: application/gzip

[PATCH] media: si470x: fix a typo at the Makefile causing build issues

[Mauro Carvalho Chehab]

Instead of +=, the rule had :=, with actually disables build
of everything else.

Fixes: 58757984ca3c ("media: si470x: allow build both USB and I2C at the same 
time")
Reported-by: Daniel Scheller 
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/radio/si470x/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/radio/si470x/Makefile 
b/drivers/media/radio/si470x/Makefile
index 563500823e04..682b3146397e 100644
--- a/drivers/media/radio/si470x/Makefile
+++ b/drivers/media/radio/si470x/Makefile
@@ -2,6 +2,6 @@
 # Makefile for radios with Silicon Labs Si470x FM Radio Receivers
 #
 
-obj-$(CONFIG_RADIO_SI470X) := radio-si470x-common.o
+obj-$(CONFIG_RADIO_SI470X) += radio-si470x-common.o
 obj-$(CONFIG_USB_SI470X) += radio-si470x-usb.o
 obj-$(CONFIG_I2C_SI470X) += radio-si470x-i2c.o
-- 
2.14.3

Re: [PATCH v2 18/19] media: si470x: allow build both USB and I2C at the same time

[Mauro Carvalho Chehab]

Em Wed, 18 Apr 2018 21:06:12 +0200
Daniel Scheller  escreveu:

> Am Wed, 18 Apr 2018 15:53:09 -0300
> schrieb Mauro Carvalho Chehab :
> 
> > Em Wed, 18 Apr 2018 19:07:40 +0200
> > Daniel Scheller  escreveu:
> >   
> > > Am Fri, 6 Apr 2018 13:46:03 -0300
> > > schrieb Mauro Carvalho Chehab :
> > > 
> > > > Em Sat, 7 Apr 2018 00:21:07 +0800
> > > > kbuild test robot  escreveu:
> > > >   
> > > > > Hi Mauro,
> > > > > 
> > > > > I love your patch! Yet something to improve:
> > > > > [...]  
> > > > 
> > > > Fixed patch enclosed.
> > > > 
> > > > Thanks,
> > > > Mauro
> > > > 
> > > > [PATCH] media: si470x: allow build both USB and I2C at the same
> > > > time
> > > > 
> > > > Currently, either USB or I2C is built. Change it to allow
> > > > having both enabled at the same time.
> > > > 
> > > > The main reason is that COMPILE_TEST all[yes/mod]builds will
> > > > now contain all drivers under drivers/media.
> > > > 
> > > > Signed-off-by: Mauro Carvalho Chehab
> > > >   
> > > 
> > > FWIW, this patch (which seemingly is commit
> > > 58757984ca3c73284a45dd53ac66f1414057cd09 in media_tree.git) seems
> > > to break media_build in a way that on my systems only 20 drivers
> > > and modules are built now, while it should be in the 650+ modules
> > > range. Hans' automated daily testbuilds suffer from the same issue,
> > > looking at todays daily build logs (Wednesday.tar.bz2). I
> > > personally build against Kernel 4.16.2 on Gentoo.
> > > 
> > > This specific commit/patch was found using
> > > 
> > >   # git bisect good v4.17-rc1
> > >   # git bisect bad media_tree/master
> > > 
> > > And, "git revert 58767984..." makes all drivers being built again by
> > > media_build.
> > > 
> > > Not sure if there's something other for which this patch acts as the
> > > trigger of if this needs adaption in media_build, though I thought
> > > reporting this doesn't hurt.
> > > 
> > > Best regards,
> > > Daniel Scheller
> > 
> > Please try this:
> > 
> > diff --git a/drivers/media/radio/si470x/Makefile
> > b/drivers/media/radio/si470x/Makefile index
> > 563500823e04..682b3146397e 100644 ---
> > a/drivers/media/radio/si470x/Makefile +++
> > b/drivers/media/radio/si470x/Makefile @@ -2,6 +2,6 @@
> >  # Makefile for radios with Silicon Labs Si470x FM Radio Receivers
> >  #
> >  
> > -obj-$(CONFIG_RADIO_SI470X) := radio-si470x-common.o
> > +obj-$(CONFIG_RADIO_SI470X) += radio-si470x-common.o
> >  obj-$(CONFIG_USB_SI470X) += radio-si470x-usb.o
> >  obj-$(CONFIG_I2C_SI470X) += radio-si470x-i2c.o  
> 
> That (ontop of media_tree.git HEAD) fixes it, back to 656 modules.

Good! I'll merge this with a proper description and apply ASAP.

Regards,
Mauro

Re: [PATCH v2 18/19] media: si470x: allow build both USB and I2C at the same time

[Daniel Scheller]

Am Wed, 18 Apr 2018 15:53:09 -0300
schrieb Mauro Carvalho Chehab :

> Em Wed, 18 Apr 2018 19:07:40 +0200
> Daniel Scheller  escreveu:
> 
> > Am Fri, 6 Apr 2018 13:46:03 -0300
> > schrieb Mauro Carvalho Chehab :
> >   
> > > Em Sat, 7 Apr 2018 00:21:07 +0800
> > > kbuild test robot  escreveu:
> > > 
> > > > Hi Mauro,
> > > > 
> > > > I love your patch! Yet something to improve:
> > > > [...]
> > > 
> > > Fixed patch enclosed.
> > > 
> > > Thanks,
> > > Mauro
> > > 
> > > [PATCH] media: si470x: allow build both USB and I2C at the same
> > > time
> > > 
> > > Currently, either USB or I2C is built. Change it to allow
> > > having both enabled at the same time.
> > > 
> > > The main reason is that COMPILE_TEST all[yes/mod]builds will
> > > now contain all drivers under drivers/media.
> > > 
> > > Signed-off-by: Mauro Carvalho Chehab
> > > 
> > 
> > FWIW, this patch (which seemingly is commit
> > 58757984ca3c73284a45dd53ac66f1414057cd09 in media_tree.git) seems
> > to break media_build in a way that on my systems only 20 drivers
> > and modules are built now, while it should be in the 650+ modules
> > range. Hans' automated daily testbuilds suffer from the same issue,
> > looking at todays daily build logs (Wednesday.tar.bz2). I
> > personally build against Kernel 4.16.2 on Gentoo.
> > 
> > This specific commit/patch was found using
> > 
> >   # git bisect good v4.17-rc1
> >   # git bisect bad media_tree/master
> > 
> > And, "git revert 58767984..." makes all drivers being built again by
> > media_build.
> > 
> > Not sure if there's something other for which this patch acts as the
> > trigger of if this needs adaption in media_build, though I thought
> > reporting this doesn't hurt.
> > 
> > Best regards,
> > Daniel Scheller  
> 
> Please try this:
> 
> diff --git a/drivers/media/radio/si470x/Makefile
> b/drivers/media/radio/si470x/Makefile index
> 563500823e04..682b3146397e 100644 ---
> a/drivers/media/radio/si470x/Makefile +++
> b/drivers/media/radio/si470x/Makefile @@ -2,6 +2,6 @@
>  # Makefile for radios with Silicon Labs Si470x FM Radio Receivers
>  #
>  
> -obj-$(CONFIG_RADIO_SI470X) := radio-si470x-common.o
> +obj-$(CONFIG_RADIO_SI470X) += radio-si470x-common.o
>  obj-$(CONFIG_USB_SI470X) += radio-si470x-usb.o
>  obj-$(CONFIG_I2C_SI470X) += radio-si470x-i2c.o

That (ontop of media_tree.git HEAD) fixes it, back to 656 modules.

Thanks!

Best regards,
Daniel Scheller
-- 
https://github.com/herrnst

Re: [PATCH v2 18/19] media: si470x: allow build both USB and I2C at the same time

[Mauro Carvalho Chehab]

Em Wed, 18 Apr 2018 19:07:40 +0200
Daniel Scheller  escreveu:

> Am Fri, 6 Apr 2018 13:46:03 -0300
> schrieb Mauro Carvalho Chehab :
> 
> > Em Sat, 7 Apr 2018 00:21:07 +0800
> > kbuild test robot  escreveu:
> >   
> > > Hi Mauro,
> > > 
> > > I love your patch! Yet something to improve:
> > > [...]  
> > 
> > Fixed patch enclosed.
> > 
> > Thanks,
> > Mauro
> > 
> > [PATCH] media: si470x: allow build both USB and I2C at the same time
> > 
> > Currently, either USB or I2C is built. Change it to allow
> > having both enabled at the same time.
> > 
> > The main reason is that COMPILE_TEST all[yes/mod]builds will
> > now contain all drivers under drivers/media.
> > 
> > Signed-off-by: Mauro Carvalho Chehab   
> 
> FWIW, this patch (which seemingly is commit
> 58757984ca3c73284a45dd53ac66f1414057cd09 in media_tree.git) seems to break 
> media_build in a way that on my systems only 20 drivers and modules are built 
> now, while it should be in the 650+ modules range. Hans' automated daily 
> testbuilds suffer from the same issue, looking at todays daily build logs 
> (Wednesday.tar.bz2). I personally build against Kernel 4.16.2 on Gentoo.
> 
> This specific commit/patch was found using
> 
>   # git bisect good v4.17-rc1
>   # git bisect bad media_tree/master
> 
> And, "git revert 58767984..." makes all drivers being built again by
> media_build.
> 
> Not sure if there's something other for which this patch acts as the
> trigger of if this needs adaption in media_build, though I thought
> reporting this doesn't hurt.
> 
> Best regards,
> Daniel Scheller

Please try this:

diff --git a/drivers/media/radio/si470x/Makefile 
b/drivers/media/radio/si470x/Makefile
index 563500823e04..682b3146397e 100644
--- a/drivers/media/radio/si470x/Makefile
+++ b/drivers/media/radio/si470x/Makefile
@@ -2,6 +2,6 @@
 # Makefile for radios with Silicon Labs Si470x FM Radio Receivers
 #
 
-obj-$(CONFIG_RADIO_SI470X) := radio-si470x-common.o
+obj-$(CONFIG_RADIO_SI470X) += radio-si470x-common.o
 obj-$(CONFIG_USB_SI470X) += radio-si470x-usb.o
 obj-$(CONFIG_I2C_SI470X) += radio-si470x-i2c.o





Thanks,
Mauro

[PATCH v3] videobuf: Change return type to vm_fault_t

[Souptick Joarder]

Use new return type vm_fault_t for fault handler. For
now, this is just documenting that the function returns
a VM_FAULT value rather than an errno. Once all instances
are converted, vm_fault_t will become a distinct type.

Reference id -> 1c8f422059ae ("mm: change return type to
vm_fault_t")

Signed-off-by: Souptick Joarder 
Acked-by: Sakari Ailus 
---
v2: Updated patch subject

v3: Correcting patch subject

 drivers/media/v4l2-core/videobuf-dma-sg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c 
b/drivers/media/v4l2-core/videobuf-dma-sg.c
index f412429..54257ea 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -435,7 +435,7 @@ static void videobuf_vm_close(struct vm_area_struct *vma)
  * now ...).  Bounce buffers don't work very well for the data rates
  * video capture has.
  */
-static int videobuf_vm_fault(struct vm_fault *vmf)
+static vm_fault_t videobuf_vm_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
struct page *page;
--
1.9.1

Re: [PATCH v2] media: v4l2-core: videobuf-dma-sg: Change return type to vm_fault_t

[Matthew Wilcox]

On Wed, Apr 18, 2018 at 11:49:19PM +0530, Souptick Joarder wrote:
> Use new return type vm_fault_t for fault handler. For
> now, this is just documenting that the function returns
> a VM_FAULT value rather than an errno. Once all instances
> are converted, vm_fault_t will become a distinct type.
> 
> Reference id -> 1c8f422059ae ("mm: change return type to
> vm_fault_t")
> 
> Signed-off-by: Souptick Joarder 
> Acked-by: Sakari Ailus 
> ---
> v2: Updated patch subject

I'm pretty sure what Sakari meant was:

videobuf: Change return type to vm_fault_t

Re: cx88 invalid video opcodes when VBI enabled

[Daniel Glöckner]

Hi,

On Tue, Apr 17, 2018 at 09:49:22PM -0400, Devin Heitmueller wrote:
> [   68.750809] cx88[0]: video y / packed - dma channel status dump
> [   68.750811] cx88[0]:   cmds: initial risc: 0x8aa98000
> [   68.750813] cx88[0]:   cmds: cdt base: 0x00180440
> [   68.750815] cx88[0]:   cmds: cdt size: 0x000c
> [   68.750816] cx88[0]:   cmds: iq base : 0x00180400
[...]
> [   68.750887] cx88[0]: vbi - dma channel status dump
[...]
> [   68.750899] cx88[0]:   cmds: iq wr ptr   : 0x017f
> [   68.750901] cx88[0]:   cmds: iq rd ptr   : 0x0011
> [   68.750902] cx88[0]:   cmds: cdt current : 0x0638
[...]
> [   68.750910] cx88[0]:   risc0: 0x8aa98000 [ sync sol irq2 23 21 19 cnt0 
> resync count=0 ]
> [   68.750913] cx88[0]:   risc1: 0x00180440 [ INVALID 20 19 count=1088 ]
> [   68.750915] cx88[0]:   risc2: 0x000c [ INVALID count=12 ]
> [   68.750918] cx88[0]:   risc3: 0x00180400 [ INVALID 20 19 count=1024 ]

The VBI instruction queue read pointer points outside the VBI instruction
queue and into the video y/packed CMDS (to 0x18+0x11*4). The values
next to the iq rd ptr look ok.

We only initialize the iq rd ptr to zero in cx88_sram_channel_setup and
then never touch it again. The hardware takes care of updating it.
Maybe cx88_sram_channel_setup is sometimes called for channel 24 while the
VBI risc engine is still running? One could try adding a WARN_ONCE at the
top of the function to catch that case.

Best regards,

  Daniel

[PATCH v2] media: v4l2-core: videobuf-dma-sg: Change return type to vm_fault_t

[Souptick Joarder]

Use new return type vm_fault_t for fault handler. For
now, this is just documenting that the function returns
a VM_FAULT value rather than an errno. Once all instances
are converted, vm_fault_t will become a distinct type.

Reference id -> 1c8f422059ae ("mm: change return type to
vm_fault_t")

Signed-off-by: Souptick Joarder 
Acked-by: Sakari Ailus 
---
v2: Updated patch subject

 drivers/media/v4l2-core/videobuf-dma-sg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c 
b/drivers/media/v4l2-core/videobuf-dma-sg.c
index f412429..54257ea 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -435,7 +435,7 @@ static void videobuf_vm_close(struct vm_area_struct *vma)
  * now ...).  Bounce buffers don't work very well for the data rates
  * video capture has.
  */
-static int videobuf_vm_fault(struct vm_fault *vmf)
+static vm_fault_t videobuf_vm_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
struct page *page;
--
1.9.1

Re: [PATCH v2 7/7] media: rc: mceusb: allow the timeout to be configurable

[Matthias Reichl]

Hi Sean,

On Wed, Apr 18, 2018 at 12:24:29PM +0100, Sean Young wrote:
> Hello Hias,
> 
> On Tue, Apr 17, 2018 at 09:14:57PM +0200, Matthias Reichl wrote:
> > On Sun, Apr 08, 2018 at 10:19:42PM +0100, Sean Young wrote:
> > > mceusb devices have a default timeout of 100ms, but this can be changed.
> > 
> > We finally added a backport of the v2 series (and also the mce_kbd
> > series) to LibreELEC yesterday and ratcher quickly received 2 bugreports
> > from users using mceusb receivers.
> > 
> > Local testing on RPi/gpio-ir and Intel NUC/ite-cir was fine, I've
> > been using the v2 series for over a week without issues on
> > LibreELEC (RPi with kernel 4.14).
> > 
> > Here are the links to the bugreports and logs:
> > https://forum.kodi.tv/showthread.php?tid=298461=2726684#pid2726684
> > https://forum.kodi.tv/showthread.php?tid=298462=2726690#pid2726690
> > 
> > Both users are using similar mceusb receivers:
> > 
> > Log 1:
> > [6.418218] rc rc0: Media Center Ed. eHome Infrared Remote Transceiver 
> > (147a:e017) as 
> > /devices/platform/soc/3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/rc/rc0
> > [6.418358] input: Media Center Ed. eHome Infrared Remote Transceiver 
> > (147a:e017) as 
> > /devices/platform/soc/3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/rc/rc0/input0
> > [6.419443] rc rc0: lirc_dev: driver ir-lirc-codec (mceusb) registered 
> > at minor = 0
> > [6.608114] mceusb 1-1.3:1.0: Registered Formosa21 SnowflakeEmulation 
> > with mce emulator interface version 1
> > [6.608125] mceusb 1-1.3:1.0: 0 tx ports (0x0 cabled) and 1 rx sensors 
> > (0x1 active)
> > 
> > Log 2:
> > [3.023361] rc rc0: Media Center Ed. eHome Infrared Remote Transceiver 
> > (147a:e03e) as /devices/pci:00/:00:14.0/usb1/1-10/1-10:1.0/rc/rc0
> > [3.023393] input: Media Center Ed. eHome Infrared Remote Transceiver 
> > (147a:e03e) as 
> > /devices/pci:00/:00:14.0/usb1/1-10/1-10:1.0/rc/rc0/input11
> > [3.023868] rc rc0: lirc_dev: driver ir-lirc-codec (mceusb) registered 
> > at minor = 0
> > [3.119384] input: eventlircd as /devices/virtual/input/input21
> > [3.138625] ip6_tables: (C) 2000-2006 Netfilter Core Team
> > [3.196830] mceusb 1-10:1.0: Registered Formosa21 eHome Infrared 
> > Transceiver with mce emulator interface version 2
> > [3.196836] mceusb 1-10:1.0: 0 tx ports (0x0 cabled) and 1 rx sensors 
> > (0x1 active)
> > 
> > In both cases ir-keytable doesn't report any scancodes and the
> > ir-ctl -r output contains very odd long space values where I'd expect
> > a short timeout instead:
> > 
> > gap between messages:
> > space 800
> > pulse 450
> > space 16777215
> > space 25400
> > pulse 2650
> > space 800
> > 
> > end of last message:
> > space 800
> > pulse 450
> > space 16777215
> > timeout 31750
> > 
> > This patch applied cleanly on 4.14 and the mceusb history from
> > 4.14 to media/master looked rather unsuspicious. I'm not 100% sure
> > if I might have missed a dependency when backporting the patch
> > or if this is indeed an issue of this patch on these particular
> > (or maybe some more) mceusb receivers.
> 
> Thanks again for a great bug report and analysis! So, it seems with the
> shorter timeout, some mceusb devices add a specific "timeout" code to
> the IR data stream (0x80) rather than a space. The current mceusb code
> resets the decoders in this case, causing the IR decoders to reset and
> lirc to report a space of 0xff.
> 
> Turns out that one of my mceusb devices also suffers from this, I don't
> know how I missed this. Anyway hopefully this will solve the problem.

Thanks a lot for the quick fix!

I can't test myself as I don't have the hardware, but we will
include the patch in tonight's LibreELEC build and I asked the
users to test it. I'll keep you posted about the outcome.

so long,

Hias

> 
> 
> Thanks
> 
> Sean
> 
> >>From 92d27b206e51993e927dc0b3aba210a621eef3d0 Mon Sep 17 00:00:00 2001
> From: Sean Young 
> Date: Wed, 18 Apr 2018 10:36:25 +0100
> Subject: [PATCH] media: rc: mceusb: IR of length 0 means IR timeout, not reset
> 
> The last usb packet with IR data will end with 0x80 (MCE_IRDATA_TRAILER).
> If we reset the decoder state at this point, IR decoding can fail.
> 
> Signed-off-by: Sean Young 
> ---
>  drivers/media/rc/mceusb.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
> index c97cb2eb1c5f..5c0bf61fae26 100644
> --- a/drivers/media/rc/mceusb.c
> +++ b/drivers/media/rc/mceusb.c
> @@ -1201,7 +1201,12 @@ static void mceusb_process_ir_data(struct mceusb_dev 
> *ir, int buf_len)
>   if (ir->rem) {
>   ir->parser_state = PARSE_IRDATA;
>   } else {
> - ir_raw_event_reset(ir->rc);
> + init_ir_raw_event();
> + rawir.timeout = 1;
> + rawir.duration 

Re: [PATCH v2 18/19] media: si470x: allow build both USB and I2C at the same time

[Daniel Scheller]

Am Fri, 6 Apr 2018 13:46:03 -0300
schrieb Mauro Carvalho Chehab :

> Em Sat, 7 Apr 2018 00:21:07 +0800
> kbuild test robot  escreveu:
> 
> > Hi Mauro,
> > 
> > I love your patch! Yet something to improve:
> > [...]
> 
> Fixed patch enclosed.
> 
> Thanks,
> Mauro
> 
> [PATCH] media: si470x: allow build both USB and I2C at the same time
> 
> Currently, either USB or I2C is built. Change it to allow
> having both enabled at the same time.
> 
> The main reason is that COMPILE_TEST all[yes/mod]builds will
> now contain all drivers under drivers/media.
> 
> Signed-off-by: Mauro Carvalho Chehab 

FWIW, this patch (which seemingly is commit
58757984ca3c73284a45dd53ac66f1414057cd09 in media_tree.git) seems to break 
media_build in a way that on my systems only 20 drivers and modules are built 
now, while it should be in the 650+ modules range. Hans' automated daily 
testbuilds suffer from the same issue, looking at todays daily build logs 
(Wednesday.tar.bz2). I personally build against Kernel 4.16.2 on Gentoo.

This specific commit/patch was found using

  # git bisect good v4.17-rc1
  # git bisect bad media_tree/master

And, "git revert 58767984..." makes all drivers being built again by
media_build.

Not sure if there's something other for which this patch acts as the
trigger of if this needs adaption in media_build, though I thought
reporting this doesn't hurt.

Best regards,
Daniel Scheller
-- 
https://github.com/herrnst

[PATCH 3/5] lgdt3306a v3.6 i2c mux backport

[Brad Love]

Another trivial patch

Signed-off-by: Brad Love 
---
 backports/v3.6_i2c_add_mux_adapter.patch | 12 
 1 file changed, 12 insertions(+)

diff --git a/backports/v3.6_i2c_add_mux_adapter.patch 
b/backports/v3.6_i2c_add_mux_adapter.patch
index 8172316..d26b82b 100644
--- a/backports/v3.6_i2c_add_mux_adapter.patch
+++ b/backports/v3.6_i2c_add_mux_adapter.patch
@@ -62,3 +62,15 @@ index a29c345..725c13a 100644
_i2c_mux_select,
NULL);
  
+diff --git a/drivers/media/dvb-frontends/lgdt3306a.c 
b/drivers/media/dvb-frontends/lgdt3306a.c
+--- a/drivers/media/dvb-frontends/lgdt3306a.c  2018-01-23 22:13:14.095893561 
+
 b/drivers/media/dvb-frontends/lgdt3306a.c  2018-01-23 22:13:52.796701755 
+
+@@ -2301,7 +2301,7 @@ static int lgdt3306a_probe(struct i2c_cl
+ 
+   /* create mux i2c adapter for tuner */
+   state->i2c_adap = i2c_add_mux_adapter(client->adapter, >dev,
+-  client, 0, 0, 0, lgdt3306a_select, lgdt3306a_deselect);
++  client, 0, 0, lgdt3306a_select, lgdt3306a_deselect);
+   if (state->i2c_adap == NULL) {
+   ret = -ENODEV;
+   goto err_kfree;
-- 
2.7.4

[PATCH 4/5] lgdt3306a v4.6 i2c mux backport

[Brad Love]

Based on other included backports.

Includes unlocked i2c gate control callbacks.

Signed-off-by: Brad Love 
---
 backports/v4.6_i2c_mux.patch | 213 +++
 1 file changed, 213 insertions(+)

diff --git a/backports/v4.6_i2c_mux.patch b/backports/v4.6_i2c_mux.patch
index 8f1ab88..ce89faa 100644
--- a/backports/v4.6_i2c_mux.patch
+++ b/backports/v4.6_i2c_mux.patch
@@ -1455,3 +1455,216 @@ index c76e78f..5c805f8 100644
pdata.dvb_frontend = adap->fe[0];
pdata.dvb_usb_device = d;
pdata.v4l2_subdev = subdev;
+diff --git a/drivers/media/dvb-frontends/lgdt3306a.c 
b/drivers/media/dvb-frontends/lgdt3306a.c
+index ab16d3a..2988262 100644
+--- a/drivers/media/dvb-frontends/lgdt3306a.c
 b/drivers/media/dvb-frontends/lgdt3306a.c
+@@ -79,8 +79,6 @@ struct lgdt3306a_state {
+   enum fe_modulation current_modulation;
+   u32 current_frequency;
+   u32 snr;
+-
+-  struct i2c_mux_core *muxc;
+ };
+ 
+ /*
+@@ -2182,20 +2180,142 @@ static const struct dvb_frontend_ops lgdt3306a_ops = {
+   .search   = lgdt3306a_search,
+ };
+ 
+-static int lgdt3306a_select(struct i2c_mux_core *muxc, u32 chan)
++/*
++ * I2C gate logic
++ * We must use unlocked I2C I/O because I2C adapter lock is already taken
++ * by the caller (usually tuner driver).
++ * select/unselect are unlocked versions of lgdt3306a_i2c_gate_ctrl
++ */
++static int lgdt3306a_select(struct i2c_adapter *adap, void *mux_priv, u32 
chan)
+ {
+-  struct i2c_client *client = i2c_mux_priv(muxc);
+-  struct lgdt3306a_state *state = i2c_get_clientdata(client);
++  struct i2c_client *client = mux_priv;
++  int ret;
++  u8 val;
++  u8 buf[3];
++
++  struct i2c_msg read_msg_1 = {
++  .addr = client->addr,
++  .flags = 0,
++  .buf = "\x00\x02",
++  .len = 2,
++  };
++  struct i2c_msg read_msg_2 = {
++  .addr = client->addr,
++  .flags = I2C_M_RD,
++  .buf = ,
++  .len = 1,
++  };
++
++  struct i2c_msg write_msg = {
++  .addr = client->addr,
++  .flags = 0,
++  .len = 3,
++  .buf = buf,
++  };
++
++  ret = __i2c_transfer(client->adapter, _msg_1, 1);
++  if (ret != 1)
++  {
++  pr_err("error (addr %02x reg 0x002 error (ret == %i)\n",
++ client->addr, ret);
++  if (ret < 0)
++  return ret;
++  else
++  return -EREMOTEIO;
++  }
+ 
+-  return lgdt3306a_i2c_gate_ctrl(>frontend, 1);
++  ret = __i2c_transfer(client->adapter, _msg_2, 1);
++  if (ret != 1)
++  {
++  pr_err("error (addr %02x reg 0x002 error (ret == %i)\n",
++ client->addr, ret);
++  if (ret < 0)
++  return ret;
++  else
++  return -EREMOTEIO;
++  }
++
++  buf[0] = 0x00;
++  buf[1] = 0x02;
++  val &= 0x7F;
++  val |= LG3306_TUNERI2C_ON;
++  buf[2] = val;
++  ret = __i2c_transfer(client->adapter, _msg, 1);
++  if (ret != 1) {
++  pr_err("error (addr %02x %02x <- %02x, err = %i)\n",
++ write_msg.buf[0], write_msg.buf[1], write_msg.buf[2], 
ret);
++  if (ret < 0)
++  return ret;
++  else
++  return -EREMOTEIO;
++  }
++  return 0;
+ }
+ 
+-static int lgdt3306a_deselect(struct i2c_mux_core *muxc, u32 chan)
++static int lgdt3306a_deselect(struct i2c_adapter *adap, void *mux_priv, u32 
chan)
+ {
+-  struct i2c_client *client = i2c_mux_priv(muxc);
+-  struct lgdt3306a_state *state = i2c_get_clientdata(client);
++  struct i2c_client *client = mux_priv;
++  int ret;
++  u8 val;
++  u8 buf[3];
++
++  struct i2c_msg read_msg_1 = {
++  .addr = client->addr,
++  .flags = 0,
++  .buf = "\x00\x02",
++  .len = 2,
++  };
++  struct i2c_msg read_msg_2 = {
++  .addr = client->addr,
++  .flags = I2C_M_RD,
++  .buf = ,
++  .len = 1,
++  };
+ 
+-  return lgdt3306a_i2c_gate_ctrl(>frontend, 0);
++  struct i2c_msg write_msg = {
++  .addr = client->addr,
++  .flags = 0,
++  .len = 3,
++  .buf = buf,
++  };
++
++  ret = __i2c_transfer(client->adapter, _msg_1, 1);
++  if (ret != 1)
++  {
++  pr_err("error (addr %02x reg 0x002 error (ret == %i)\n",
++ client->addr, ret);
++  if (ret < 0)
++  return ret;
++  else
++  return -EREMOTEIO;
++  }
++
++  ret = __i2c_transfer(client->adapter, _msg_2, 1);
++  if (ret != 1)
++  {
++  pr_err("error (addr %02x 

[PATCH 2/5] lgdt3306a v3.4 i2c mux backport

[Brad Love]

Trivial patch

Signed-off-by: Brad Love 
---
 backports/v3.4_i2c_add_mux_adapter.patch | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/backports/v3.4_i2c_add_mux_adapter.patch 
b/backports/v3.4_i2c_add_mux_adapter.patch
index 5f93275..fc1b697 100644
--- a/backports/v3.4_i2c_add_mux_adapter.patch
+++ b/backports/v3.4_i2c_add_mux_adapter.patch
@@ -66,3 +66,17 @@ index 725c13a..35e3ac1 100644
dev /* mux_priv */,
0,
mux_no /* chan_id */,
+diff --git a/drivers/media/dvb-frontends/lgdt3306a.c 
b/drivers/media/dvb-frontends/lgdt3306a.c
+--- a/drivers/media/dvb-frontends/lgdt3306a.c  2018-01-23 22:15:12.146359404 
+
 b/drivers/media/dvb-frontends/lgdt3306a.c  2018-01-23 22:16:01.055381481 
+
+@@ -2300,8 +2300,8 @@ static int lgdt3306a_probe(struct i2c_cl
+   state = fe->demodulator_priv;
+ 
+   /* create mux i2c adapter for tuner */
+-  state->i2c_adap = i2c_add_mux_adapter(client->adapter, >dev,
+-  client, 0, 0, lgdt3306a_select, lgdt3306a_deselect);
++  state->i2c_adap = i2c_add_mux_adapter(client->adapter, client,
++  0, 0, lgdt3306a_select, lgdt3306a_deselect);
+   if (state->i2c_adap == NULL) {
+   ret = -ENODEV;
+   goto err_kfree;
-- 
2.7.4

[PATCH 5/5] Remove lgdt3306a v4.7 limitation

[Brad Love]

Backports are now included for lgdt3306a back
to v3.4 and the driver has been well tested
back to kernel v3.2

Signed-off-by: Brad Love 
---
 v4l/versions.txt | 1 -
 1 file changed, 1 deletion(-)

diff --git a/v4l/versions.txt b/v4l/versions.txt
index 6149f7a..6220485 100644
--- a/v4l/versions.txt
+++ b/v4l/versions.txt
@@ -15,7 +15,6 @@ VIDEO_OV5670
 
 [4.7.0]
 # needs i2c_mux_alloc
-DVB_LGDT3306A
 DVB_RTL2830
 DVB_RTL2832
 DVB_M88DS3103
-- 
2.7.4

[PATCH 0/5] media_build: Backport fixes and patches

[Brad Love]

This patch set provides lgdt3306a backports for
v3.4, v3.6, and v4.6. The v4l/versions.txt file
is also fixed up for a couple drivers with
included backports.


Brad Love (5):
  Enable two drivers with backports
  lgdt3306a v3.4 i2c mux backport
  lgdt3306a v3.6 i2c mux backport
  lgdt3306a v4.6 i2c mux backport
  Remove lgdt3306a v4.7 limitation

 backports/v3.4_i2c_add_mux_adapter.patch |  14 ++
 backports/v3.6_i2c_add_mux_adapter.patch |  12 ++
 backports/v4.6_i2c_mux.patch | 213 +++
 v4l/versions.txt |   3 -
 4 files changed, 239 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH 1/5] Enable two drivers with backports

[Brad Love]

si2168 has i2c mux related backports to v3.4
cx231xx has i2c mux related backports to v3.4

Both drivers are tested working in kernel v3.2

Signed-off-by: Brad Love 
---
 v4l/versions.txt | 2 --
 1 file changed, 2 deletions(-)

diff --git a/v4l/versions.txt b/v4l/versions.txt
index 4912fc2..6149f7a 100644
--- a/v4l/versions.txt
+++ b/v4l/versions.txt
@@ -15,9 +15,7 @@ VIDEO_OV5670
 
 [4.7.0]
 # needs i2c_mux_alloc
-VIDEO_CX231XX
 DVB_LGDT3306A
-DVB_SI2168
 DVB_RTL2830
 DVB_RTL2832
 DVB_M88DS3103
-- 
2.7.4

Re: [PATCH] [media] include/media: fix missing | operator when setting cfg

[Sylwester Nawrocki]

On 04/18/2018 05:24 PM, Colin Ian King wrote:
> Oops, shall I re-send?

There is no need to, thanks.

[PATCH] media: cec: set ev rather than v with CEC_PIN_EVENT_FL_DROPPED bit

[Colin King]

From: Colin Ian King 

Setting v with the CEC_PIN_EVENT_FL_DROPPED is incorrect, instead
ev should be set with this bit. Fix this.

Detected by CoverityScan, CID#1467974 ("Extra high-order bits")

Fixes: 6ec1cbf6b125 ("media: cec: improve CEC pin event handling")
Signed-off-by: Colin Ian King 
---
 drivers/media/cec/cec-pin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/cec/cec-pin.c b/drivers/media/cec/cec-pin.c
index 2a5df99735fa..6e311424f0dc 100644
--- a/drivers/media/cec/cec-pin.c
+++ b/drivers/media/cec/cec-pin.c
@@ -119,7 +119,7 @@ static void cec_pin_update(struct cec_pin *pin, bool v, 
bool force)
 
if (pin->work_pin_events_dropped) {
pin->work_pin_events_dropped = false;
-   v |= CEC_PIN_EVENT_FL_DROPPED;
+   ev |= CEC_PIN_EVENT_FL_DROPPED;
}
pin->work_pin_events[pin->work_pin_events_wr] = ev;
pin->work_pin_ts[pin->work_pin_events_wr] = ktime_get();
-- 
2.17.0

Re: [PATCH] [media] include/media: fix missing | operator when setting cfg

[Colin Ian King]

On 18/04/18 16:23, Sylwester Nawrocki wrote:
> On 04/18/2018 05:20 PM, Sylwester Nawrocki wrote:
>> On 04/18/2018 05:06 PM, Colin King wrote:
>>> From: Colin Ian King 
>>>
>>> The value from a readl is being masked with ITE_REG_CIOCAN_MASK however
>>> this is not being used and cfg is being re-assigned.  I believe the
>>> assignment operator should actually be instead the |= operator.
>>>
>>> Detected by CoverityScan, CID#1467987 ("Unused value")
>>>
>>> Signed-off-by: Colin Ian King 
>> Thanks for the patch.
>>
>> Acked-by: Sylwester Nawrocki 
> 
> I forgot to mention that the subject should rather looks something
> like:
> 
> "exynos4-is: fimc-lite: : fix missing | operator when setting cfg"
> 
Oops, shall I re-send?

Re: [PATCH] [media] include/media: fix missing | operator when setting cfg

[Sylwester Nawrocki]

On 04/18/2018 05:20 PM, Sylwester Nawrocki wrote:
> On 04/18/2018 05:06 PM, Colin King wrote:
>> From: Colin Ian King 
>>
>> The value from a readl is being masked with ITE_REG_CIOCAN_MASK however
>> this is not being used and cfg is being re-assigned.  I believe the
>> assignment operator should actually be instead the |= operator.
>>
>> Detected by CoverityScan, CID#1467987 ("Unused value")
>>
>> Signed-off-by: Colin Ian King 
> Thanks for the patch.
> 
> Acked-by: Sylwester Nawrocki 

I forgot to mention that the subject should rather looks something
like:

"exynos4-is: fimc-lite: : fix missing | operator when setting cfg"

-- 
Regards,
Sylwester

Re: [PATCH] [media] include/media: fix missing | operator when setting cfg

[Sylwester Nawrocki]

On 04/18/2018 05:06 PM, Colin King wrote:
> From: Colin Ian King 
> 
> The value from a readl is being masked with ITE_REG_CIOCAN_MASK however
> this is not being used and cfg is being re-assigned.  I believe the
> assignment operator should actually be instead the |= operator.
> 
> Detected by CoverityScan, CID#1467987 ("Unused value")
> 
> Signed-off-by: Colin Ian King 

Thanks for the patch.

Acked-by: Sylwester Nawrocki 

[PATCH] [media] include/media: fix missing | operator when setting cfg

[Colin King]

From: Colin Ian King 

The value from a readl is being masked with ITE_REG_CIOCAN_MASK however
this is not being used and cfg is being re-assigned.  I believe the
assignment operator should actually be instead the |= operator.

Detected by CoverityScan, CID#1467987 ("Unused value")

Signed-off-by: Colin Ian King 
---
 drivers/media/platform/exynos4-is/fimc-lite-reg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-lite-reg.c 
b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
index f0acc550d065..16565a0b4bf1 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite-reg.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
@@ -254,7 +254,7 @@ void flite_hw_set_dma_window(struct fimc_lite *dev, struct 
flite_frame *f)
/* Maximum output pixel size */
cfg = readl(dev->regs + FLITE_REG_CIOCAN);
cfg &= ~FLITE_REG_CIOCAN_MASK;
-   cfg = (f->f_height << 16) | f->f_width;
+   cfg |= (f->f_height << 16) | f->f_width;
writel(cfg, dev->regs + FLITE_REG_CIOCAN);
 
/* DMA offsets */
-- 
2.17.0

Re: [PATCH v2 10/10] media: ov772x: avoid accessing registers under power saving mode

[jacopo mondi]

Hi Sakari,

On Wed, Apr 18, 2018 at 04:17:02PM +0300, Sakari Ailus wrote:
> On Wed, Apr 18, 2018 at 02:55:36PM +0200, jacopo mondi wrote:
> > Hi Akinobu,
> >
> > On Mon, Apr 16, 2018 at 11:51:51AM +0900, Akinobu Mita wrote:
> > > The set_fmt() in subdev pad ops, the s_ctrl() for subdev control handler,
> > > and the s_frame_interval() in subdev video ops could be called when the
> > > device is under power saving mode.  These callbacks for ov772x driver
> > > cause updating H/W registers that will fail under power saving mode.
> > >
> >
> > I might be wrong, but if the sensor is powered off, you should not
> > receive any subdev_pad_ops function call if sensor is powered off.
>
> This happens (now that the driver supports sub-device uAPI) if the user
> opens a sub-device node but the main driver has not powered the sensor on.

Indeed. Sorry for the noise. Could you please check my reply to [08/10] as well?

Thanks
   j

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


signature.asc
Description: PGP signature

Re: [PATCH v2 10/10] media: ov772x: avoid accessing registers under power saving mode

[Sakari Ailus]

On Wed, Apr 18, 2018 at 02:55:36PM +0200, jacopo mondi wrote:
> Hi Akinobu,
> 
> On Mon, Apr 16, 2018 at 11:51:51AM +0900, Akinobu Mita wrote:
> > The set_fmt() in subdev pad ops, the s_ctrl() for subdev control handler,
> > and the s_frame_interval() in subdev video ops could be called when the
> > device is under power saving mode.  These callbacks for ov772x driver
> > cause updating H/W registers that will fail under power saving mode.
> >
> 
> I might be wrong, but if the sensor is powered off, you should not
> receive any subdev_pad_ops function call if sensor is powered off.

This happens (now that the driver supports sub-device uAPI) if the user
opens a sub-device node but the main driver has not powered the sensor on.

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

[PATCH] staging: media: davinci_vpfe: fix spin_lock/unlock imbalance

[Gustavo A. R. Silva]

It seems that this is a copy-paste error and that the proper
variable to use in this particular case is video_out2 instead
of video_out.

Addresses-Coverity-ID: 1467961 ("Copy-paste error")
Fixes: 45e46b3bbe18 ("[media] davinci: vpfe: dm365: resizer driver based on 
media framework")
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/staging/media/davinci_vpfe/dm365_resizer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_resizer.c 
b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
index df6d55e..2b79747 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_resizer.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
@@ -1060,7 +1060,7 @@ static void resizer_ss_isr(struct vpfe_resizer_device 
*resizer)
/* If resizer B is enabled */
if (pipe->output_num > 1 && resizer->resizer_b.output ==
RESIZER_OUTPUT_MEMORY) {
-   spin_lock(_out->dma_queue_lock);
+   spin_lock(_out2->dma_queue_lock);
vpfe_video_process_buffer_complete(video_out2);
video_out2->state = VPFE_VIDEO_BUFFER_NOT_QUEUED;
vpfe_video_schedule_next_buffer(video_out2);
-- 
2.7.4

Re: [PATCH v2 04/10] media: ov772x: add media controller support

[Sakari Ailus]

On Wed, Apr 18, 2018 at 02:13:17PM +0200, jacopo mondi wrote:
> Hi Sakari,
> 
> On Wed, Apr 18, 2018 at 02:58:16PM +0300, Sakari Ailus wrote:
> > On Wed, Apr 18, 2018 at 01:28:14PM +0200, jacopo mondi wrote:
> > > Hi Akinobu,
> > >
> > > On Mon, Apr 16, 2018 at 11:51:45AM +0900, Akinobu Mita wrote:
> > > > Create a source pad and set the media controller type to the sensor.
> > > >
> > > > Cc: Jacopo Mondi 
> > > > Cc: Laurent Pinchart 
> > > > Cc: Hans Verkuil 
> > > > Cc: Sakari Ailus 
> > > > Cc: Mauro Carvalho Chehab 
> > > > Signed-off-by: Akinobu Mita 
> > >
> > > Reviewed-by: Jacopo Mondi 
> > >
> > > Not strictly on this patch, but I'm a bit confused on the difference
> > > between CONFIG_MEDIA_CONTROLLER and CONFIG_VIDEO_V4L2_SUBDEV_API...
> > > Doesn't media controller support mandate implementing subdev APIs as
> > > well?
> >
> > The subdev uAPI depends on MC.
> 
> Again, sorry for not being clear. Can an mc-compliant device not
> implement sudev uAPIs ?

In principle, yes. Still, for a sensor driver supporting MC it only makes
sense if it also supports sub-device uAPI.

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

Re: [PATCH v2 10/10] media: ov772x: avoid accessing registers under power saving mode

[jacopo mondi]

Hi Akinobu,

On Mon, Apr 16, 2018 at 11:51:51AM +0900, Akinobu Mita wrote:
> The set_fmt() in subdev pad ops, the s_ctrl() for subdev control handler,
> and the s_frame_interval() in subdev video ops could be called when the
> device is under power saving mode.  These callbacks for ov772x driver
> cause updating H/W registers that will fail under power saving mode.
>

I might be wrong, but if the sensor is powered off, you should not
receive any subdev_pad_ops function call if sensor is powered off.

For this driver that's up to the platform driver to handle this
correctly, have you found any case where it is necessary to handle
this in the sensor driver? Have I mis-interpreted the use case of this
patch?


> This avoids it by not apply any changes to H/W if the device is not powered
> up.  Instead the changes will be restored right after power-up.
>
> Cc: Jacopo Mondi 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: Akinobu Mita 
> ---
> * v2
> - New patch
>
>  drivers/media/i2c/ov772x.c | 77 
> +-
>  1 file changed, 62 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 1297a21..c44728f 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -741,19 +741,29 @@ static int ov772x_s_frame_interval(struct v4l2_subdev 
> *sd,
>   struct ov772x_priv *priv = to_ov772x(sd);
>   struct v4l2_fract *tpf = >interval;
>   unsigned int fps;
> - int ret;
> + int ret = 0;
>
>   fps = ov772x_select_fps(priv, tpf);
>
> - ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win);
> - if (ret)
> - return ret;
> + mutex_lock(>power_lock);
> + /*
> +  * If the device is not powered up by the host driver do
> +  * not apply any changes to H/W at this time. Instead
> +  * the frame rate will be restored right after power-up.
> +  */
> + if (priv->power_count > 0) {
> + ret = ov772x_set_frame_rate(priv, fps, priv->cfmt, priv->win);
> + if (ret)
> + goto error;
> + }
>
>   tpf->numerator = 1;
>   tpf->denominator = fps;
>   priv->fps = fps;
> +error:
> + mutex_unlock(>power_lock);
>
> - return 0;
> + return ret;
>  }
>
>  static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
> @@ -765,6 +775,16 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
>   int ret = 0;
>   u8 val;
>
> + /* v4l2_ctrl_lock() locks our own mutex */
> +
> + /*
> +  * If the device is not powered up by the host driver do
> +  * not apply any controls to H/W at this time. Instead
> +  * the controls will be restored right after power-up.
> +  */
> + if (priv->power_count == 0)
> + return 0;
> +
>   switch (ctrl->id) {
>   case V4L2_CID_VFLIP:
>   val = ctrl->val ? VFLIP_IMG : 0x00;
> @@ -888,6 +908,10 @@ static int ov772x_power_off(struct ov772x_priv *priv)
>   return 0;
>  }
>
> +static int ov772x_set_params(struct ov772x_priv *priv,
> +  const struct ov772x_color_format *cfmt,
> +  const struct ov772x_win_size *win);
> +
>  static int ov772x_s_power(struct v4l2_subdev *sd, int on)
>  {
>   struct ov772x_priv *priv = to_ov772x(sd);
> @@ -898,8 +922,20 @@ static int ov772x_s_power(struct v4l2_subdev *sd, int on)
>   /* If the power count is modified from 0 to != 0 or from != 0 to 0,
>* update the power state.
>*/
> - if (priv->power_count == !on)
> - ret = on ? ov772x_power_on(priv) : ov772x_power_off(priv);
> + if (priv->power_count == !on) {
> + if (on) {
> + ret = ov772x_power_on(priv);
> + /* Restore the controls */
> + if (!ret)
> + ret = ov772x_set_params(priv, priv->cfmt,
> + priv->win);
> + /* Restore the format and the frame rate */
> + if (!ret)
> + ret = __v4l2_ctrl_handler_setup(>hdl);

frame interval is not listed in the sensor control list, it won't be
restored if I'm not wrong...

Thanks
   j

> + } else {
> + ret = ov772x_power_off(priv);
> + }
> + }
>
>   if (!ret) {
>   /* Update the power count. */
> @@ -1163,7 +1199,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
>   struct v4l2_mbus_framefmt *mf = >format;
>   const struct ov772x_color_format *cfmt;
>   const struct ov772x_win_size *win;
> - int ret;
> + int ret = 0;
>
>   if (format->pad)
>   return -EINVAL;
> @@ 

Re: [PATCH v2 08/10] media: ov772x: handle nested s_power() calls

[jacopo mondi]

Hi Akinobu,

On Mon, Apr 16, 2018 at 11:51:49AM +0900, Akinobu Mita wrote:
> Depending on the v4l2 driver, calling s_power() could be nested.  So the
> actual transitions between power saving mode and normal operation mode
> should only happen at the first power on and the last power off.

What do you mean with 'nested' ?

My understanding is that:
- if a sensor driver is mc compliant, it's s_power is called from
  v4l2_mc.c pipeline_pm_power_one() only when the power state changes
- if a sensor driver is not mc compliant, the s_power routine is
  called from the platform driver, and it should not happen that it is
  called twice with the same power state
- if a sensor implements subdev's internal operations open and close
  it may call it's own s_power routines from there, and it can be
  opened more that once.

My understanding is that this driver s_power routines are only called
from platform drivers, and they -should- be safe.

Although, I'm not against this protection completely. Others might be,
though.

>
> This adds an s_power() nesting counter and updates the power state if the
> counter is modified from 0 to != 0 or from != 0 to 0.
>
> Cc: Jacopo Mondi 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: Akinobu Mita 
> ---
> * v2
> - New patch
>
>  drivers/media/i2c/ov772x.c | 33 +
>  1 file changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 4245a46..2cd6e85 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -424,6 +424,9 @@ struct ov772x_priv {
>   /* band_filter = COM8[5] ? 256 - BDBASE : 0 */
>   unsigned shortband_filter;
>   unsigned int  fps;
> + /* lock to protect power_count */
> + struct mutex  power_lock;
> + int   power_count;
>  #ifdef CONFIG_MEDIA_CONTROLLER
>   struct media_pad pad;
>  #endif
> @@ -871,9 +874,25 @@ static int ov772x_power_off(struct ov772x_priv *priv)
>  static int ov772x_s_power(struct v4l2_subdev *sd, int on)
>  {
>   struct ov772x_priv *priv = to_ov772x(sd);
> + int ret = 0;
> +
> + mutex_lock(>power_lock);
>
> - return on ? ov772x_power_on(priv) :
> - ov772x_power_off(priv);
> + /* If the power count is modified from 0 to != 0 or from != 0 to 0,
> +  * update the power state.
> +  */
> + if (priv->power_count == !on)
> + ret = on ? ov772x_power_on(priv) : ov772x_power_off(priv);

Just release the mutex and return 0 if (power_count == on)
The code will be more readable imho.

> +
> + if (!ret) {
> + /* Update the power count. */
> + priv->power_count += on ? 1 : -1;
> + WARN_ON(priv->power_count < 0);
> + }
> +
> + mutex_unlock(>power_lock);
> +
> + return ret;
>  }
>
>  static const struct ov772x_win_size *ov772x_select_win(u32 width, u32 height)
> @@ -1303,6 +1322,7 @@ static int ov772x_probe(struct i2c_client *client,
>   return -ENOMEM;
>
>   priv->info = client->dev.platform_data;
> + mutex_init(>power_lock);
>
>   v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
>   priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> @@ -1314,8 +1334,10 @@ static int ov772x_probe(struct i2c_client *client,
>   v4l2_ctrl_new_std(>hdl, _ctrl_ops,
> V4L2_CID_BAND_STOP_FILTER, 0, 256, 1, 0);
>   priv->subdev.ctrl_handler = >hdl;
> - if (priv->hdl.error)
> - return priv->hdl.error;
> + if (priv->hdl.error) {
> + ret = priv->hdl.error;
> + goto error_mutex_destroy;
> + }
>
>   priv->clk = clk_get(>dev, "xclk");
>   if (IS_ERR(priv->clk)) {
> @@ -1363,6 +1385,8 @@ static int ov772x_probe(struct i2c_client *client,
>   clk_put(priv->clk);
>  error_ctrl_free:
>   v4l2_ctrl_handler_free(>hdl);
> +error_mutex_destroy:
> + mutex_destroy(>power_lock);

mutex_destroy() does nothing most of the time. It won't hurt though.

Thanks
   j

>
>   return ret;
>  }
> @@ -1377,6 +1401,7 @@ static int ov772x_remove(struct i2c_client *client)
>   gpiod_put(priv->pwdn_gpio);
>   v4l2_async_unregister_subdev(>subdev);
>   v4l2_ctrl_handler_free(>hdl);
> + mutex_destroy(>power_lock);
>
>   return 0;
>  }
> --
> 2.7.4
>


signature.asc
Description: PGP signature

Re: [PATCH v2 04/10] media: ov772x: add media controller support

[jacopo mondi]

Hi Sakari,

On Wed, Apr 18, 2018 at 02:58:16PM +0300, Sakari Ailus wrote:
> On Wed, Apr 18, 2018 at 01:28:14PM +0200, jacopo mondi wrote:
> > Hi Akinobu,
> >
> > On Mon, Apr 16, 2018 at 11:51:45AM +0900, Akinobu Mita wrote:
> > > Create a source pad and set the media controller type to the sensor.
> > >
> > > Cc: Jacopo Mondi 
> > > Cc: Laurent Pinchart 
> > > Cc: Hans Verkuil 
> > > Cc: Sakari Ailus 
> > > Cc: Mauro Carvalho Chehab 
> > > Signed-off-by: Akinobu Mita 
> >
> > Reviewed-by: Jacopo Mondi 
> >
> > Not strictly on this patch, but I'm a bit confused on the difference
> > between CONFIG_MEDIA_CONTROLLER and CONFIG_VIDEO_V4L2_SUBDEV_API...
> > Doesn't media controller support mandate implementing subdev APIs as
> > well?
>
> The subdev uAPI depends on MC.

Again, sorry for not being clear. Can an mc-compliant device not
implement sudev uAPIs ?

Thanks
  j

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


signature.asc
Description: PGP signature

Re: [PATCH v2 01/10] media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING

[jacopo mondi]

Hi Sakari,

On Wed, Apr 18, 2018 at 01:41:54PM +0300, Sakari Ailus wrote:
> On Wed, Apr 18, 2018 at 12:05:49PM +0200, jacopo mondi wrote:
> > Hi Akinobu,
> >
> > On Mon, Apr 16, 2018 at 11:51:42AM +0900, Akinobu Mita wrote:
> > > The ov772x driver only works when the i2c controller have
> > > I2C_FUNC_PROTOCOL_MANGLING.  However, many i2c controller drivers don't
> > > support it.
> > >
> > > The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that
> > > it doesn't support repeated starts.
> > >
> > > This changes the reading ov772x register method so that it doesn't
> > > require I2C_FUNC_PROTOCOL_MANGLING by calling two separated i2c messages.
> > >
> > > Cc: Jacopo Mondi 
> > > Cc: Laurent Pinchart 
> > > Cc: Hans Verkuil 
> > > Cc: Sakari Ailus 
> > > Cc: Mauro Carvalho Chehab 
> > > Signed-off-by: Akinobu Mita 
> > > ---
> > > * v2
> > > - Replace the implementation of ov772x_read() instead of adding an
> > >   alternative method
> >
> > I now wonder if my initial reply to this patch was wrong, and where
> > possible we should try to use smbus operations...
> >
> > From Documentation/i2c/smbus-protocol
> > "If you write a driver for some I2C device, please try to use the SMBus
> > commands if at all possible... " and that's because, according to
> > documentation, most I2c adapters support smbus protocol but may not
> > support the full i2c command set.
> >
> > The fact this driver then restricts the supported adapters to the ones
> > that support protocol mangling makes me think your change is fine,
> > but as it often happens, I would scale this to more knowledgable
> > people...
>
> Do you actually need to use this on SMBus adapters? A lot of sensor drivers
> just use I²C; if SMBus support is really needed it can be always added back
> later on...

That was actually my question, sorry for not being clear.

As the documentation says that SMBus has to be preferred when
possible, I was wondering if ditching it completely in favour of plain
I2c was wrong or not... I assume from your answer it is fine.

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


signature.asc
Description: PGP signature

Re: [PATCH v2 04/10] media: ov772x: add media controller support

[Sakari Ailus]

On Wed, Apr 18, 2018 at 01:28:14PM +0200, jacopo mondi wrote:
> Hi Akinobu,
> 
> On Mon, Apr 16, 2018 at 11:51:45AM +0900, Akinobu Mita wrote:
> > Create a source pad and set the media controller type to the sensor.
> >
> > Cc: Jacopo Mondi 
> > Cc: Laurent Pinchart 
> > Cc: Hans Verkuil 
> > Cc: Sakari Ailus 
> > Cc: Mauro Carvalho Chehab 
> > Signed-off-by: Akinobu Mita 
> 
> Reviewed-by: Jacopo Mondi 
> 
> Not strictly on this patch, but I'm a bit confused on the difference
> between CONFIG_MEDIA_CONTROLLER and CONFIG_VIDEO_V4L2_SUBDEV_API...
> Doesn't media controller support mandate implementing subdev APIs as
> well?

The subdev uAPI depends on MC.

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

Re: [PATCH v2 07/10] media: ov772x: support device tree probing

[jacopo mondi]

Hi Akinobu,

On Mon, Apr 16, 2018 at 11:51:48AM +0900, Akinobu Mita wrote:
> The ov772x driver currently only supports legacy platform data probe.
> This change enables device tree probing.
>
> Note that the platform data probe can select auto or manual edge control
> mode, but the device tree probling can only select auto edge control mode
> for now.
>
> Cc: Jacopo Mondi 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: Akinobu Mita 
> ---
> * v2
> - Add missing NULL checks for priv->info
> - Leave the check for the missing platform data if legacy platform data
>   probe is used.
>
>  drivers/media/i2c/ov772x.c | 61 
> --
>  1 file changed, 43 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 88d1418a..4245a46 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -749,13 +749,13 @@ static int ov772x_s_ctrl(struct v4l2_ctrl *ctrl)
>   case V4L2_CID_VFLIP:
>   val = ctrl->val ? VFLIP_IMG : 0x00;
>   priv->flag_vflip = ctrl->val;
> - if (priv->info->flags & OV772X_FLAG_VFLIP)
> + if (priv->info && (priv->info->flags & OV772X_FLAG_VFLIP))
>   val ^= VFLIP_IMG;
>   return ov772x_mask_set(client, COM3, VFLIP_IMG, val);
>   case V4L2_CID_HFLIP:
>   val = ctrl->val ? HFLIP_IMG : 0x00;
>   priv->flag_hflip = ctrl->val;
> - if (priv->info->flags & OV772X_FLAG_HFLIP)
> + if (priv->info && (priv->info->flags & OV772X_FLAG_HFLIP))
>   val ^= HFLIP_IMG;
>   return ov772x_mask_set(client, COM3, HFLIP_IMG, val);
>   case V4L2_CID_BAND_STOP_FILTER:
> @@ -914,19 +914,14 @@ static void ov772x_select_params(const struct 
> v4l2_mbus_framefmt *mf,
>   *win = ov772x_select_win(mf->width, mf->height);
>  }
>
> -static int ov772x_set_params(struct ov772x_priv *priv,
> -  const struct ov772x_color_format *cfmt,
> -  const struct ov772x_win_size *win)
> +static int ov772x_edgectrl(struct ov772x_priv *priv)
>  {
>   struct i2c_client *client = v4l2_get_subdevdata(>subdev);
> - struct v4l2_fract tpf;
>   int ret;
> - u8  val;
>
> - /* Reset hardware. */
> - ov772x_reset(client);
> + if (!priv->info)
> + return 0;
>
> - /* Edge Ctrl. */
>   if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) {
>   /*
>* Manual Edge Control Mode.
> @@ -937,19 +932,19 @@ static int ov772x_set_params(struct ov772x_priv *priv,
>
>   ret = ov772x_mask_set(client, DSPAUTO, EDGE_ACTRL, 0x00);
>   if (ret < 0)
> - goto ov772x_set_fmt_error;
> + return ret;
>
>   ret = ov772x_mask_set(client,
> EDGE_TRSHLD, OV772X_EDGE_THRESHOLD_MASK,
> priv->info->edgectrl.threshold);
>   if (ret < 0)
> - goto ov772x_set_fmt_error;
> + return ret;
>
>   ret = ov772x_mask_set(client,
> EDGE_STRNGT, OV772X_EDGE_STRENGTH_MASK,
> priv->info->edgectrl.strength);
>   if (ret < 0)
> - goto ov772x_set_fmt_error;
> + return ret;
>
>   } else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) {
>   /*
> @@ -961,15 +956,35 @@ static int ov772x_set_params(struct ov772x_priv *priv,
> EDGE_UPPER, OV772X_EDGE_UPPER_MASK,
> priv->info->edgectrl.upper);
>   if (ret < 0)
> - goto ov772x_set_fmt_error;
> + return ret;
>
>   ret = ov772x_mask_set(client,
> EDGE_LOWER, OV772X_EDGE_LOWER_MASK,
> priv->info->edgectrl.lower);
>   if (ret < 0)
> - goto ov772x_set_fmt_error;
> + return ret;
>   }
>
> + return 0;
> +}
> +
> +static int ov772x_set_params(struct ov772x_priv *priv,
> +  const struct ov772x_color_format *cfmt,
> +  const struct ov772x_win_size *win)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(>subdev);
> + struct v4l2_fract tpf;
> + int ret;
> + u8  val;
> +
> + /* Reset hardware. */
> + ov772x_reset(client);
> +
> + /* Edge Ctrl. */
> + ret =  ov772x_edgectrl(priv);
> + if (ret < 0)
> + goto 

Re: [PATCH v2 05/10] media: ov772x: use generic names for reset and powerdown gpios

[jacopo mondi]

Hi Akinobu,

On Mon, Apr 16, 2018 at 11:51:46AM +0900, Akinobu Mita wrote:
> The ov772x driver uses "rstb-gpios" and "pwdn-gpios" for reset and
> powerdown pins.  However, using generic names for thse gpios is preferred.

nit: 'these gpios'

> ("reset-gpios" and "powerdown-gpios" respectively)
>
> There is only one mainline user for these gpios, so rename to generic
> names.
>
> Cc: Jacopo Mondi 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: Akinobu Mita 

Bindings update should come first.
Not a big deal.

Reviewed-by: Jacopo Mondi 

> ---
> * v2
> - New patch
>
>  arch/sh/boards/mach-migor/setup.c | 5 +++--
>  drivers/media/i2c/ov772x.c| 8 
>  2 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/arch/sh/boards/mach-migor/setup.c 
> b/arch/sh/boards/mach-migor/setup.c
> index 271dfc2..73b9ee4 100644
> --- a/arch/sh/boards/mach-migor/setup.c
> +++ b/arch/sh/boards/mach-migor/setup.c
> @@ -351,8 +351,9 @@ static struct platform_device migor_ceu_device = {
>  static struct gpiod_lookup_table ov7725_gpios = {
>   .dev_id = "0-0021",
>   .table  = {
> - GPIO_LOOKUP("sh7722_pfc", GPIO_PTT0, "pwdn", GPIO_ACTIVE_HIGH),
> - GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "rstb", GPIO_ACTIVE_LOW),
> + GPIO_LOOKUP("sh7722_pfc", GPIO_PTT0, "powerdown",
> + GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP("sh7722_pfc", GPIO_PTT3, "reset", GPIO_ACTIVE_LOW),
>   },
>  };
>
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 0ae2a4f..88d1418a 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -837,10 +837,10 @@ static int ov772x_power_on(struct ov772x_priv *priv)
>* available to handle this cleanly, request the GPIO temporarily
>* to avoid conflicts.
>*/
> - priv->rstb_gpio = gpiod_get_optional(>dev, "rstb",
> + priv->rstb_gpio = gpiod_get_optional(>dev, "reset",
>GPIOD_OUT_LOW);
>   if (IS_ERR(priv->rstb_gpio)) {
> - dev_info(>dev, "Unable to get GPIO \"rstb\"");
> + dev_info(>dev, "Unable to get GPIO \"reset\"");
>   return PTR_ERR(priv->rstb_gpio);
>   }
>
> @@ -1309,10 +1309,10 @@ static int ov772x_probe(struct i2c_client *client,
>   goto error_ctrl_free;
>   }
>
> - priv->pwdn_gpio = gpiod_get_optional(>dev, "pwdn",
> + priv->pwdn_gpio = gpiod_get_optional(>dev, "powerdown",
>GPIOD_OUT_LOW);
>   if (IS_ERR(priv->pwdn_gpio)) {
> - dev_info(>dev, "Unable to get GPIO \"pwdn\"");
> + dev_info(>dev, "Unable to get GPIO \"powerdown\"");
>   ret = PTR_ERR(priv->pwdn_gpio);
>   goto error_clk_put;
>   }
> --
> 2.7.4
>


signature.asc
Description: PGP signature

Re: [PATCH v2 06/10] media: dt-bindings: ov772x: add device tree binding

[jacopo mondi]

On Mon, Apr 16, 2018 at 11:51:47AM +0900, Akinobu Mita wrote:
> This adds a device tree binding documentation for OV7720/OV7725 sensor.
>
> Cc: Jacopo Mondi 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Cc: Rob Herring 
> Signed-off-by: Akinobu Mita 

Reviewed-by: Jacopo Mondi 

> ---
> * v2
> - Add "dt-bindings:" in the subject
> - Add a brief description of the sensor
> - Update the GPIO names
> - Indicate the GPIO active level
>
>  .../devicetree/bindings/media/i2c/ov772x.txt   | 42 
> ++
>  MAINTAINERS|  1 +
>  2 files changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov772x.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov772x.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> new file mode 100644
> index 000..b045503
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov772x.txt
> @@ -0,0 +1,42 @@
> +* Omnivision OV7720/OV7725 CMOS sensor
> +
> +The Omnivision OV7720/OV7725 sensor supports multiple resolutions output,
> +such as VGA, QVGA, and any size scaling down from CIF to 40x30. It also can
> +support the YUV422, RGB565/555/444, GRB422 or raw RGB output formats.
> +
> +Required Properties:
> +- compatible: shall be one of
> + "ovti,ov7720"
> + "ovti,ov7725"
> +- clocks: reference to the xclk input clock.
> +- clock-names: shall be "xclk".
> +
> +Optional Properties:
> +- reset-gpios: reference to the GPIO connected to the RSTB pin which is
> +  active low, if any.
> +- powerdown-gpios: reference to the GPIO connected to the PWDN pin which is
> +  active high, if any.
> +
> +The device node shall contain one 'port' child node with one child 'endpoint'
> +subnode for its digital output video port, in accordance with the video
> +interface bindings defined in Documentation/devicetree/bindings/media/
> +video-interfaces.txt.
> +
> +Example:
> +
> + {
> + ov772x: camera@21 {
> + compatible = "ovti,ov7725";
> + reg = <0x21>;
> + reset-gpios = <_gpio_0 0 GPIO_ACTIVE_LOW>;
> + powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_LOW>;
> + clocks = <>;
> + clock-names = "xclk";
> +
> + port {
> + ov772x_0: endpoint {
> + remote-endpoint = <_in0>;
> + };
> + };
> + };
> +};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0a1410d..f500953 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10344,6 +10344,7 @@ T:git git://linuxtv.org/media_tree.git
>  S:   Odd fixes
>  F:   drivers/media/i2c/ov772x.c
>  F:   include/media/i2c/ov772x.h
> +F:   Documentation/devicetree/bindings/media/i2c/ov772x.txt
>
>  OMNIVISION OV7740 SENSOR DRIVER
>  M:   Wenyou Yang 
> --
> 2.7.4
>


signature.asc
Description: PGP signature

Re: [PATCH v2 04/10] media: ov772x: add media controller support

[jacopo mondi]

Hi Akinobu,

On Mon, Apr 16, 2018 at 11:51:45AM +0900, Akinobu Mita wrote:
> Create a source pad and set the media controller type to the sensor.
>
> Cc: Jacopo Mondi 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: Akinobu Mita 

Reviewed-by: Jacopo Mondi 

Not strictly on this patch, but I'm a bit confused on the difference
between CONFIG_MEDIA_CONTROLLER and CONFIG_VIDEO_V4L2_SUBDEV_API...
Doesn't media controller support mandate implementing subdev APIs as
well?

Thanks
   j
> ---
> * v2
> - Move video_probe() before the entity initialization and remove the #ifdef
>   around the media_entity_cleanup()
>
>  drivers/media/i2c/ov772x.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 188f2f1..0ae2a4f 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -424,6 +424,9 @@ struct ov772x_priv {
>   /* band_filter = COM8[5] ? 256 - BDBASE : 0 */
>   unsigned shortband_filter;
>   unsigned int  fps;
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + struct media_pad pad;
> +#endif
>  };
>
>  /*
> @@ -1318,16 +1321,26 @@ static int ov772x_probe(struct i2c_client *client,
>   if (ret < 0)
>   goto error_gpio_put;
>
> +#ifdef CONFIG_MEDIA_CONTROLLER
> + priv->pad.flags = MEDIA_PAD_FL_SOURCE;
> + priv->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> + ret = media_entity_pads_init(>subdev.entity, 1, >pad);
> + if (ret < 0)
> + goto error_gpio_put;
> +#endif
> +
>   priv->cfmt = _cfmts[0];
>   priv->win = _win_sizes[0];
>   priv->fps = 15;
>
>   ret = v4l2_async_register_subdev(>subdev);
>   if (ret)
> - goto error_gpio_put;
> + goto error_entity_cleanup;
>
>   return 0;
>
> +error_entity_cleanup:
> + media_entity_cleanup(>subdev.entity);
>  error_gpio_put:
>   if (priv->pwdn_gpio)
>   gpiod_put(priv->pwdn_gpio);
> @@ -1343,6 +1356,7 @@ static int ov772x_remove(struct i2c_client *client)
>  {
>   struct ov772x_priv *priv = to_ov772x(i2c_get_clientdata(client));
>
> + media_entity_cleanup(>subdev.entity);
>   clk_put(priv->clk);
>   if (priv->pwdn_gpio)
>   gpiod_put(priv->pwdn_gpio);
> --
> 2.7.4
>


signature.asc
Description: PGP signature

Re: [PATCH v2 7/7] media: rc: mceusb: allow the timeout to be configurable

[Sean Young]

Hello Hias,

On Tue, Apr 17, 2018 at 09:14:57PM +0200, Matthias Reichl wrote:
> On Sun, Apr 08, 2018 at 10:19:42PM +0100, Sean Young wrote:
> > mceusb devices have a default timeout of 100ms, but this can be changed.
> 
> We finally added a backport of the v2 series (and also the mce_kbd
> series) to LibreELEC yesterday and ratcher quickly received 2 bugreports
> from users using mceusb receivers.
> 
> Local testing on RPi/gpio-ir and Intel NUC/ite-cir was fine, I've
> been using the v2 series for over a week without issues on
> LibreELEC (RPi with kernel 4.14).
> 
> Here are the links to the bugreports and logs:
> https://forum.kodi.tv/showthread.php?tid=298461=2726684#pid2726684
> https://forum.kodi.tv/showthread.php?tid=298462=2726690#pid2726690
> 
> Both users are using similar mceusb receivers:
> 
> Log 1:
> [6.418218] rc rc0: Media Center Ed. eHome Infrared Remote Transceiver 
> (147a:e017) as 
> /devices/platform/soc/3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/rc/rc0
> [6.418358] input: Media Center Ed. eHome Infrared Remote Transceiver 
> (147a:e017) as 
> /devices/platform/soc/3f98.usb/usb1/1-1/1-1.3/1-1.3:1.0/rc/rc0/input0
> [6.419443] rc rc0: lirc_dev: driver ir-lirc-codec (mceusb) registered at 
> minor = 0
> [6.608114] mceusb 1-1.3:1.0: Registered Formosa21 SnowflakeEmulation with 
> mce emulator interface version 1
> [6.608125] mceusb 1-1.3:1.0: 0 tx ports (0x0 cabled) and 1 rx sensors 
> (0x1 active)
> 
> Log 2:
> [3.023361] rc rc0: Media Center Ed. eHome Infrared Remote Transceiver 
> (147a:e03e) as /devices/pci:00/:00:14.0/usb1/1-10/1-10:1.0/rc/rc0
> [3.023393] input: Media Center Ed. eHome Infrared Remote Transceiver 
> (147a:e03e) as 
> /devices/pci:00/:00:14.0/usb1/1-10/1-10:1.0/rc/rc0/input11
> [3.023868] rc rc0: lirc_dev: driver ir-lirc-codec (mceusb) registered at 
> minor = 0
> [3.119384] input: eventlircd as /devices/virtual/input/input21
> [3.138625] ip6_tables: (C) 2000-2006 Netfilter Core Team
> [3.196830] mceusb 1-10:1.0: Registered Formosa21 eHome Infrared 
> Transceiver with mce emulator interface version 2
> [3.196836] mceusb 1-10:1.0: 0 tx ports (0x0 cabled) and 1 rx sensors (0x1 
> active)
> 
> In both cases ir-keytable doesn't report any scancodes and the
> ir-ctl -r output contains very odd long space values where I'd expect
> a short timeout instead:
> 
> gap between messages:
> space 800
> pulse 450
> space 16777215
> space 25400
> pulse 2650
> space 800
> 
> end of last message:
> space 800
> pulse 450
> space 16777215
> timeout 31750
> 
> This patch applied cleanly on 4.14 and the mceusb history from
> 4.14 to media/master looked rather unsuspicious. I'm not 100% sure
> if I might have missed a dependency when backporting the patch
> or if this is indeed an issue of this patch on these particular
> (or maybe some more) mceusb receivers.

Thanks again for a great bug report and analysis! So, it seems with the
shorter timeout, some mceusb devices add a specific "timeout" code to
the IR data stream (0x80) rather than a space. The current mceusb code
resets the decoders in this case, causing the IR decoders to reset and
lirc to report a space of 0xff.

Turns out that one of my mceusb devices also suffers from this, I don't
know how I missed this. Anyway hopefully this will solve the problem.


Thanks

Sean

>From 92d27b206e51993e927dc0b3aba210a621eef3d0 Mon Sep 17 00:00:00 2001
From: Sean Young 
Date: Wed, 18 Apr 2018 10:36:25 +0100
Subject: [PATCH] media: rc: mceusb: IR of length 0 means IR timeout, not reset

The last usb packet with IR data will end with 0x80 (MCE_IRDATA_TRAILER).
If we reset the decoder state at this point, IR decoding can fail.

Signed-off-by: Sean Young 
---
 drivers/media/rc/mceusb.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/mceusb.c b/drivers/media/rc/mceusb.c
index c97cb2eb1c5f..5c0bf61fae26 100644
--- a/drivers/media/rc/mceusb.c
+++ b/drivers/media/rc/mceusb.c
@@ -1201,7 +1201,12 @@ static void mceusb_process_ir_data(struct mceusb_dev 
*ir, int buf_len)
if (ir->rem) {
ir->parser_state = PARSE_IRDATA;
} else {
-   ir_raw_event_reset(ir->rc);
+   init_ir_raw_event();
+   rawir.timeout = 1;
+   rawir.duration = ir->rc->timeout;
+   if (ir_raw_event_store_with_filter(ir->rc,
+  ))
+   event = true;
ir->pulse_tunit = 0;
ir->pulse_count = 0;
}
-- 
2.14.3

Re: [PATCH] media: v4l2-core: Change return type to vm_fault_t

[Souptick Joarder]

On Wed, Apr 18, 2018 at 2:38 PM, Sakari Ailus  wrote:
> On Tue, Apr 17, 2018 at 08:13:06PM +0530, Souptick Joarder wrote:
>> Use new return type vm_fault_t for fault handler. For
>> now, this is just documenting that the function returns
>> a VM_FAULT value rather than an errno. Once all instances
>> are converted, vm_fault_t will become a distinct type.
>>
>> Reference id -> 1c8f422059ae ("mm: change return type to
>> vm_fault_t")
>>
>> Signed-off-by: Souptick Joarder 
>
> Subject should mention "videobuf"; videobuf is not part of V4L2 core
> (albeit the directory structure would certainly seem to suggest that). With
> that,
>
> Acked-by: Sakari Ailus 

Thanks. I will update the subject and send v2.

[PATCH v4l-utils] media-ctl: add --get-dv option

[Philipp Zabel]

Printing the queried and current DV timings is already supported as part
of the --print-topology option. Add a --get-dv option to print DV
timings of an individual entitiy, to complement --set-dv.

Signed-off-by: Philipp Zabel 
---
 utils/media-ctl/media-ctl.c | 12 
 utils/media-ctl/options.c   |  7 +++
 utils/media-ctl/options.h   |  1 +
 3 files changed, 20 insertions(+)

diff --git a/utils/media-ctl/media-ctl.c b/utils/media-ctl/media-ctl.c
index a9417a3a..51da7f8a 100644
--- a/utils/media-ctl/media-ctl.c
+++ b/utils/media-ctl/media-ctl.c
@@ -604,6 +604,18 @@ int main(int argc, char **argv)
 V4L2_SUBDEV_FORMAT_ACTIVE);
}
 
+   if (media_opts.get_dv_pad) {
+   struct media_pad *pad;
+
+   pad = media_parse_pad(media, media_opts.get_dv_pad, NULL);
+   if (pad == NULL) {
+   printf("Pad '%s' not found\n", media_opts.get_dv_pad);
+   goto out;
+   }
+
+   v4l2_subdev_print_subdev_dv(pad->entity);
+   }
+
if (media_opts.dv_pad) {
struct v4l2_dv_timings timings;
struct media_pad *pad;
diff --git a/utils/media-ctl/options.c b/utils/media-ctl/options.c
index 83ca1cac..16367857 100644
--- a/utils/media-ctl/options.c
+++ b/utils/media-ctl/options.c
@@ -46,6 +46,7 @@ static void usage(const char *argv0)
printf("-e, --entity name   Print the device name associated with 
the given entity\n");
printf("-V, --set-v4l2 v4l2 Comma-separated list of formats to 
setup\n");
printf("--get-v4l2 pad  Print the active format on a given 
pad\n");
+   printf("--get-dv padPrint detected and current DV timings 
on a given pad\n");
printf("--set-dv padConfigure DV timings on a given pad\n");
printf("-h, --help  Show verbose help and exit\n");
printf("-i, --interactive   Modify links interactively\n");
@@ -117,6 +118,7 @@ static void usage(const char *argv0)
 #define OPT_GET_FORMAT 257
 #define OPT_SET_DV 258
 #define OPT_LIST_KNOWN_MBUS_FMTS   259
+#define OPT_GET_DV 260
 
 static struct option opts[] = {
{"device", 1, 0, 'd'},
@@ -125,6 +127,7 @@ static struct option opts[] = {
{"set-v4l2", 1, 0, 'V'},
{"get-format", 1, 0, OPT_GET_FORMAT},
{"get-v4l2", 1, 0, OPT_GET_FORMAT},
+   {"get-dv", 1, 0, OPT_GET_DV},
{"set-dv", 1, 0, OPT_SET_DV},
{"help", 0, 0, 'h'},
{"interactive", 0, 0, 'i'},
@@ -222,6 +225,10 @@ int parse_cmdline(int argc, char **argv)
media_opts.fmt_pad = optarg;
break;
 
+   case OPT_GET_DV:
+   media_opts.get_dv_pad = optarg;
+   break;
+
case OPT_SET_DV:
media_opts.dv_pad = optarg;
break;
diff --git a/utils/media-ctl/options.h b/utils/media-ctl/options.h
index 9b5f314e..7e0556fc 100644
--- a/utils/media-ctl/options.h
+++ b/utils/media-ctl/options.h
@@ -34,6 +34,7 @@ struct media_options
const char *formats;
const char *links;
const char *fmt_pad;
+   const char *get_dv_pad;
const char *dv_pad;
 };
 
-- 
2.16.3

Re: [PATCH v2 01/10] media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING

[Sakari Ailus]

On Wed, Apr 18, 2018 at 12:05:49PM +0200, jacopo mondi wrote:
> Hi Akinobu,
> 
> On Mon, Apr 16, 2018 at 11:51:42AM +0900, Akinobu Mita wrote:
> > The ov772x driver only works when the i2c controller have
> > I2C_FUNC_PROTOCOL_MANGLING.  However, many i2c controller drivers don't
> > support it.
> >
> > The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that
> > it doesn't support repeated starts.
> >
> > This changes the reading ov772x register method so that it doesn't
> > require I2C_FUNC_PROTOCOL_MANGLING by calling two separated i2c messages.
> >
> > Cc: Jacopo Mondi 
> > Cc: Laurent Pinchart 
> > Cc: Hans Verkuil 
> > Cc: Sakari Ailus 
> > Cc: Mauro Carvalho Chehab 
> > Signed-off-by: Akinobu Mita 
> > ---
> > * v2
> > - Replace the implementation of ov772x_read() instead of adding an
> >   alternative method
> 
> I now wonder if my initial reply to this patch was wrong, and where
> possible we should try to use smbus operations...
> 
> From Documentation/i2c/smbus-protocol
> "If you write a driver for some I2C device, please try to use the SMBus
> commands if at all possible... " and that's because, according to
> documentation, most I2c adapters support smbus protocol but may not
> support the full i2c command set.
> 
> The fact this driver then restricts the supported adapters to the ones
> that support protocol mangling makes me think your change is fine,
> but as it often happens, I would scale this to more knowledgable
> people...

Do you actually need to use this on SMBus adapters? A lot of sensor drivers
just use I²C; if SMBus support is really needed it can be always added back
later on...

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

Re: [PATCH v2 02/10] media: ov772x: add checks for register read errors

[jacopo mondi]

Hi Akinobu,

On Mon, Apr 16, 2018 at 11:51:43AM +0900, Akinobu Mita wrote:
> This change adds checks for register read errors and returns correct
> error code.
>
> Cc: Jacopo Mondi 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: Akinobu Mita 

Reviewed-by: Jacopo Mondi 

> ---
> * v2
> - Assign the ov772x_read() return value to pid and ver directly
> - Do the same for MIDH and MIDL
>
>  drivers/media/i2c/ov772x.c | 20 ++--
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 7e79da0..8badd6f 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -1146,7 +1146,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
>  static int ov772x_video_probe(struct ov772x_priv *priv)
>  {
>   struct i2c_client  *client = v4l2_get_subdevdata(>subdev);
> - u8  pid, ver;
> + int pid, ver, midh, midl;
>   const char *devname;
>   int ret;
>
> @@ -1156,7 +1156,11 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
>
>   /* Check and show product ID and manufacturer ID. */
>   pid = ov772x_read(client, PID);
> + if (pid < 0)
> + return pid;
>   ver = ov772x_read(client, VER);
> + if (ver < 0)
> + return ver;
>
>   switch (VERSION(pid, ver)) {
>   case OV7720:
> @@ -1172,13 +1176,17 @@ static int ov772x_video_probe(struct ov772x_priv 
> *priv)
>   goto done;
>   }
>
> + midh = ov772x_read(client, MIDH);
> + if (midh < 0)
> + return midh;
> + midl = ov772x_read(client, MIDL);
> + if (midl < 0)
> + return midl;
> +
>   dev_info(>dev,
>"%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
> -  devname,
> -  pid,
> -  ver,
> -  ov772x_read(client, MIDH),
> -  ov772x_read(client, MIDL));
> +  devname, pid, ver, midh, midl);
> +
>   ret = v4l2_ctrl_handler_setup(>hdl);
>
>  done:
> --
> 2.7.4
>


signature.asc
Description: PGP signature

Re: [PATCH v2 01/10] media: ov772x: allow i2c controllers without I2C_FUNC_PROTOCOL_MANGLING

[jacopo mondi]

Hi Akinobu,

On Mon, Apr 16, 2018 at 11:51:42AM +0900, Akinobu Mita wrote:
> The ov772x driver only works when the i2c controller have
> I2C_FUNC_PROTOCOL_MANGLING.  However, many i2c controller drivers don't
> support it.
>
> The reason that the ov772x requires I2C_FUNC_PROTOCOL_MANGLING is that
> it doesn't support repeated starts.
>
> This changes the reading ov772x register method so that it doesn't
> require I2C_FUNC_PROTOCOL_MANGLING by calling two separated i2c messages.
>
> Cc: Jacopo Mondi 
> Cc: Laurent Pinchart 
> Cc: Hans Verkuil 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Signed-off-by: Akinobu Mita 
> ---
> * v2
> - Replace the implementation of ov772x_read() instead of adding an
>   alternative method

I now wonder if my initial reply to this patch was wrong, and where
possible we should try to use smbus operations...

From Documentation/i2c/smbus-protocol
"If you write a driver for some I2C device, please try to use the SMBus
commands if at all possible... " and that's because, according to
documentation, most I2c adapters support smbus protocol but may not
support the full i2c command set.

The fact this driver then restricts the supported adapters to the ones
that support protocol mangling makes me think your change is fine,
but as it often happens, I would scale this to more knowledgable
people...

>
>  drivers/media/i2c/ov772x.c | 19 ++-
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index b62860c..7e79da0 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -542,9 +542,19 @@ static struct ov772x_priv *to_ov772x(struct v4l2_subdev 
> *sd)
>   return container_of(sd, struct ov772x_priv, subdev);
>  }
>
> -static inline int ov772x_read(struct i2c_client *client, u8 addr)
> +static int ov772x_read(struct i2c_client *client, u8 addr)
>  {
> - return i2c_smbus_read_byte_data(client, addr);
> + int ret;
> + u8 val;
> +
> + ret = i2c_master_send(client, , 1);
> + if (ret < 0)
> + return ret;
> + ret = i2c_master_recv(client, , 1);
> + if (ret < 0)
> + return ret;
> +
> + return val;
>  }
>
>  static inline int ov772x_write(struct i2c_client *client, u8 addr, u8 value)
> @@ -1255,10 +1265,9 @@ static int ov772x_probe(struct i2c_client *client,
>   return -EINVAL;
>   }
>
> - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
> -   I2C_FUNC_PROTOCOL_MANGLING)) {
> + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
>   dev_err(>dev,
> - "I2C-Adapter doesn't support SMBUS_BYTE_DATA or 
> PROTOCOL_MANGLING\n");
> + "I2C-Adapter doesn't support SMBUS_BYTE_DATA\n");
>   return -EIO;
>   }
>   client->flags |= I2C_CLIENT_SCCB;

I think you can remove this flag if we're using plain i2c
transactions. Sorry, I have missed this in v1.

Thanks
   j

> --
> 2.7.4
>


signature.asc
Description: PGP signature

Re: [PATCH] media: v4l2-compat-ioctl32: simplify casts

[Mauro Carvalho Chehab]

Em Wed, 18 Apr 2018 11:57:13 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> On Tue, Apr 17, 2018 at 10:25:37AM -0400, Mauro Carvalho Chehab wrote:
> > Making the cast right for get_user/put_user is not trivial, as
> > it needs to ensure that the types are the correct ones.
> > 
> > Improve it by using macros.
> > 
> > Tested with vivid with:
> > $ sudo modprobe vivid no_error_inj=1
> > $ v4l2-compliance-32bits -a -s10 >32bits && v4l2-compliance-64bits -a 
> > -s10 > 64bits && diff -U0 32bits 64bits
> > --- 32bits  2018-04-17 11:18:29.141240772 -0300
> > +++ 64bits  2018-04-17 11:18:40.635282341 -0300
> > @@ -1 +1 @@
> > -v4l2-compliance SHA   : bc71e4a67c6fbc5940062843bc41e7c8679634ce, 32 
> > bits
> > +v4l2-compliance SHA   : bc71e4a67c6fbc5940062843bc41e7c8679634ce, 64 
> > bits
> > 
> > Using the latest version of v4l-utils with this patch applied:
> > https://patchwork.linuxtv.org/patch/48746/
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 40 
> > ++-
> >  1 file changed, 27 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
> > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > index 8c05dd9660d3..d2f0268427c2 100644
> > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > @@ -30,6 +30,24 @@
> > get_user(__assign_tmp, from) || put_user(__assign_tmp, to); \
> >  })
> >  
> > +#define get_user_cast(__x, __ptr)  \
> > +({ \
> > +   get_user(__x, (typeof(*__ptr) __user *)(__ptr));\
> > +})
> > +
> > +#define put_user_force(__x, __ptr) \
> > +({ \
> > +   put_user((typeof(*__x) __force *)(__x), __ptr); \
> > +})
> > +
> > +#define assign_in_user_cast(to, from)  
> > \
> > +({ \
> > +   typeof(*from) __assign_tmp; \
> > +   \
> > +   get_user_cast(__assign_tmp, from) || put_user(__assign_tmp, to);\
> > +})  
> 
> I must say I like the approach. This makes it harder to get things wrong as
> the compiler still does type checking.
> 
> About the naming --- I understand the postfix comes from the modifier used
> in the put_user case but in the others the same approach would lead to
> names such as get_user_user() which would look a bit confusing.

Yes. That's why I used _cast for the version that does the __user cast.

> 
> I presume the patch depends on another patch from you. 

Yes, it comes after the p32/p64 rename.

> Hans had comments on
> that one that I think affect this one as well; 

This patch was added to address Hans comments on patch 4/5. Due to
the renaming, folding it with patch 4/5 is not possible without big
efforts.

> could you add this one to
> the same patchset?

My plan is to apply all those 3 patches together after being reviewed.

The 3 omap3 patches + the 3 ioctl32 patches are altogether here:
https://git.linuxtv.org/mchehab/experimental.git/log/?h=compile_test_v6b

> 
> > +
> > +
> >  static long native_ioctl(struct file *file, unsigned int cmd, unsigned 
> > long arg)
> >  {
> > long ret = -ENOIOCTLCMD;
> > @@ -543,8 +561,7 @@ static int get_v4l2_buffer32(struct v4l2_buffer __user 
> > *p64,
> > return -EFAULT;
> >  
> > uplane = aux_buf;
> > -   if (put_user((__force struct v4l2_plane *)uplane,
> > ->m.planes))
> > +   if (put_user_force(uplane, >m.planes))
> > return -EFAULT;
> >  
> > while (num_planes--) {
> > @@ -682,7 +699,7 @@ static int get_v4l2_framebuffer32(struct 
> > v4l2_framebuffer __user *p64,
> >  
> > if (!access_ok(VERIFY_READ, p32, sizeof(*p32)) ||
> > get_user(tmp, >base) ||
> > -   put_user((void __force *)compat_ptr(tmp), >base) ||
> > +   put_user_force(compat_ptr(tmp), >base) ||
> > assign_in_user(>capability, >capability) ||
> > assign_in_user(>flags, >flags) ||
> > copy_in_user(>fmt, >fmt, sizeof(p64->fmt)))
> > @@ -831,8 +848,7 @@ static int get_v4l2_ext_controls32(struct file *file,
> > if (aux_space < count * sizeof(*kcontrols))
> > return -EFAULT;
> > kcontrols = aux_buf;
> > -   if (put_user((__force struct v4l2_ext_control *)kcontrols,
> > ->controls))
> > +   if (put_user_force(kcontrols, >controls))
> > return -EFAULT;
> >  
> > for (n = 0; n < count; n++) {
> > @@ -898,12 +914,11 @@ static int put_v4l2_ext_controls32(struct file 

Re: [PATCH] media: v4l2-core: Change return type to vm_fault_t

[Sakari Ailus]

On Tue, Apr 17, 2018 at 08:13:06PM +0530, Souptick Joarder wrote:
> Use new return type vm_fault_t for fault handler. For
> now, this is just documenting that the function returns
> a VM_FAULT value rather than an errno. Once all instances
> are converted, vm_fault_t will become a distinct type.
> 
> Reference id -> 1c8f422059ae ("mm: change return type to
> vm_fault_t")
> 
> Signed-off-by: Souptick Joarder 

Subject should mention "videobuf"; videobuf is not part of V4L2 core
(albeit the directory structure would certainly seem to suggest that). With
that,

Acked-by: Sakari Ailus 

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

Re: Regression: DVBSky S960 USB tuner doesn't work in 4.10 or newer

[Antti Palosaari]

On 04/18/2018 07:49 AM, Olli Salonen wrote:

Thank you for your response Peter!

Indeed, it seems strange. dvbsky.c driver seems to use mutex_lock in
very much the same way as many other drivers. I've now confirmed that
I can get a 4.10 kernel with working DVBSky S960 by reverting the
following 4 patches:

549bdd3 Revert "locking/mutex: Add lock handoff to avoid starvation"
3210f31 Revert "locking/mutex: Restructure wait loop"
418a170 Revert "locking/mutex: Simplify some ww_mutex code in
__mutex_lock_common()"
0b1fb8f Revert "locking/mutex: Enable optimistic spinning of woken waiter"
c470abd Linux 4.10


These kind of issues tend to be timing issues very often. Just add some 
sleeps to i2c adapter algo / usb control messages and test.


regards
Antti




--
http://palosaari.fi/

Re: [PATCH v8 2/2] media: video-i2c: add video-i2c driver

[Matt Ranostay]

On Wed, Apr 18, 2018 at 1:03 AM, Sakari Ailus  wrote:
> Hi Matt,
>
> Thanks for the update.
>
> On Fri, Apr 06, 2018 at 03:52:31PM -0700, Matt Ranostay wrote:
>> There are several thermal sensors that only have a low-speed bus
>> interface but output valid video data. This patchset enables support
>> for the AMG88xx "Grid-Eye" sensor family.
>>
>> Signed-off-by: Matt Ranostay 
>> ---
>>  MAINTAINERS   |   6 +
>>  drivers/media/i2c/Kconfig |  13 +
>>  drivers/media/i2c/Makefile|   1 +
>>  drivers/media/i2c/video-i2c.c | 563 
>> ++
>>  4 files changed, 583 insertions(+)
>>  create mode 100644 drivers/media/i2c/video-i2c.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index dc153da22e8a..928b6a862626 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -14880,6 +14880,12 @@ L:   linux-media@vger.kernel.org
>>  S:   Maintained
>>  F:   drivers/media/platform/video-mux.c
>>
>> +VIDEO I2C POLLING DRIVER
>> +M:   Matt Ranostay 
>> +L:   linux-media@vger.kernel.org
>> +S:   Maintained
>> +F:   drivers/media/i2c/video-i2c.c
>> +
>>  VIDEOBUF2 FRAMEWORK
>>  M:   Pawel Osciak 
>>  M:   Marek Szyprowski 
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index 541f0d28afd8..faaaceb94832 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -974,6 +974,19 @@ config VIDEO_M52790
>>
>>To compile this driver as a module, choose M here: the
>>module will be called m52790.
>> +
>> +config VIDEO_I2C
>> + tristate "I2C transport video support"
>> + depends on VIDEO_V4L2 && I2C
>> + select VIDEOBUF2_VMALLOC
>> + ---help---
>> +   Enable the I2C transport video support which supports the
>> +   following:
>> +* Panasonic AMG88xx Grid-Eye Sensors
>> +
>> +   To compile this driver as a module, choose M here: the
>> +   module will be called video-i2c
>> +
>>  endmenu
>>
>>  menu "Sensors used on soc_camera driver"
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index ea34aee1a85a..84cc472238ef 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -96,6 +96,7 @@ obj-$(CONFIG_VIDEO_LM3646)  += lm3646.o
>>  obj-$(CONFIG_VIDEO_SMIAPP_PLL)   += smiapp-pll.o
>>  obj-$(CONFIG_VIDEO_AK881X)   += ak881x.o
>>  obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
>> +obj-$(CONFIG_VIDEO_I2C)  += video-i2c.o
>>  obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o
>>  obj-$(CONFIG_VIDEO_OV2659)   += ov2659.o
>>  obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
>> diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
>> new file mode 100644
>> index ..ec8a597597bf
>> --- /dev/null
>> +++ b/drivers/media/i2c/video-i2c.c
>> @@ -0,0 +1,563 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * video-i2c.c - Support for I2C transport video devices
>> + *
>> + * Copyright (C) 2018 Matt Ranostay 
>> + *
>> + * Supported:
>> + * - Panasonic AMG88xx Grid-Eye Sensors
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define VIDEO_I2C_DRIVER "video-i2c"
>> +
>> +struct video_i2c_chip;
>> +
>> +struct video_i2c_buffer {
>> + struct vb2_v4l2_buffer vb;
>> + struct list_head list;
>> +};
>> +
>> +struct video_i2c_data {
>> + struct i2c_client *client;
>> + const struct video_i2c_chip *chip;
>> + struct mutex lock;
>> + spinlock_t slock;
>> + unsigned int sequence;
>> + struct mutex queue_lock;
>> +
>> + struct v4l2_device v4l2_dev;
>> + struct video_device vdev;
>> + struct vb2_queue vb_vidq;
>> +
>> + struct task_struct *kthread_vid_cap;
>> + struct list_head vid_cap_active;
>> +};
>> +
>> +const static struct v4l2_fmtdesc amg88xx_format = {
>> + .pixelformat = V4L2_PIX_FMT_Y12,
>> +};
>> +
>> +const static struct v4l2_frmsize_discrete amg88xx_size = {
>> + .width = 8,
>> + .height = 8,
>> +};
>> +
>> +struct video_i2c_chip {
>> + /* video dimensions */
>> + const struct v4l2_fmtdesc *format;
>> + const struct v4l2_frmsize_discrete *size;
>> +
>> + /* max frames per second */
>> + unsigned int max_fps;
>> +
>> + /* pixel buffer size */
>> + unsigned int buffer_size;
>> +
>> + /* pixel size in bits */
>> + unsigned int bpp;
>> +
>> + /* xfer function */
>> + int (*xfer)(struct video_i2c_data *data, char *buf);
>> +};
>> +
>> +static int amg88xx_xfer(struct video_i2c_data *data, char *buf)
>> +{
>> + struct i2c_client *client = data->client;
>> + struct i2c_msg msg[2];
>> + u8 reg = 0x80;
>> + 

[PATCH v9 1/2] dt-bindings: media: Add Cadence MIPI-CSI2 TX Device Tree bindings

[Maxime Ripard]

The Cadence MIPI-CSI2 TX controller is a CSI2 bridge that supports up to 4
video streams and can output on up to 4 CSI-2 lanes, depending on the
hardware implementation.

It can operate with an external D-PHY, an internal one or no D-PHY at all
in some configurations.

Acked-by: Rob Herring 
Acked-by: Sakari Ailus 
Reviewed-by: Niklas Söderlund 
Signed-off-by: Maxime Ripard 
---
 .../devicetree/bindings/media/cdns,csi2tx.txt | 98 +++
 1 file changed, 98 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/cdns,csi2tx.txt

diff --git a/Documentation/devicetree/bindings/media/cdns,csi2tx.txt 
b/Documentation/devicetree/bindings/media/cdns,csi2tx.txt
new file mode 100644
index ..459c6e332f52
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/cdns,csi2tx.txt
@@ -0,0 +1,98 @@
+Cadence MIPI-CSI2 TX controller
+===
+
+The Cadence MIPI-CSI2 TX controller is a CSI-2 bridge supporting up to
+4 CSI lanes in output, and up to 4 different pixel streams in input.
+
+Required properties:
+  - compatible: must be set to "cdns,csi2tx"
+  - reg: base address and size of the memory mapped region
+  - clocks: phandles to the clocks driving the controller
+  - clock-names: must contain:
+* esc_clk: escape mode clock
+* p_clk: register bank clock
+* pixel_if[0-3]_clk: pixel stream output clock, one for each stream
+ implemented in hardware, between 0 and 3
+
+Optional properties
+  - phys: phandle to the D-PHY. If it is set, phy-names need to be set
+  - phy-names: must contain "dphy"
+
+Required subnodes:
+  - ports: A ports node with one port child node per device input and output
+   port, in accordance with the video interface bindings defined in
+   Documentation/devicetree/bindings/media/video-interfaces.txt. The
+   port nodes are numbered as follows.
+
+   Port Description
+   -
+   0CSI-2 output
+   1Stream 0 input
+   2Stream 1 input
+   3Stream 2 input
+   4Stream 3 input
+
+   The stream input port nodes are optional if they are not
+   connected to anything at the hardware level or implemented
+   in the design. Since there is only one endpoint per port,
+   the endpoints are not numbered.
+
+Example:
+
+csi2tx: csi-bridge@0d0e1000 {
+   compatible = "cdns,csi2tx";
+   reg = <0x0d0e1000 0x1000>;
+   clocks = <>, <>,
+<>, <>,
+<>, <>;
+   clock-names = "p_clk", "esc_clk",
+ "pixel_if0_clk", "pixel_if1_clk",
+ "pixel_if2_clk", "pixel_if3_clk";
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port@0 {
+   reg = <0>;
+
+   csi2tx_out: endpoint {
+   remote-endpoint = <_in>;
+   clock-lanes = <0>;
+   data-lanes = <1 2>;
+   };
+   };
+
+   port@1 {
+   reg = <1>;
+
+   csi2tx_in_stream0: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+
+   port@2 {
+   reg = <2>;
+
+   csi2tx_in_stream1: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+
+   port@3 {
+   reg = <3>;
+
+   csi2tx_in_stream2: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+
+   port@4 {
+   reg = <4>;
+
+   csi2tx_in_stream3: endpoint {
+   remote-endpoint = <_out>;
+   };
+   };
+   };
+};
-- 
2.17.0

[PATCH v9 2/2] v4l: cadence: Add Cadence MIPI-CSI2 TX driver

[Maxime Ripard]

The Cadence MIPI-CSI2 TX controller is an hardware block meant to be used
as a bridge between pixel interfaces and a CSI-2 bus.

It supports operating with an internal or external D-PHY, with up to 4
lanes, or without any D-PHY. The current code only supports the latter
case.

While the virtual channel input on the pixel interface can be directly
mapped to CSI2, the datatype input is actually a selection signal (3-bits)
mapping to a table of up to 8 preconfigured datatypes/formats (programmed
at start-up)

The block supports up to 8 input datatypes.

Reviewed-by: Niklas Söderlund 
Signed-off-by: Maxime Ripard 
---
 drivers/media/platform/cadence/Kconfig   |  11 +
 drivers/media/platform/cadence/Makefile  |   1 +
 drivers/media/platform/cadence/cdns-csi2tx.c | 530 +++
 3 files changed, 542 insertions(+)
 create mode 100644 drivers/media/platform/cadence/cdns-csi2tx.c

diff --git a/drivers/media/platform/cadence/Kconfig 
b/drivers/media/platform/cadence/Kconfig
index 70c95d79c8f7..3bf0f2454384 100644
--- a/drivers/media/platform/cadence/Kconfig
+++ b/drivers/media/platform/cadence/Kconfig
@@ -20,4 +20,15 @@ config VIDEO_CADENCE_CSI2RX
  To compile this driver as a module, choose M here: the module will be
  called cdns-csi2rx.
 
+config VIDEO_CADENCE_CSI2TX
+   tristate "Cadence MIPI-CSI2 TX Controller"
+   depends on MEDIA_CONTROLLER
+   depends on VIDEO_V4L2_SUBDEV_API
+   select V4L2_FWNODE
+   help
+ Support for the Cadence MIPI CSI2 Transceiver controller.
+
+ To compile this driver as a module, choose M here: the module will be
+ called cdns-csi2tx.
+
 endif
diff --git a/drivers/media/platform/cadence/Makefile 
b/drivers/media/platform/cadence/Makefile
index 99a4086b7448..7fe992273162 100644
--- a/drivers/media/platform/cadence/Makefile
+++ b/drivers/media/platform/cadence/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_VIDEO_CADENCE_CSI2RX) += cdns-csi2rx.o
+obj-$(CONFIG_VIDEO_CADENCE_CSI2TX) += cdns-csi2tx.o
diff --git a/drivers/media/platform/cadence/cdns-csi2tx.c 
b/drivers/media/platform/cadence/cdns-csi2tx.c
new file mode 100644
index ..9c68c78f990b
--- /dev/null
+++ b/drivers/media/platform/cadence/cdns-csi2tx.c
@@ -0,0 +1,530 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Cadence MIPI-CSI2 TX Controller
+ *
+ * Copyright (C) 2017-2018 Cadence Design Systems Inc.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#define CSI2TX_DEVICE_CONFIG_REG   0x00
+#define CSI2TX_DEVICE_CONFIG_STREAMS_MASK  GENMASK(6, 4)
+#define CSI2TX_DEVICE_CONFIG_HAS_DPHY  BIT(3)
+#define CSI2TX_DEVICE_CONFIG_LANES_MASKGENMASK(2, 0)
+
+#define CSI2TX_CONFIG_REG  0x20
+#define CSI2TX_CONFIG_CFG_REQ  BIT(2)
+#define CSI2TX_CONFIG_SRST_REQ BIT(1)
+
+#define CSI2TX_DPHY_CFG_REG0x28
+#define CSI2TX_DPHY_CFG_CLK_RESET  BIT(16)
+#define CSI2TX_DPHY_CFG_LANE_RESET(n)  BIT((n) + 12)
+#define CSI2TX_DPHY_CFG_MODE_MASK  GENMASK(9, 8)
+#define CSI2TX_DPHY_CFG_MODE_LPDT  (2 << 8)
+#define CSI2TX_DPHY_CFG_MODE_HS(1 << 8)
+#define CSI2TX_DPHY_CFG_MODE_ULPS  (0 << 8)
+#define CSI2TX_DPHY_CFG_CLK_ENABLE BIT(4)
+#define CSI2TX_DPHY_CFG_LANE_ENABLE(n) BIT(n)
+
+#define CSI2TX_DPHY_CLK_WAKEUP_REG 0x2c
+#define CSI2TX_DPHY_CLK_WAKEUP_ULPS_CYCLES(n)  ((n) & 0x)
+
+#define CSI2TX_DT_CFG_REG(n)   (0x80 + (n) * 8)
+#define CSI2TX_DT_CFG_DT(n)(((n) & 0x3f) << 2)
+
+#define CSI2TX_DT_FORMAT_REG(n)(0x84 + (n) * 8)
+#define CSI2TX_DT_FORMAT_BYTES_PER_LINE(n) (((n) & 0x) << 16)
+#define CSI2TX_DT_FORMAT_MAX_LINE_NUM(n)   ((n) & 0x)
+
+#define CSI2TX_STREAM_IF_CFG_REG(n)(0x100 + (n) * 4)
+#define CSI2TX_STREAM_IF_CFG_FILL_LEVEL(n) ((n) & 0x1f)
+
+#define CSI2TX_LANES_MAX   4
+#define CSI2TX_STREAMS_MAX 4
+
+enum csi2tx_pads {
+   CSI2TX_PAD_SOURCE,
+   CSI2TX_PAD_SINK_STREAM0,
+   CSI2TX_PAD_SINK_STREAM1,
+   CSI2TX_PAD_SINK_STREAM2,
+   CSI2TX_PAD_SINK_STREAM3,
+   CSI2TX_PAD_MAX,
+};
+
+struct csi2tx_fmt {
+   u32 mbus;
+   u32 dt;
+   u32 bpp;
+};
+
+struct csi2tx_priv {
+   struct device   *dev;
+   unsigned intcount;
+
+   /*
+* Used to prevent race conditions between multiple,
+* concurrent calls to start and stop.
+*/
+   struct mutexlock;
+
+   void __iomem*base;
+
+   struct clk  *esc_clk;
+   struct clk  *p_clk;
+   struct clk  *pixel_clk[CSI2TX_STREAMS_MAX];
+
+   struct 

[PATCH v9 0/2] media: v4l: Add support for the Cadence MIPI-CSI2 TX controller

[Maxime Ripard]

Hi,

Here is an attempt at supporting the MIPI-CSI2 TX block from Cadence.

This IP block is able to receive 4 video streams and stream them over
a MIPI-CSI2 link using up to 4 lanes. Those streams are basically the
interfaces to controllers generating some video signals, like a camera
or a pattern generator.

It is able to map input streams to CSI2 virtual channels and datatypes
dynamically. The streaming devices choose their virtual channels
through an additional signal that is transparent to the CSI2-TX. The
datatypes however are yet another additional input signal, and can be
mapped to any CSI2 datatypes.

Since v4l2 doesn't really allow for that setup at the moment, this
preliminary version is a rather dumb one in order to start the
discussion on how to address this properly.

Let me know what you think!
Maxime

Changes from v8:
  - Updated the copyright notice
  - Used masks for the device config fields
  - Used the mbus code directly in our format lookup function
  - Removed the pad index check in pads get/set_format
  - Prevent the format to be changed on the CSI2 pad
  - Check for supported formats in set_format
  - Rebased on 4.17-rc1

Changes from v7:
  - Removed unused headers
  - Fixed a function prefix inconsistency
  - Added Niklas' Reviewed-by

Changes from v6:
  - Added Niklas Reviewed-by on the bindings
  - Added MODULE_LICENSE, MODULE_DESCRIPTION and MODULE_AUTHOR

Changes from v5:
  - Changed the return type to void for csi2tx_stop
  - Checked for the pad index in get/set_pad_format
  - Made a comment that the DPHY registers are for the DPHY *interface*
register

Changes from v4:
  - After playing a bit with the pad multiplexing patches, found that it
was making much more sense to have the subdev notifiers for the source
subdev rather for the sink that might even be outside of Linux control.
Removed the notifier for now.

Changes from v3:
  - Added a comment about entity links walk concurrency
  - Changed the default resolution to 1280x720
  - Changed usleep_range calls to udelay
  - Reworked the reference counting mechanism to remove a race
condition by adding a mutex instead of an atomic count
  - Changed the entity function to MEDIA_ENT_F_VID_IF_BRIDGE
  - Changed the name of the reg variable in _get_resources to dev_cfg
  - Removed the redundant error message in the devm_ioremap_resource
error path
  - Moved the subdev s_stream call before enabling the TX bridge
  - Changed some int types to unsigned
  - Init'd the pad formats properly
  - Fixed typo in the CSI2TX_LANES_MAX define name
  - Added Sakari Acked-by

Changes from v2:
  - Use SPDX license header
  - Use the lane mapping from DT

Changes from v1:
  - Add a subdev notifier and start our downstream subdevice in
s_stream  
  - Based the decision to enable the stream or not on the link state
instead of whether a format was being set on the pad
  - Put the controller back in reset when stopping the pipeline
  - Clarified the enpoints number in the DT binding
  - Added a default format for the pads
  - Added some missing const
  - Added more explicit comments
  - Rebased on 4.15

Maxime Ripard (2):
  dt-bindings: media: Add Cadence MIPI-CSI2 TX Device Tree bindings
  v4l: cadence: Add Cadence MIPI-CSI2 TX driver

 .../devicetree/bindings/media/cdns,csi2tx.txt |  98 
 drivers/media/platform/cadence/Kconfig|  11 +
 drivers/media/platform/cadence/Makefile   |   1 +
 drivers/media/platform/cadence/cdns-csi2tx.c  | 530 ++
 4 files changed, 640 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/cdns,csi2tx.txt
 create mode 100644 drivers/media/platform/cadence/cdns-csi2tx.c

-- 
2.17.0

Re: [PATCH v8 2/2] media: video-i2c: add video-i2c driver

[Sakari Ailus]

Hi Matt,

Thanks for the update.

On Fri, Apr 06, 2018 at 03:52:31PM -0700, Matt Ranostay wrote:
> There are several thermal sensors that only have a low-speed bus
> interface but output valid video data. This patchset enables support
> for the AMG88xx "Grid-Eye" sensor family.
> 
> Signed-off-by: Matt Ranostay 
> ---
>  MAINTAINERS   |   6 +
>  drivers/media/i2c/Kconfig |  13 +
>  drivers/media/i2c/Makefile|   1 +
>  drivers/media/i2c/video-i2c.c | 563 
> ++
>  4 files changed, 583 insertions(+)
>  create mode 100644 drivers/media/i2c/video-i2c.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dc153da22e8a..928b6a862626 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14880,6 +14880,12 @@ L:   linux-media@vger.kernel.org
>  S:   Maintained
>  F:   drivers/media/platform/video-mux.c
>  
> +VIDEO I2C POLLING DRIVER
> +M:   Matt Ranostay 
> +L:   linux-media@vger.kernel.org
> +S:   Maintained
> +F:   drivers/media/i2c/video-i2c.c
> +
>  VIDEOBUF2 FRAMEWORK
>  M:   Pawel Osciak 
>  M:   Marek Szyprowski 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 541f0d28afd8..faaaceb94832 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -974,6 +974,19 @@ config VIDEO_M52790
>  
>To compile this driver as a module, choose M here: the
>module will be called m52790.
> +
> +config VIDEO_I2C
> + tristate "I2C transport video support"
> + depends on VIDEO_V4L2 && I2C
> + select VIDEOBUF2_VMALLOC
> + ---help---
> +   Enable the I2C transport video support which supports the
> +   following:
> +* Panasonic AMG88xx Grid-Eye Sensors
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called video-i2c
> +
>  endmenu
>  
>  menu "Sensors used on soc_camera driver"
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index ea34aee1a85a..84cc472238ef 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -96,6 +96,7 @@ obj-$(CONFIG_VIDEO_LM3646)  += lm3646.o
>  obj-$(CONFIG_VIDEO_SMIAPP_PLL)   += smiapp-pll.o
>  obj-$(CONFIG_VIDEO_AK881X)   += ak881x.o
>  obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
> +obj-$(CONFIG_VIDEO_I2C)  += video-i2c.o
>  obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o
>  obj-$(CONFIG_VIDEO_OV2659)   += ov2659.o
>  obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> new file mode 100644
> index ..ec8a597597bf
> --- /dev/null
> +++ b/drivers/media/i2c/video-i2c.c
> @@ -0,0 +1,563 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * video-i2c.c - Support for I2C transport video devices
> + *
> + * Copyright (C) 2018 Matt Ranostay 
> + *
> + * Supported:
> + * - Panasonic AMG88xx Grid-Eye Sensors
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define VIDEO_I2C_DRIVER "video-i2c"
> +
> +struct video_i2c_chip;
> +
> +struct video_i2c_buffer {
> + struct vb2_v4l2_buffer vb;
> + struct list_head list;
> +};
> +
> +struct video_i2c_data {
> + struct i2c_client *client;
> + const struct video_i2c_chip *chip;
> + struct mutex lock;
> + spinlock_t slock;
> + unsigned int sequence;
> + struct mutex queue_lock;
> +
> + struct v4l2_device v4l2_dev;
> + struct video_device vdev;
> + struct vb2_queue vb_vidq;
> +
> + struct task_struct *kthread_vid_cap;
> + struct list_head vid_cap_active;
> +};
> +
> +const static struct v4l2_fmtdesc amg88xx_format = {
> + .pixelformat = V4L2_PIX_FMT_Y12,
> +};
> +
> +const static struct v4l2_frmsize_discrete amg88xx_size = {
> + .width = 8,
> + .height = 8,
> +};
> +
> +struct video_i2c_chip {
> + /* video dimensions */
> + const struct v4l2_fmtdesc *format;
> + const struct v4l2_frmsize_discrete *size;
> +
> + /* max frames per second */
> + unsigned int max_fps;
> +
> + /* pixel buffer size */
> + unsigned int buffer_size;
> +
> + /* pixel size in bits */
> + unsigned int bpp;
> +
> + /* xfer function */
> + int (*xfer)(struct video_i2c_data *data, char *buf);
> +};
> +
> +static int amg88xx_xfer(struct video_i2c_data *data, char *buf)
> +{
> + struct i2c_client *client = data->client;
> + struct i2c_msg msg[2];
> + u8 reg = 0x80;
> + int ret;
> +
> + msg[0].addr = client->addr;
> + msg[0].flags = 0;
> + msg[0].len = 1;
> + msg[0].buf  = (char *)
> +
> + msg[1].addr = client->addr;
> + msg[1].flags = I2C_M_RD;
> + msg[1].len = 

Re: [PATCH v8 2/2] media: video-i2c: add video-i2c driver

[Matt Ranostay]

Slight poke on this since the merge window craziness should be over
also I stupidly forgot to CC people who had comments in the last
revision. *grin*

- Matt

On Fri, Apr 6, 2018 at 3:52 PM, Matt Ranostay
 wrote:
> There are several thermal sensors that only have a low-speed bus
> interface but output valid video data. This patchset enables support
> for the AMG88xx "Grid-Eye" sensor family.
>
> Signed-off-by: Matt Ranostay 
> ---
>  MAINTAINERS   |   6 +
>  drivers/media/i2c/Kconfig |  13 +
>  drivers/media/i2c/Makefile|   1 +
>  drivers/media/i2c/video-i2c.c | 563 
> ++
>  4 files changed, 583 insertions(+)
>  create mode 100644 drivers/media/i2c/video-i2c.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dc153da22e8a..928b6a862626 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14880,6 +14880,12 @@ L: linux-media@vger.kernel.org
>  S: Maintained
>  F: drivers/media/platform/video-mux.c
>
> +VIDEO I2C POLLING DRIVER
> +M: Matt Ranostay 
> +L: linux-media@vger.kernel.org
> +S: Maintained
> +F: drivers/media/i2c/video-i2c.c
> +
>  VIDEOBUF2 FRAMEWORK
>  M: Pawel Osciak 
>  M: Marek Szyprowski 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 541f0d28afd8..faaaceb94832 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -974,6 +974,19 @@ config VIDEO_M52790
>
>  To compile this driver as a module, choose M here: the
>  module will be called m52790.
> +
> +config VIDEO_I2C
> +   tristate "I2C transport video support"
> +   depends on VIDEO_V4L2 && I2C
> +   select VIDEOBUF2_VMALLOC
> +   ---help---
> + Enable the I2C transport video support which supports the
> + following:
> +  * Panasonic AMG88xx Grid-Eye Sensors
> +
> + To compile this driver as a module, choose M here: the
> + module will be called video-i2c
> +
>  endmenu
>
>  menu "Sensors used on soc_camera driver"
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index ea34aee1a85a..84cc472238ef 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -96,6 +96,7 @@ obj-$(CONFIG_VIDEO_LM3646)+= lm3646.o
>  obj-$(CONFIG_VIDEO_SMIAPP_PLL) += smiapp-pll.o
>  obj-$(CONFIG_VIDEO_AK881X) += ak881x.o
>  obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
> +obj-$(CONFIG_VIDEO_I2C)+= video-i2c.o
>  obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
>  obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
>  obj-$(CONFIG_VIDEO_TC358743)   += tc358743.o
> diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c
> new file mode 100644
> index ..ec8a597597bf
> --- /dev/null
> +++ b/drivers/media/i2c/video-i2c.c
> @@ -0,0 +1,563 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * video-i2c.c - Support for I2C transport video devices
> + *
> + * Copyright (C) 2018 Matt Ranostay 
> + *
> + * Supported:
> + * - Panasonic AMG88xx Grid-Eye Sensors
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define VIDEO_I2C_DRIVER   "video-i2c"
> +
> +struct video_i2c_chip;
> +
> +struct video_i2c_buffer {
> +   struct vb2_v4l2_buffer vb;
> +   struct list_head list;
> +};
> +
> +struct video_i2c_data {
> +   struct i2c_client *client;
> +   const struct video_i2c_chip *chip;
> +   struct mutex lock;
> +   spinlock_t slock;
> +   unsigned int sequence;
> +   struct mutex queue_lock;
> +
> +   struct v4l2_device v4l2_dev;
> +   struct video_device vdev;
> +   struct vb2_queue vb_vidq;
> +
> +   struct task_struct *kthread_vid_cap;
> +   struct list_head vid_cap_active;
> +};
> +
> +const static struct v4l2_fmtdesc amg88xx_format = {
> +   .pixelformat = V4L2_PIX_FMT_Y12,
> +};
> +
> +const static struct v4l2_frmsize_discrete amg88xx_size = {
> +   .width = 8,
> +   .height = 8,
> +};
> +
> +struct video_i2c_chip {
> +   /* video dimensions */
> +   const struct v4l2_fmtdesc *format;
> +   const struct v4l2_frmsize_discrete *size;
> +
> +   /* max frames per second */
> +   unsigned int max_fps;
> +
> +   /* pixel buffer size */
> +   unsigned int buffer_size;
> +
> +   /* pixel size in bits */
> +   unsigned int bpp;
> +
> +   /* xfer function */
> +   int (*xfer)(struct video_i2c_data *data, char *buf);
> +};
> +
> +static int amg88xx_xfer(struct video_i2c_data *data, char *buf)
> +{
> +   struct i2c_client *client = data->client;
> +   struct i2c_msg msg[2];
> +   u8 reg = 0x80;
> +