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

[Dan Williams]

On Fri, Jan 5, 2018 at 6:22 PM, Eric W. Biederman  wrote:
> Dan Williams  writes:
>
>> Quoting Mark's original RFC:
>>
>> "Recently, Google Project Zero discovered several classes of attack
>> against speculative execution. One of these, known as variant-1, allows
>> explicit bounds checks to be bypassed under speculation, providing an
>> arbitrary read gadget. Further details can be found on the GPZ blog [1]
>> and the Documentation patch in this series."
>>
>> This series incorporates Mark Rutland's latest api and adds the x86
>> specific implementation of nospec_barrier. The
>> nospec_{array_ptr,ptr,barrier} helpers are then combined with a kernel
>> wide analysis performed by Elena Reshetova to address static analysis
>> reports where speculative execution on a userspace controlled value
>> could bypass a bounds check. The patches address a precondition for the
>> attack discussed in the Spectre paper [2].
>
> Please expand this.
>
> It is not clear what the static analysis is looking for.  Have a clear
> description of what is being fixed is crucial for allowing any of these
> changes.
>
> For the details given in the change description what I read is magic
> changes because a magic process says this code is vunlerable.

Yes, that was my first reaction to the patches as well, I try below to
add some more background and guidance, but in the end these are static
analysis reports across a wide swath of sub-systems. It's going to
take some iteration with domain experts to improve the patch
descriptions, and that's the point of this series, to get the better
trained eyes from the actual sub-system owners to take a look at these
reports.

For example, I'm looking for feedback like what Srinivas gave where he
identified that the report is bogus, the branch condition can not be
seeded with bad values in that path. Be like Srinivas.

> Given the similarities in the code that is being patched to many other
> places in the kernel it is not at all clear that this small set of
> changes is sufficient for any purpose.

I find this assertion absurd, when in the past have we as kernel
developers ever been handed a static analysis report and then
questioned why the static analysis did not flag other call sites
before first reviewing the ones it did find?

>> A consideration worth noting for reviewing these patches is to weigh the
>> dramatic cost of being wrong about whether a given report is exploitable
>> vs the overhead nospec_{array_ptr,ptr} may introduce. In other words,
>> lets make the bar for applying these patches be "can you prove that the
>> bounds check bypass is *not* exploitable". Consider that the Spectre
>> paper reports one example of a speculation window being ~180 cycles.
>
>
>> Note that there is also a proposal from Linus, array_access [3], that
>> attempts to quash speculative execution past a bounds check without
>> introducing an lfence instruction. That may be a future optimization
>> possibility that is compatible with this api, but it would appear to
>> need guarantees from the compiler that it is not clear the kernel can
>> rely on at this point. It is also not clear that it would be a
>> significant performance win vs lfence.
>
> It is also not clear that these changes fix anything, or are in any
> sense correct for the problem they are trying to fix as the problem
> is not clearly described.

I'll try my best. I don't have first hand knowledge of how the static
analyzer is doing this job, and I don't think it matters for
evaluating these reports. I'll give you my thoughts on how I would
handle one of these reports if it flagged one of the sub-systems I
maintain.

Start with the example from the Spectre paper:

if (x < array1_size)
y = array2[array1[x] * 256];

In all the patches 'x' and 'array1' are called out explicitly. For example:

net: mpls: prevent bounds-check bypass via speculative execution

Static analysis reports that 'index' may be a user controlled value that
is used as a data dependency reading 'rt' from the 'platform_label'
array...

So the first thing to review is whether the analyzer got it wrong and
'x' is not arbitrarily controllable by userspace to cause speculation
outside of the checked bounds. Be like Srinivas. The next step is to
ask whether the code can be refactored so that 'x' is sanitized
earlier in the call stack, especially if the nospec_array_ptr() lands
in a hot path. The next aspect that I expect most would be tempted to
go check is whether 'array2[array1[x]]' occurs later in the code
stream, but with speculation windows being architecture dependent and
potentially large (~180 cycles in one case says the paper) I submit
that we should err on the side of caution and not guess if that second
dependent read has been emitted somewhere in the instruction stream.

> In at least one place (mpls) you are patching a fast path.  Compile out
> or don't load mpls by all means.  But it is not 

cron job: media_tree daily build: ERRORS

[Hans Verkuil]

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

Results of the daily build of media_tree:

date:   Sat Jan  6 05:00:17 CET 2018
media-tree git hash:e3ee691dbf24096ea51b3200946b11d68ce75361
media_build git hash:   46c9dc0a08499791cedfc7ee0df387e475f075a2
v4l-utils git hash: 8aa401d119afaeb1b4fe4d2994789cd3e9396554
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3911-g6f737e1f
smatch version: v0.5.0-3911-g6f737e1f
host hardware:  x86_64
host os:4.13.0-164

linux-git-arm-at91: ERRORS
linux-git-arm-davinci: OK
linux-git-arm-multi: ERRORS
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-blackfin-bf561: ERRORS
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: ERRORS
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9.26-i686: ERRORS
linux-4.10.14-i686: WARNINGS
linux-4.11-i686: WARNINGS
linux-4.12.1-i686: WARNINGS
linux-4.13-i686: WARNINGS
linux-4.14-i686: WARNINGS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.26-x86_64: ERRORS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: WARNINGS
linux-4.14-x86_64: WARNINGS
apps: OK
spec-git: OK
smatch: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

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

[Eric W. Biederman]

Dan Williams  writes:

> Quoting Mark's original RFC:
>
> "Recently, Google Project Zero discovered several classes of attack
> against speculative execution. One of these, known as variant-1, allows
> explicit bounds checks to be bypassed under speculation, providing an
> arbitrary read gadget. Further details can be found on the GPZ blog [1]
> and the Documentation patch in this series."
>
> This series incorporates Mark Rutland's latest api and adds the x86
> specific implementation of nospec_barrier. The
> nospec_{array_ptr,ptr,barrier} helpers are then combined with a kernel
> wide analysis performed by Elena Reshetova to address static analysis
> reports where speculative execution on a userspace controlled value
> could bypass a bounds check. The patches address a precondition for the
> attack discussed in the Spectre paper [2].

Please expand this.

It is not clear what the static analysis is looking for.  Have a clear
description of what is being fixed is crucial for allowing any of these
changes.

For the details given in the change description what I read is magic
changes because a magic process says this code is vunlerable.

Given the similarities in the code that is being patched to many other
places in the kernel it is not at all clear that this small set of
changes is sufficient for any purpose.

> A consideration worth noting for reviewing these patches is to weigh the
> dramatic cost of being wrong about whether a given report is exploitable
> vs the overhead nospec_{array_ptr,ptr} may introduce. In other words,
> lets make the bar for applying these patches be "can you prove that the
> bounds check bypass is *not* exploitable". Consider that the Spectre
> paper reports one example of a speculation window being ~180 cycles.


> Note that there is also a proposal from Linus, array_access [3], that
> attempts to quash speculative execution past a bounds check without
> introducing an lfence instruction. That may be a future optimization
> possibility that is compatible with this api, but it would appear to
> need guarantees from the compiler that it is not clear the kernel can
> rely on at this point. It is also not clear that it would be a
> significant performance win vs lfence.

It is also not clear that these changes fix anything, or are in any
sense correct for the problem they are trying to fix as the problem
is not clearly described.

In at least one place (mpls) you are patching a fast path.  Compile out
or don't load mpls by all means.  But it is not acceptable to change the
fast path without even considering performance.

So because the description sucks, and the due diligence is not there.

Nacked-by: "Eric W. Biederman" 

to the series.


>
> These patches also will also be available via the 'nospec' git branch
> here:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/djbw/linux nospec
>
> [1]: 
> https://googleprojectzero.blogspot.co.uk/2018/01/reading-privileged-memory-with-side.html
> [2]: https://spectreattack.com/spectre.pdf
> [3]: https://marc.info/?l=linux-kernel=151510446027625=2
>
> ---
>
> Andi Kleen (1):
>   x86, barrier: stop speculation for failed access_ok
>
> Dan Williams (13):
>   x86: implement nospec_barrier()
>   [media] uvcvideo: prevent bounds-check bypass via speculative execution
>   carl9170: prevent bounds-check bypass via speculative execution
>   p54: prevent bounds-check bypass via speculative execution
>   qla2xxx: prevent bounds-check bypass via speculative execution
>   cw1200: prevent bounds-check bypass via speculative execution
>   Thermal/int340x: prevent bounds-check bypass via speculative execution
>   ipv6: prevent bounds-check bypass via speculative execution
>   ipv4: prevent bounds-check bypass via speculative execution
>   vfs, fdtable: prevent bounds-check bypass via speculative execution
>   net: mpls: prevent bounds-check bypass via speculative execution
>   udf: prevent bounds-check bypass via speculative execution
>   userns: prevent bounds-check bypass via speculative execution
>
> Mark Rutland (4):
>   asm-generic/barrier: add generic nospec helpers
>   Documentation: document nospec helpers
>   arm64: implement nospec_ptr()
>   arm: implement nospec_ptr()
>
>  Documentation/speculation.txt  |  166 
> 
>  arch/arm/include/asm/barrier.h |   75 +
>  arch/arm64/include/asm/barrier.h   |   55 +++
>  arch/x86/include/asm/barrier.h |6 +
>  arch/x86/include/asm/uaccess.h |   17 ++
>  drivers/media/usb/uvc/uvc_v4l2.c   |7 +
>  drivers/net/wireless/ath/carl9170/main.c   |6 -
>  drivers/net/wireless/intersil/p54/main.c   |8 +
>  drivers/net/wireless/st/cw1200/sta.c   |   10 +
>  drivers/net/wireless/st/cw1200/wsm.h   |

[PATCH 07/18] [media] uvcvideo: prevent bounds-check bypass via speculative execution

[Dan Williams]

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

Based on an original patch by Elena Reshetova.

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

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

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

[Dan Williams]

Quoting Mark's original RFC:

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

This series incorporates Mark Rutland's latest api and adds the x86
specific implementation of nospec_barrier. The
nospec_{array_ptr,ptr,barrier} helpers are then combined with a kernel
wide analysis performed by Elena Reshetova to address static analysis
reports where speculative execution on a userspace controlled value
could bypass a bounds check. The patches address a precondition for the
attack discussed in the Spectre paper [2].

A consideration worth noting for reviewing these patches is to weigh the
dramatic cost of being wrong about whether a given report is exploitable
vs the overhead nospec_{array_ptr,ptr} may introduce. In other words,
lets make the bar for applying these patches be "can you prove that the
bounds check bypass is *not* exploitable". Consider that the Spectre
paper reports one example of a speculation window being ~180 cycles.

Note that there is also a proposal from Linus, array_access [3], that
attempts to quash speculative execution past a bounds check without
introducing an lfence instruction. That may be a future optimization
possibility that is compatible with this api, but it would appear to
need guarantees from the compiler that it is not clear the kernel can
rely on at this point. It is also not clear that it would be a
significant performance win vs lfence.

These patches also will also be available via the 'nospec' git branch
here:

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

[1]: 
https://googleprojectzero.blogspot.co.uk/2018/01/reading-privileged-memory-with-side.html
[2]: https://spectreattack.com/spectre.pdf
[3]: https://marc.info/?l=linux-kernel=151510446027625=2

---

Andi Kleen (1):
  x86, barrier: stop speculation for failed access_ok

Dan Williams (13):
  x86: implement nospec_barrier()
  [media] uvcvideo: prevent bounds-check bypass via speculative execution
  carl9170: prevent bounds-check bypass via speculative execution
  p54: prevent bounds-check bypass via speculative execution
  qla2xxx: prevent bounds-check bypass via speculative execution
  cw1200: prevent bounds-check bypass via speculative execution
  Thermal/int340x: prevent bounds-check bypass via speculative execution
  ipv6: prevent bounds-check bypass via speculative execution
  ipv4: prevent bounds-check bypass via speculative execution
  vfs, fdtable: prevent bounds-check bypass via speculative execution
  net: mpls: prevent bounds-check bypass via speculative execution
  udf: prevent bounds-check bypass via speculative execution
  userns: prevent bounds-check bypass via speculative execution

Mark Rutland (4):
  asm-generic/barrier: add generic nospec helpers
  Documentation: document nospec helpers
  arm64: implement nospec_ptr()
  arm: implement nospec_ptr()

 Documentation/speculation.txt  |  166 
 arch/arm/include/asm/barrier.h |   75 +
 arch/arm64/include/asm/barrier.h   |   55 +++
 arch/x86/include/asm/barrier.h |6 +
 arch/x86/include/asm/uaccess.h |   17 ++
 drivers/media/usb/uvc/uvc_v4l2.c   |7 +
 drivers/net/wireless/ath/carl9170/main.c   |6 -
 drivers/net/wireless/intersil/p54/main.c   |8 +
 drivers/net/wireless/st/cw1200/sta.c   |   10 +
 drivers/net/wireless/st/cw1200/wsm.h   |4 
 drivers/scsi/qla2xxx/qla_mr.c  |   15 +-
 .../thermal/int340x_thermal/int340x_thermal_zone.c |   14 +-
 fs/udf/misc.c  |   39 +++--
 include/asm-generic/barrier.h  |   68 
 include/linux/fdtable.h|5 -
 kernel/user_namespace.c|   10 -
 net/ipv4/raw.c |9 +
 net/ipv6/raw.c |9 +
 net/mpls/af_mpls.c |   12 +
 19 files changed, 466 insertions(+), 69 deletions(-)
 create mode 100644 Documentation/speculation.txt

[PATCH 4/4] cx23885: Add support for new Hauppauge QuadHD (885)

[Brad Love]

Add new QuadHD digital only PCIe boards to driver list.
Differentiate them from 888 digital/analog QuadHD models.

Signed-off-by: Brad Love 
---
 drivers/media/pci/cx23885/cx23885-cards.c | 42 ++-
 drivers/media/pci/cx23885/cx23885-core.c  |  8 ++
 drivers/media/pci/cx23885/cx23885-dvb.c   |  6 +
 drivers/media/pci/cx23885/cx23885.h   |  2 ++
 4 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c 
b/drivers/media/pci/cx23885/cx23885-cards.c
index 70f1047..a0d8858 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -771,11 +771,21 @@ struct cx23885_board cx23885_boards[] = {
.portb= CX23885_MPEG_DVB,
.portc= CX23885_MPEG_DVB,
},
+   [CX23885_BOARD_HAUPPAUGE_QUADHD_DVB_885] = {
+   .name   = "Hauppauge WinTV-QuadHD-DVB(885)",
+   .portb= CX23885_MPEG_DVB,
+   .portc= CX23885_MPEG_DVB,
+   },
[CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC] = {
.name= "Hauppauge WinTV-QuadHD-ATSC",
.portb= CX23885_MPEG_DVB,
.portc= CX23885_MPEG_DVB,
},
+   [CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC_885] = {
+   .name   = "Hauppauge WinTV-QuadHD-ATSC(885)",
+   .portb= CX23885_MPEG_DVB,
+   .portc= CX23885_MPEG_DVB,
+   },
[CX23885_BOARD_HAUPPAUGE_HVR1265_K4] = {
.name   = "Hauppauge WinTV-HVR-1265(16)",
.portc  = CX23885_MPEG_DVB,
@@ -1311,25 +1321,25 @@ static void hauppauge_eeprom(struct cx23885_dev *dev, 
u8 *eeprom_data)
case 16:
/* WinTV-HVR-1265 (PCIe, Analog/ATSC/QAM-B) */
break;
-   case 166100:
+   case 166100: /* 888 version, hybrid */
+   case 166200: /* 885 version, DVB only */
/* WinTV-QuadHD (DVB) Tuner Pair 1 (PCIe, IR, half height,
   DVB-T/T2/C, DVB-T/T2/C */
break;
-   case 166101:
+   case 166101: /* 888 version, hybrid */
+   case 166201: /* 885 version, DVB only */
/* WinTV-QuadHD (DVB) Tuner Pair 2 (PCIe, IR, half height,
   DVB-T/T2/C, DVB-T/T2/C */
break;
-   case 165100:
-   /*
-* WinTV-QuadHD (ATSC) Tuner Pair 1 (PCIe, IR, half height,
-* ATSC, ATSC
-*/
+   case 165100: /* 888 version, hybrid */
+   case 165200: /* 885 version, digital only */
+   /* WinTV-QuadHD (ATSC) Tuner Pair 1 (PCIe, IR, half height,
+* ATSC/QAM-B, ATSC/QAM-B */
break;
-   case 165101:
-   /*
-* WinTV-QuadHD (DVB) Tuner Pair 2 (PCIe, IR, half height,
-* ATSC, ATSC
-*/
+   case 165101: /* 888 version, hybrid */
+   case 165201: /* 885 version, digital only */
+   /* WinTV-QuadHD (ATSC) Tuner Pair 2 (PCIe, IR, half height,
+* ATSC/QAM-B, ATSC/QAM-B */
break;
default:
pr_warn("%s: warning: unknown hauppauge model #%d\n",
@@ -1835,6 +1845,8 @@ void cx23885_gpio_setup(struct cx23885_dev *dev)
 */
break;
case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+   case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB_885:
+   case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC_885:
/*
 * GPIO-08 TER1_RESN
 * GPIO-09 TER2_RESN
@@ -2094,7 +2106,9 @@ void cx23885_card_setup(struct cx23885_dev *dev)
case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
case CX23885_BOARD_HAUPPAUGE_STARBURST2:
case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+   case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB_885:
case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
+   case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC_885:
if (dev->i2c_bus[0].i2c_rc == 0)
hauppauge_eeprom(dev, eeprom+0xc0);
break;
@@ -2243,7 +2257,9 @@ void cx23885_card_setup(struct cx23885_dev *dev)
break;
case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
+   case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB_885:
case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
+   case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC_885:
ts1->gen_ctrl_val  = 0xc; /* Serial bus + punctured clock */
ts1->ts_clk_en_val = 0x1; /* Enable TS_CLK */
ts1->src_sel_val   = CX23885_SRC_SEL_PARALLEL_MPEG_VIDEO;
@@ -2301,6 +2317,8 @@ void cx23885_card_setup(struct cx23885_dev *dev)
case CX23885_BOARD_HAUPPAUGE_HVR1255:
case CX23885_BOARD_HAUPPAUGE_HVR1255_22111:
case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+   

[PATCH 1/4] cx23885: Enable new Hauppauge PCIe ImpactVCBe variant

[Brad Love]

Add ID of new card revision to driver list

Analog PAL/NTSC capture.

Signed-off-by: Brad Love 
---
 drivers/media/pci/cx23885/cx23885-cards.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c 
b/drivers/media/pci/cx23885/cx23885-cards.c
index 3622521..c4b3123 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -1028,6 +1028,10 @@ struct cx23885_subid cx23885_subids[] = {
.subdevice = 0x7133,
.card  = CX23885_BOARD_HAUPPAUGE_IMPACTVCBE,
}, {
+   .subvendor = 0x0070,
+   .subdevice = 0x7137,
+   .card  = CX23885_BOARD_HAUPPAUGE_IMPACTVCBE,
+   }, {
.subvendor = 0x18ac,
.subdevice = 0xdb98,
.card  = CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2,
-- 
2.7.4

[PATCH 2/4] cx23885: Add support for Hauppauge PCIe HVR1265 K4

[Brad Love]

Add new PCIe board to driver list and board register/configure functions

cx23885 + lgdt3306a + si2157 digital/analog

Signed-off-by: Brad Love 
---
 drivers/media/pci/cx23885/cx23885-cards.c | 27 ++
 drivers/media/pci/cx23885/cx23885-dvb.c   | 46 +++
 drivers/media/pci/cx23885/cx23885-input.c |  3 ++
 drivers/media/pci/cx23885/cx23885-video.c |  5 +++-
 drivers/media/pci/cx23885/cx23885.h   |  1 +
 5 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c 
b/drivers/media/pci/cx23885/cx23885-cards.c
index c4b3123..fe42893 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -776,6 +776,11 @@ struct cx23885_board cx23885_boards[] = {
.portb= CX23885_MPEG_DVB,
.portc= CX23885_MPEG_DVB,
},
+   [CX23885_BOARD_HAUPPAUGE_HVR1265_K4] = {
+   .name   = "Hauppauge WinTV-HVR-1265(16)",
+   .portc  = CX23885_MPEG_DVB,
+   .force_bff  = 1,
+   },
 };
 const unsigned int cx23885_bcount = ARRAY_SIZE(cx23885_boards);
 
@@ -1091,6 +1096,10 @@ struct cx23885_subid cx23885_subids[] = {
.subvendor = 0x0070,
.subdevice = 0x6b18,
.card  = CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC, /* Tuner Pair 
2 */
+   }, {
+   .subvendor = 0x0070,
+   .subdevice = 0x2a18,
+   .card  = CX23885_BOARD_HAUPPAUGE_HVR1265_K4, /* Hauppauge 
WinTV HVR-1265 (Model 161xx1, Hybrid ATSC/QAM-B) */
},
 };
 const unsigned int cx23885_idcount = ARRAY_SIZE(cx23885_subids);
@@ -1291,6 +1300,9 @@ static void hauppauge_eeprom(struct cx23885_dev *dev, u8 
*eeprom_data)
case 150329:
/* WinTV-HVR5525 (PCIe, DVB-S/S2, DVB-T/T2/C) */
break;
+   case 16:
+   /* WinTV-HVR-1265 K4 (PCIe, Analog/ATSC/QAM-B) */
+   break;
case 166100:
/* WinTV-QuadHD (DVB) Tuner Pair 1 (PCIe, IR, half height,
   DVB-T/T2/C, DVB-T/T2/C */
@@ -1813,6 +1825,18 @@ void cx23885_gpio_setup(struct cx23885_dev *dev)
 * card does not have any GPIO's connected to subcomponents.
 */
break;
+   case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+   /*
+* GPIO-08 TER1_RESN
+* GPIO-09 TER2_RESN
+*/
+   /* Put the parts into reset and back */
+   cx23885_gpio_enable(dev, GPIO_8 | GPIO_9, 1);
+   cx23885_gpio_clear(dev, GPIO_8 | GPIO_9);
+   msleep(100);
+   cx23885_gpio_set(dev, GPIO_8 | GPIO_9);
+   msleep(100);
+   break;
}
 }
 
@@ -2058,6 +2082,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
case CX23885_BOARD_HAUPPAUGE_STARBURST:
case CX23885_BOARD_HAUPPAUGE_IMPACTVCBE:
case CX23885_BOARD_HAUPPAUGE_HVR5525:
+   case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
if (dev->i2c_bus[0].i2c_rc == 0)
@@ -2205,6 +2230,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
ts2->ts_clk_en_val = 0x1; /* Enable TS_CLK */
ts2->src_sel_val   = CX23885_SRC_SEL_PARALLEL_MPEG_VIDEO;
break;
+   case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
ts1->gen_ctrl_val  = 0xc; /* Serial bus + punctured clock */
@@ -2263,6 +2289,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
case CX23885_BOARD_COMPRO_VIDEOMATE_E800:
case CX23885_BOARD_HAUPPAUGE_HVR1255:
case CX23885_BOARD_HAUPPAUGE_HVR1255_22111:
+   case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
case CX23885_BOARD_HAUPPAUGE_HVR1270:
case CX23885_BOARD_HAUPPAUGE_HVR1850:
case CX23885_BOARD_MYGICA_X8506:
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c 
b/drivers/media/pci/cx23885/cx23885-dvb.c
index 700422b..2b23636 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -930,6 +930,18 @@ static const struct m88ds3103_config 
hauppauge_hvr5525_m88ds3103_config = {
.agc = 0x99,
 };
 
+static struct lgdt3306a_config hauppauge_hvr1265k4_config = {
+   .i2c_addr   = 0x59,
+   .qam_if_khz = 4000,
+   .vsb_if_khz = 3250,
+   .deny_i2c_rptr  = 1, /* Disabled */
+   .spectral_inversion = 0, /* Disabled */
+   .mpeg_mode  = LGDT3306A_MPEG_SERIAL,
+   .tpclk_edge = LGDT3306A_TPCLK_RISING_EDGE,
+   .tpvalid_polarity   = LGDT3306A_TP_VALID_HIGH,
+   .xtalMHz= 25, /* 24 or 

[PATCH 3/4] cx23885: Add support for Hauppauge PCIe Starburst2

[Brad Love]

Add new PCIe DVB-S/S2.
A single port Hauppauge HVR-5525

cx23885 + a8293 + m88rs6000t

Signed-off-by: Brad Love 
---
 drivers/media/pci/cx23885/cx23885-cards.c | 11 +++
 drivers/media/pci/cx23885/cx23885-dvb.c   | 18 ++
 drivers/media/pci/cx23885/cx23885.h   |  1 +
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c 
b/drivers/media/pci/cx23885/cx23885-cards.c
index fe42893..70f1047 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -781,6 +781,10 @@ struct cx23885_board cx23885_boards[] = {
.portc  = CX23885_MPEG_DVB,
.force_bff  = 1,
},
+   [CX23885_BOARD_HAUPPAUGE_STARBURST2] = {
+   .name   = "Hauppauge WinTV-Starburst2",
+   .portb  = CX23885_MPEG_DVB,
+   },
 };
 const unsigned int cx23885_bcount = ARRAY_SIZE(cx23885_boards);
 
@@ -1100,6 +1104,10 @@ struct cx23885_subid cx23885_subids[] = {
.subvendor = 0x0070,
.subdevice = 0x2a18,
.card  = CX23885_BOARD_HAUPPAUGE_HVR1265_K4, /* Hauppauge 
WinTV HVR-1265 (Model 161xx1, Hybrid ATSC/QAM-B) */
+   }, {
+   .subvendor = 0x0070,
+   .subdevice = 0xf02a,
+   .card  = CX23885_BOARD_HAUPPAUGE_STARBURST2,
},
 };
 const unsigned int cx23885_idcount = ARRAY_SIZE(cx23885_subids);
@@ -1796,6 +1804,7 @@ void cx23885_gpio_setup(struct cx23885_dev *dev)
case CX23885_BOARD_HAUPPAUGE_HVR5525:
case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
+   case CX23885_BOARD_HAUPPAUGE_STARBURST2:
/*
 * HVR5525 GPIO Details:
 *  GPIO-00 IR_WIDE
@@ -2083,6 +2092,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
case CX23885_BOARD_HAUPPAUGE_IMPACTVCBE:
case CX23885_BOARD_HAUPPAUGE_HVR5525:
case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+   case CX23885_BOARD_HAUPPAUGE_STARBURST2:
case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
if (dev->i2c_bus[0].i2c_rc == 0)
@@ -2223,6 +2233,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
ts2->src_sel_val   = CX23885_SRC_SEL_PARALLEL_MPEG_VIDEO;
break;
case CX23885_BOARD_HAUPPAUGE_HVR5525:
+   case CX23885_BOARD_HAUPPAUGE_STARBURST2:
ts1->gen_ctrl_val  = 0x5; /* Parallel */
ts1->ts_clk_en_val = 0x1; /* Enable TS_CLK */
ts1->src_sel_val   = CX23885_SRC_SEL_PARALLEL_MPEG_VIDEO;
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c 
b/drivers/media/pci/cx23885/cx23885-dvb.c
index 2b23636..260d843 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -1206,6 +1206,8 @@ static int dvb_register(struct cx23885_tsport *port)
struct si2157_config si2157_config;
struct ts2020_config ts2020_config;
struct m88ds3103_platform_data m88ds3103_pdata;
+   struct m88rs6000t_config m88rs6000t_config = {};
+   struct a8293_platform_data a8293_pdata = {};
struct i2c_board_info info;
struct i2c_adapter *adapter;
struct i2c_client *client_demod = NULL, *client_tuner = NULL;
@@ -2229,9 +2231,10 @@ static int dvb_register(struct cx23885_tsport *port)
}
port->i2c_client_tuner = client_tuner;
break;
-   case CX23885_BOARD_HAUPPAUGE_HVR5525: {
-   struct m88rs6000t_config m88rs6000t_config;
-   struct a8293_platform_data a8293_pdata = {};
+   case CX23885_BOARD_HAUPPAUGE_STARBURST2:
+   case CX23885_BOARD_HAUPPAUGE_HVR5525:
+   i2c_bus = >i2c_bus[0];
+   i2c_bus2 = >i2c_bus[1];
 
switch (port->nr) {
 
@@ -2240,7 +2243,7 @@ static int dvb_register(struct cx23885_tsport *port)
/* attach frontend */
fe0->dvb.frontend = dvb_attach(m88ds3103_attach,
_hvr5525_m88ds3103_config,
-   >i2c_bus[0].i2c_adap, );
+   _bus->i2c_adap, );
if (fe0->dvb.frontend == NULL)
break;
 
@@ -2251,7 +2254,7 @@ static int dvb_register(struct cx23885_tsport *port)
info.addr = 0x0b;
info.platform_data = _pdata;
request_module("a8293");
-   client_sec = i2c_new_device(>i2c_bus[0].i2c_adap, 
);
+   client_sec = i2c_new_device(_bus->i2c_adap, );
if (!client_sec || !client_sec->dev.driver)
goto frontend_detach;
if 

[PATCH 0/4] New Hauppauge PCIe devices

[Brad Love]

Included is support for the following four PCIe devices:
- Hauppauge ImpactVCBe - analog PAL/NTSC
- Hauppauge HVR1264K4 - ATSC/QAM + analog
- Hauppauge Starburst2 - DVB-C/S/T/T2 + analog
- Hauppauge digital only QuadHD models - ATSC/QAM|DVB-C/T/T2

Brad Love (4):
  cx23885: Enable new Hauppauge PCIe ImpactVCBe variant
  cx23885: Add support for Hauppauge PCIe HVR1265 K4
  cx23885: Add support for Hauppauge PCIe Starburst2
  cx23885: Add support for new Hauppauge QuadHD (885)

 drivers/media/pci/cx23885/cx23885-cards.c | 86 ++-
 drivers/media/pci/cx23885/cx23885-core.c  |  8 +++
 drivers/media/pci/cx23885/cx23885-dvb.c   | 70 ++---
 drivers/media/pci/cx23885/cx23885-input.c |  3 ++
 drivers/media/pci/cx23885/cx23885-video.c |  5 +-
 drivers/media/pci/cx23885/cx23885.h   |  4 ++
 6 files changed, 155 insertions(+), 21 deletions(-)

-- 
2.7.4

Re: [PATCH] media: ov9650: support device tree probing

[jacopo mondi]

Hello Akinobu,

On Sat, Jan 06, 2018 at 02:49:03AM +0900, Akinobu Mita wrote:
> The ov9650 driver currently only supports legacy platform data probe.
> This change adds device tree probing.
>
> There has been an attempt to add device tree support for ov9650 driver
> by Hugues Fruchet as a part of the patchset that adds support of OV9655
> camera (http://www.spinics.net/lists/linux-media/msg117903.html), but
> it wasn't merged into mainline because creating a separate driver for
> OV9655 is preferred.
>
> This is very similar to Hugues's patch, but not supporting new device.
>
> Cc: H. Nikolaus Schaller 
> Cc: Hugues Fruchet 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Cc: Rob Herring 
> Signed-off-by: Akinobu Mita 
> ---
>  .../devicetree/bindings/media/i2c/ov9650.txt   |  35 +++
>  drivers/media/i2c/ov9650.c | 107 
> -

I'm not sure if separating bindings patches from implementation is
mandatory only for newly introduced bindings or this holds true also for
changes to existing bindings.

I think it's anyway good to separate them, and please CC devicetree ml


>  2 files changed, 117 insertions(+), 25 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov9650.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
> new file mode 100644
> index 000..aa5024d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
> @@ -0,0 +1,35 @@
> +* Omnivision OV9650/OV9652 CMOS sensor
> +
> +Required Properties:
> +- compatible: should be one of

s/should/shall

> + "ovti,ov9650"
> + "ovti,ov9652"
> +- clocks: reference to the xvclk input clock.
> +
> +Optional Properties:
> +- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
> +  Active is high.
> +- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
> +  Active is high.
> +
> +The device node must contain one 'port' child node for its digital output
> +video port, in accordance with the video interface bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +Example:
> +
> + {
> + ov9650: camera@30 {
> + compatible = "ovti,ov9650";
> + reg = <0x30>;
> + reset-gpios = <_gpio_0 0 GPIO_ACTIVE_HIGH>;
> + powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_HIGH>;
> + clocks = <>;
> +
> + port {
> + ov9650_0: endpoint {
> + remote-endpoint = <_in0>;
> + };
> + };
> + };
> +};
> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
> index 69433e1..1affdc0 100644
> --- a/drivers/media/i2c/ov9650.c
> +++ b/drivers/media/i2c/ov9650.c
> @@ -11,8 +11,10 @@
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
>   */
> +#include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -249,9 +251,10 @@ struct ov965x {
>   struct v4l2_subdev sd;
>   struct media_pad pad;
>   enum v4l2_mbus_type bus_type;
> - int gpios[NUM_GPIOS];
> + struct gpio_desc *gpios[NUM_GPIOS];
>   /* External master clock frequency */
>   unsigned long mclk_frequency;
> + struct clk *clk;
>
>   /* Protects the struct fields below */
>   struct mutex lock;
> @@ -513,10 +516,9 @@ static int ov965x_set_color_matrix(struct ov965x *ov965x)
>   return 0;
>  }
>
> -static void ov965x_gpio_set(int gpio, int val)
> +static void ov965x_gpio_set(struct gpio_desc *gpio, int val)
>  {
> - if (gpio_is_valid(gpio))
> - gpio_set_value(gpio, val);
> + gpiod_set_value_cansleep(gpio, val);
>  }

I see this function being used in very few places, can you replace it
with a call to 'gpiod_set_value_cansleep()' as that's the only thing
it actually does?

>
>  static void __ov965x_set_power(struct ov965x *ov965x, int on)
> @@ -1408,16 +1410,17 @@ static const struct v4l2_subdev_ops ov965x_subdev_ops 
> = {
>  /*
>   * Reset and power down GPIOs configuration
>   */
> -static int ov965x_configure_gpios(struct ov965x *ov965x,
> -   const struct ov9650_platform_data *pdata)
> +static int ov965x_configure_gpios_pdata(struct ov965x *ov965x,
> + const struct ov9650_platform_data *pdata)
>  {
>   int ret, i;
> + int gpios[NUM_GPIOS];
>
> - ov965x->gpios[GPIO_PWDN] = pdata->gpio_pwdn;
> - ov965x->gpios[GPIO_RST]  = pdata->gpio_reset;
> + gpios[GPIO_PWDN] = pdata->gpio_pwdn;
> + gpios[GPIO_RST]  = pdata->gpio_reset;
>
>   for (i = 0; i < ARRAY_SIZE(ov965x->gpios); i++) {
> - int gpio = ov965x->gpios[i];
> +  

Re: [PATCH v5 3/5] media: dt-bindings: ov5640: refine CSI-2 and add parallel interface

[Rob Herring]

On Wed, Jan 03, 2018 at 10:57:30AM +0100, Hugues Fruchet wrote:
> Refine CSI-2 endpoint documentation and add bindings
> for DVP parallel interface support.
> 
> Signed-off-by: Hugues Fruchet 
> ---
>  .../devicetree/bindings/media/i2c/ov5640.txt   | 46 
> +-
>  1 file changed, 44 insertions(+), 2 deletions(-)

Reviewed-by: Rob Herring  

Re: [PATCH] media: ov9650: support device tree probing

[Rob Herring]

On Fri, Jan 5, 2018 at 11:49 AM, Akinobu Mita  wrote:
> The ov9650 driver currently only supports legacy platform data probe.
> This change adds device tree probing.
>
> There has been an attempt to add device tree support for ov9650 driver
> by Hugues Fruchet as a part of the patchset that adds support of OV9655
> camera (http://www.spinics.net/lists/linux-media/msg117903.html), but
> it wasn't merged into mainline because creating a separate driver for
> OV9655 is preferred.
>
> This is very similar to Hugues's patch, but not supporting new device.
>
> Cc: H. Nikolaus Schaller 
> Cc: Hugues Fruchet 
> Cc: Sakari Ailus 
> Cc: Mauro Carvalho Chehab 
> Cc: Rob Herring 
> Signed-off-by: Akinobu Mita 
> ---
>  .../devicetree/bindings/media/i2c/ov9650.txt   |  35 +++

Please split bindings and send to the DT list.

>  drivers/media/i2c/ov9650.c | 107 
> -
>  2 files changed, 117 insertions(+), 25 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt

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

[jacopo mondi]

Hi Laurent,

On Fri, Jan 05, 2018 at 12:28:03AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Thursday, 4 January 2018 18:03:09 EET Jacopo Mondi wrote:
> > Add bindings documentation for Renesas Capture Engine Unit (CEU).
> >
> > Signed-off-by: Jacopo Mondi 
> > ---
> >  .../devicetree/bindings/media/renesas,ceu.txt  | 85 +++
> >  1 file changed, 85 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/renesas,ceu.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/renesas,ceu.txt
> > b/Documentation/devicetree/bindings/media/renesas,ceu.txt new file mode
> > 100644
> > index 000..a4f3c05
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
> > @@ -0,0 +1,85 @@
> > +Renesas Capture Engine Unit (CEU)
> > +--
> > +
> > +The Capture Engine Unit is the image capture interface found in the Renesas
> > +SH Mobile and RZ SoCs.
> > +
> > +The interface supports a single parallel input with data bus width of 8 or
> > 16
> > +bits.
> > +
> > +Required properties:
> > +- compatible: Must be one of the following.
> > +   - "renesas,r7s72100-ceu" for CEU units found in R7S72100 (RZ/A1) SoCs.
> > +   - "renesas,ceu" as a generic fallback.
>
> As asked in my review of patch 3/9, what's your policy for compatible strings
> ? As far as I understand there's no generic CEU, as the SH4 and RZ versions
> are not compatible. Should the "renesas,ceu" compatible string then be
> replaced by "renesas,rz-ceu" ?

No, there's actually no generic CEU, as it differs from RZ and SH4 and
it differs even more between RZ/A1-[H|M] and, say, RZ/A1-L (not sure about
RZ/G and other variants mentioned by Geert's reply to your comment
though). In facts, between the RZ/A1-H and SH4 one I (you actually)
have found a single register difference, while between A1-H and A1-L
there are differences in the supported capture modes (RZ/A1-L supports
'data enable fetch mode' while A1-H does not).

Can we drop the generic fallback completely, and do as we've done for
the RZ pin controller, where we use the part number for all compatible
string?

So that:
r7s72100-ceu would be RZ/A1-[H|M]
r7s72102-ceu would be RZ/A1-L
r7s72103-ceu would be RZ/A1-LU

>
> > +   SH Mobile CPUs do not currently support OF, and they configure their
> > +   CEU interfaces using platform data. The "compatible" list will be
> > +   expanded once SH Mobile will be made OF-capable.
>
> The first sentence is out of scope, or at least its second half. I'd drop this
> completely, or possibly capture it in the commit message.
>

I was under the impression you asked for this, I'll drop it

> > +- reg: Registers address base and size.
> > +- interrupts: The interrupt specifier.
> > +
> > +The CEU supports a single parallel input and should contain a single 'port'
> > +subnode with a single 'endpoint'. Connection to input devices are modeled
> > +according to the video interfaces OF bindings specified in:
> > +Documentation/devicetree/bindings/media/video-interfaces.txt
> > +
> > +Optional endpoint properties applicable to parallel input bus described in
> > +the above mentioned "video-interfaces.txt" file are supported by this
> > driver:
>
> I know it's hard to get rid of that habit, but drivers are out of scope for DT
> bindings so they should not be mentioned. You should of course document the
> properties, and explain whether they are mandatory and optional.
>

That's out of scope from this series, but what's the rationale behind
this? I mean, the CEU hw can do -a lot- of things this driver does not
support, what I'm describing here is what the driver controlling this
hw supports...

Anyway, I'll 's/by this driver//'

> > +
> > +- hsync-active: Active state of the HSYNC signal, 0/1 for LOW/HIGH
> > respectively.
> > +- vsync-active: Active state of the VSYNC signal, 0/1 for LOW/HIGH
> > respectively.
> > +
> > +Example:
> > +
> > +The example describes the connection between the Capture Engine Unit and an
> > +OV7670 image sensor connected to i2c1 interface.
> > +
> > +ceu: ceu@e821 {
> > +   reg = <0xe821 0x209c>;
> > +   compatible = "renesas,ceu";
>
> Don't forget to update this example when you'll address my comment about
> compatible strings.

Right, I'll use the r7s72100 string.

Thanks
   j
>
> > +   interrupts = ;
> > +
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_pins>;
> > +
> > +   status = "okay";
> > +
> > +   port {
> > +   ceu_in: endpoint {
> > +   remote-endpoint = <_out>;
> > +
> > +   hsync-active = <1>;
> > +   vsync-active = <0>;
> > +   };
> > +   };
> > +};
> > +
> > +i2c1: i2c@fcfee400 {
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_pins>;
> > +
> > +   status = "okay";
> > +
> > +   clock-frequency = <10>;
> > +
> > +   ov7670: camera@21 {
> > +   compatible = 

[PATCH] media: xilinx-video: support pipeline power management

[Akinobu Mita]

This enables pipeline power management for Xilinx Video IP driver.

Some V4L2 subdevices are put their power status into power down mode
after their probe function, and require to be powered up by calling
s_power() operation when the pipeline is in use.

So this change is necessary if the video pipeline contains such a V4L2
subdevice.

Cc: Hyun Kwon 
Cc: Laurent Pinchart 
Signed-off-by: Akinobu Mita 
---
 drivers/media/platform/xilinx/xilinx-dma.c  | 35 +++--
 drivers/media/platform/xilinx/xilinx-vipp.c |  7 +-
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-dma.c 
b/drivers/media/platform/xilinx/xilinx-dma.c
index 522cdfd..cd0b846 100644
--- a/drivers/media/platform/xilinx/xilinx-dma.c
+++ b/drivers/media/platform/xilinx/xilinx-dma.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -644,11 +645,41 @@ static const struct v4l2_ioctl_ops xvip_dma_ioctl_ops = {
  * V4L2 file operations
  */
 
+static int xvip_dma_open(struct file *file)
+{
+   struct video_device *vdev = video_devdata(file);
+   int ret;
+
+   ret = v4l2_pipeline_pm_use(>entity, 1);
+   if (ret)
+   return ret;
+
+   ret = v4l2_fh_open(file);
+   if (ret)
+   v4l2_pipeline_pm_use(>entity, 0);
+
+   return ret;
+}
+
+static int xvip_dma_release(struct file *file)
+{
+   struct video_device *vdev = video_devdata(file);
+   int ret;
+
+   ret = vb2_fop_release(file);
+   if (ret)
+   return ret;
+
+   v4l2_pipeline_pm_use(>entity, 0);
+
+   return 0;
+}
+
 static const struct v4l2_file_operations xvip_dma_fops = {
.owner  = THIS_MODULE,
.unlocked_ioctl = video_ioctl2,
-   .open   = v4l2_fh_open,
-   .release= vb2_fop_release,
+   .open   = xvip_dma_open,
+   .release= xvip_dma_release,
.poll   = vb2_fop_poll,
.mmap   = vb2_fop_mmap,
 };
diff --git a/drivers/media/platform/xilinx/xilinx-vipp.c 
b/drivers/media/platform/xilinx/xilinx-vipp.c
index f4c3e48..6d098e3 100644
--- a/drivers/media/platform/xilinx/xilinx-vipp.c
+++ b/drivers/media/platform/xilinx/xilinx-vipp.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "xilinx-dma.h"
 #include "xilinx-vipp.h"
@@ -573,6 +574,10 @@ static void xvip_composite_v4l2_cleanup(struct 
xvip_composite_device *xdev)
media_device_cleanup(>media_dev);
 }
 
+static const struct media_device_ops xvip_media_ops = {
+   .link_notify = v4l2_pipeline_link_notify,
+};
+
 static int xvip_composite_v4l2_init(struct xvip_composite_device *xdev)
 {
int ret;
@@ -581,7 +586,7 @@ static int xvip_composite_v4l2_init(struct 
xvip_composite_device *xdev)
strlcpy(xdev->media_dev.model, "Xilinx Video Composite Device",
sizeof(xdev->media_dev.model));
xdev->media_dev.hw_revision = 0;
-
+   xdev->media_dev.ops = _media_ops;
media_device_init(>media_dev);
 
xdev->v4l2_dev.mdev = >media_dev;
-- 
2.7.4

[PATCH] media: ov9650: support device tree probing

[Akinobu Mita]

The ov9650 driver currently only supports legacy platform data probe.
This change adds device tree probing.

There has been an attempt to add device tree support for ov9650 driver
by Hugues Fruchet as a part of the patchset that adds support of OV9655
camera (http://www.spinics.net/lists/linux-media/msg117903.html), but
it wasn't merged into mainline because creating a separate driver for
OV9655 is preferred.

This is very similar to Hugues's patch, but not supporting new device.

Cc: H. Nikolaus Schaller 
Cc: Hugues Fruchet 
Cc: Sakari Ailus 
Cc: Mauro Carvalho Chehab 
Cc: Rob Herring 
Signed-off-by: Akinobu Mita 
---
 .../devicetree/bindings/media/i2c/ov9650.txt   |  35 +++
 drivers/media/i2c/ov9650.c | 107 -
 2 files changed, 117 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov9650.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov9650.txt 
b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
new file mode 100644
index 000..aa5024d
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov9650.txt
@@ -0,0 +1,35 @@
+* Omnivision OV9650/OV9652 CMOS sensor
+
+Required Properties:
+- compatible: should be one of
+   "ovti,ov9650"
+   "ovti,ov9652"
+- clocks: reference to the xvclk input clock.
+
+Optional Properties:
+- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
+  Active is high.
+- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
+  Active is high.
+
+The device node must contain one 'port' child node for its digital output
+video port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+ {
+   ov9650: camera@30 {
+   compatible = "ovti,ov9650";
+   reg = <0x30>;
+   reset-gpios = <_gpio_0 0 GPIO_ACTIVE_HIGH>;
+   powerdown-gpios = <_gpio_0 1 GPIO_ACTIVE_HIGH>;
+   clocks = <>;
+
+   port {
+   ov9650_0: endpoint {
+   remote-endpoint = <_in0>;
+   };
+   };
+   };
+};
diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
index 69433e1..1affdc0 100644
--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -11,8 +11,10 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -249,9 +251,10 @@ struct ov965x {
struct v4l2_subdev sd;
struct media_pad pad;
enum v4l2_mbus_type bus_type;
-   int gpios[NUM_GPIOS];
+   struct gpio_desc *gpios[NUM_GPIOS];
/* External master clock frequency */
unsigned long mclk_frequency;
+   struct clk *clk;
 
/* Protects the struct fields below */
struct mutex lock;
@@ -513,10 +516,9 @@ static int ov965x_set_color_matrix(struct ov965x *ov965x)
return 0;
 }
 
-static void ov965x_gpio_set(int gpio, int val)
+static void ov965x_gpio_set(struct gpio_desc *gpio, int val)
 {
-   if (gpio_is_valid(gpio))
-   gpio_set_value(gpio, val);
+   gpiod_set_value_cansleep(gpio, val);
 }
 
 static void __ov965x_set_power(struct ov965x *ov965x, int on)
@@ -1408,16 +1410,17 @@ static const struct v4l2_subdev_ops ov965x_subdev_ops = 
{
 /*
  * Reset and power down GPIOs configuration
  */
-static int ov965x_configure_gpios(struct ov965x *ov965x,
- const struct ov9650_platform_data *pdata)
+static int ov965x_configure_gpios_pdata(struct ov965x *ov965x,
+   const struct ov9650_platform_data *pdata)
 {
int ret, i;
+   int gpios[NUM_GPIOS];
 
-   ov965x->gpios[GPIO_PWDN] = pdata->gpio_pwdn;
-   ov965x->gpios[GPIO_RST]  = pdata->gpio_reset;
+   gpios[GPIO_PWDN] = pdata->gpio_pwdn;
+   gpios[GPIO_RST]  = pdata->gpio_reset;
 
for (i = 0; i < ARRAY_SIZE(ov965x->gpios); i++) {
-   int gpio = ov965x->gpios[i];
+   int gpio = gpios[i];
 
if (!gpio_is_valid(gpio))
continue;
@@ -1427,14 +1430,52 @@ static int ov965x_configure_gpios(struct ov965x *ov965x,
return ret;
v4l2_dbg(1, debug, >sd, "set gpio %d to 1\n", gpio);
 
-   gpio_set_value(gpio, 1);
+   gpio_set_value_cansleep(gpio, 1);
gpio_export(gpio, 0);
-   ov965x->gpios[i] = gpio;
+   ov965x->gpios[i] = gpio_to_desc(gpio);
+   }
+
+   return 0;
+}
+
+static int ov965x_configure_gpios(struct ov965x *ov965x)
+{
+   struct device *dev = >client->dev;
+
+ 

Re: [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

[Philipp Zabel]

Hi Laurent,

On Fri, 2018-01-05 at 15:30 +0200, Laurent Pinchart wrote:
> Hi Philipp,
> 
> Thank you for the patch.
> 
> On Friday, 5 January 2018 00:51:29 EET Philipp Zabel wrote:
> > The Microsoft HoloLens Sensors device has two separate frame descriptors
> > with the same dimensions, each with a single different frame interval:
> > 
> >   VideoStreaming Interface Descriptor:
> > bLength30
> > bDescriptorType36
> > bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> > bFrameIndex 1
> > bmCapabilities   0x00
> >   Still image unsupported
> > wWidth   1280
> > wHeight   481
> > dwMinBitRate147763200
> > dwMaxBitRate147763200
> > dwMaxVideoFrameBufferSize  615680
> > dwDefaultFrameInterval 33
> > bFrameIntervalType  1
> > dwFrameInterval( 0)33
> >   VideoStreaming Interface Descriptor:
> > bLength30
> > bDescriptorType36
> > bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> > bFrameIndex 2
> > bmCapabilities   0x00
> >   Still image unsupported
> > wWidth   1280
> > wHeight   481
> > dwMinBitRate443289600
> > dwMaxBitRate443289600
> > dwMaxVideoFrameBufferSize  615680
> > dwDefaultFrameInterval 11
> > bFrameIntervalType  1
> > dwFrameInterval( 0)11
> > 
> > Skip duplicate dimensions in enum_framesizes, let enum_frameintervals list
> > the intervals from both frame descriptors. Change set_streamparm to switch
> > to the correct frame index when changing the interval. This enables 90 fps
> > capture on a Lenovo Explorer Windows Mixed Reality headset.
> > 
> > Signed-off-by: Philipp Zabel 
> > ---
> > Changes since v1 [1]:
> > - Break out of frame size loop if maxd == 0 in uvc_v4l2_set_streamparm.
> > - Moved d and tmp variables in uvc_v4l2_set_streamparm into loop,
> >   renamed tmp variable to tmp_ival.
> > - Changed i loop variables to unsigned int.
> > - Changed index variables to unsigned int.
> > - One line per variable declaration.
> > 
> > [1] https://patchwork.linuxtv.org/patch/46109/
> > ---
> >  drivers/media/usb/uvc/uvc_v4l2.c | 71
> > +++- 1 file changed, 55 insertions(+),
> > 16 deletions(-)
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> > b/drivers/media/usb/uvc/uvc_v4l2.c index f5ab8164bca5..d9ee400bf47c 100644
> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > @@ -373,7 +373,10 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming
> > *stream, {
> > struct uvc_streaming_control probe;
> > struct v4l2_fract timeperframe;
> > -   uint32_t interval;
> > +   struct uvc_format *format;
> > +   struct uvc_frame *frame;
> > +   __u32 interval, maxd;
> > +   unsigned int i;
> > int ret;
> > 
> > if (parm->type != stream->type)
> > @@ -396,9 +399,33 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming
> > *stream, return -EBUSY;
> > }
> > 
> > +   format = stream->cur_format;
> > +   frame = stream->cur_frame;
> > probe = stream->ctrl;
> > -   probe.dwFrameInterval =
> > -   uvc_try_frame_interval(stream->cur_frame, interval);
> > +   probe.dwFrameInterval = uvc_try_frame_interval(frame, interval);
> > +   maxd = abs((__s32)probe.dwFrameInterval - interval);
> > +
> > +   /* Try frames with matching size to find the best frame interval. */
> > +   for (i = 0; i < format->nframes && maxd != 0; i++) {
> > +   __u32 d, tmp_ival;
>
> How about ival instead of tmp_ival ?
> 
> Apart from that,
> 
> Reviewed-by: Laurent Pinchart 
> 
> If you're fine with the change there's no need to resubmit.

Absolutely, thanks for the review!

regards
Philipp

Re: [PATCH v3 1/6] media: rc: update sunxi-ir driver to get base clock frequency from devicetree

[Sean Young]

On Tue, Dec 19, 2017 at 09:07:42AM +0100, Philipp Rossak wrote:
> This patch updates the sunxi-ir driver to set the base clock frequency from
> devicetree.
> 
> This is necessary since there are different ir receivers on the
> market, that operate with different frequencies. So this value could be
> set if the attached ir receiver needs a different base clock frequency,
> than the default 8 MHz.
> 
> Signed-off-by: Philipp Rossak 

Acked-by: Sean Young 


Sean

> ---
>  drivers/media/rc/sunxi-cir.c | 19 +++
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 97f367b446c4..f500cea228a9 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -72,12 +72,8 @@
>  /* CIR_REG register idle threshold */
>  #define REG_CIR_ITHR(val)(((val) << 8) & (GENMASK(15, 8)))
>  
> -/* Required frequency for IR0 or IR1 clock in CIR mode */
> +/* Required frequency for IR0 or IR1 clock in CIR mode (default) */
>  #define SUNXI_IR_BASE_CLK 800
> -/* Frequency after IR internal divider  */
> -#define SUNXI_IR_CLK  (SUNXI_IR_BASE_CLK / 64)
> -/* Sample period in ns */
> -#define SUNXI_IR_SAMPLE   (10ul / SUNXI_IR_CLK)
>  /* Noise threshold in samples  */
>  #define SUNXI_IR_RXNOISE  1
>  /* Idle Threshold in samples */
> @@ -122,7 +118,8 @@ static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
>   /* for each bit in fifo */
>   dt = readb(ir->base + SUNXI_IR_RXFIFO_REG);
>   rawir.pulse = (dt & 0x80) != 0;
> - rawir.duration = ((dt & 0x7f) + 1) * SUNXI_IR_SAMPLE;
> + rawir.duration = ((dt & 0x7f) + 1) *
> +  ir->rc->rx_resolution;
>   ir_raw_event_store_with_filter(ir->rc, );
>   }
>   }
> @@ -148,6 +145,7 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>   struct device_node *dn = dev->of_node;
>   struct resource *res;
>   struct sunxi_ir *ir;
> + u32 b_clk_freq = SUNXI_IR_BASE_CLK;
>  
>   ir = devm_kzalloc(dev, sizeof(struct sunxi_ir), GFP_KERNEL);
>   if (!ir)
> @@ -172,6 +170,9 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>   return PTR_ERR(ir->clk);
>   }
>  
> + /* Base clock frequency (optional) */
> + of_property_read_u32(dn, "clock-frequency", _clk_freq);
> +
>   /* Reset (optional) */
>   ir->rst = devm_reset_control_get_optional_exclusive(dev, NULL);
>   if (IS_ERR(ir->rst))
> @@ -180,11 +181,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>   if (ret)
>   return ret;
>  
> - ret = clk_set_rate(ir->clk, SUNXI_IR_BASE_CLK);
> + ret = clk_set_rate(ir->clk, b_clk_freq);
>   if (ret) {
>   dev_err(dev, "set ir base clock failed!\n");
>   goto exit_reset_assert;
>   }
> + dev_dbg(dev, "set base clock frequency to %d Hz.\n", b_clk_freq);
>  
>   if (clk_prepare_enable(ir->apb_clk)) {
>   dev_err(dev, "try to enable apb_ir_clk failed\n");
> @@ -225,7 +227,8 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>   ir->rc->map_name = ir->map_name ?: RC_MAP_EMPTY;
>   ir->rc->dev.parent = dev;
>   ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
> - ir->rc->rx_resolution = SUNXI_IR_SAMPLE;
> + /* Frequency after IR internal divider with sample period in ns */
> + ir->rc->rx_resolution = (10ul / (b_clk_freq / 64));
>   ir->rc->timeout = MS_TO_NS(SUNXI_IR_TIMEOUT);
>   ir->rc->driver_name = SUNXI_IR_DEV;
>  
> -- 
> 2.11.0

Mygica 230C defined in two drivers

[Olli Salonen]

Hi Stefan and all,

I noticed that the Mygica 230C is currently defined in two different drivers.

in dvbsky.c:

{ DVB_USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_T230C,
_t230c_props, "MyGica Mini DVB-T2 USB Stick T230C",
RC_MAP_TOTAL_MEDIA_IN_HAND_02) },


in cxusb.c:

[MYGICA_T230] = {
USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_T230)
},
[MYGICA_T230C] = {
USB_DEVICE(USB_VID_CONEXANT, USB_PID_MYGICA_T230+1)
},

and in dvb-usb-ids.h:

#define USB_PID_MYGICA_T2300xc688
#define USB_PID_MYGICA_T230C0xc689

I think you've played around with this device earlier. Do you have any
insight on which driver works better or all they all the same?

Cheers,
-olli

Re: [PATCH 2/9] em28xx: Bulk transfer implementation fix

[Brad Love]

On 2018-01-05 08:20, Devin Heitmueller wrote:
> Hi Brad,
>
> My documents indicate that Register 0x5D and 0x5E are read-only, and
> populated based on the eeprom programming.
>
> On your device, what is the value of those registers prior to you changing 
> them?
>
> If you write to those registers, do they reflect the new values if you
> read them back?
>
> Does changing these values result in any change to the device's
> endpoint configuration (which is typically statically defined when the
> device is probed)?
>
> What precisely is the behavior you were seeing prior to this patch?
>
> Devin

Hey Devin,

We have devices programmed ISOC and bulk in eeprom, but we were seeing
before that bulk transfers were not happening as expected. This included
continuity errors and corrupted packets. After speaking with Empia they
supplied this patch to configure the multiplier explicitly. They also
suggested changing the usb configuration to match this multiplier. This
is done in patch 3/9. I will add some instrumentation to check out the
data you're looking for though. I can say offhand that modifying those
values does have tangible effects. On 'native' machines, there is little
difference, but the multiplier values are make or break in VMWare.

Will reply with the data later.

Cheers,

Brad



> On Thu, Jan 4, 2018 at 7:22 PM, Michael Ira Krufky  
> wrote:
>> On Thu, Jan 4, 2018 at 7:04 PM, Brad Love  wrote:
>>> Set appropriate bulk/ISOC transfer multiplier on capture start.
>>> This sets ISOC transfer to 940 bytes (188 * 5)
>>> This sets bulk transfer to 48128 bytes (188 * 256)
>>>
>>> The above values are maximum allowed according to Empia.
>>>
>>> Signed-off-by: Brad Love 
>> :+1
>>
>> Reviewed-by: Michael Ira Krufky 
>>
>>> ---
>>>  drivers/media/usb/em28xx/em28xx-core.c | 12 
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/media/usb/em28xx/em28xx-core.c 
>>> b/drivers/media/usb/em28xx/em28xx-core.c
>>> index ef38e56..67ed6a3 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-core.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-core.c
>>> @@ -638,6 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
>>> dev->chip_id == CHIP_ID_EM28174 ||
>>> dev->chip_id == CHIP_ID_EM28178) {
>>> /* The Transport Stream Enable Register moved in em2874 */
>>> +   if (dev->dvb_xfer_bulk) {
>>> +   /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 
>>> 2 */
>>> +   em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>>> +   EM2874_R5D_TS1_PKT_SIZE :
>>> +   EM2874_R5E_TS2_PKT_SIZE,
>>> +   0xFF);
>>> +   } else {
>>> +   /* TS2 Maximum Transfer Size = 188 * 5 */
>>> +   em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>>> +   EM2874_R5D_TS1_PKT_SIZE :
>>> +   EM2874_R5E_TS2_PKT_SIZE, 0x05);
>>> +   }
>>> if (dev->ts == PRIMARY_TS)
>>> rc = em28xx_write_reg_bits(dev,
>>> EM2874_R5F_TS_ENABLE,
>>> --
>>> 2.7.4
>>>
>
>

Re: [PATCH v3 0/6] arm: sunxi: IR support for A83T

[Maxime Ripard]

Hi,

On Fri, Jan 05, 2018 at 12:02:53PM +, Sean Young wrote:
> On Tue, Dec 19, 2017 at 09:07:41AM +0100, Philipp Rossak wrote:
> > This patch series adds support for the sunxi A83T ir module and enhances 
> > the sunxi-ir driver. Right now the base clock frequency for the ir driver
> > is a hard coded define and is set to 8 MHz.
> > This works for the most common ir receivers. On the Sinovoip Bananapi M3 
> > the ir receiver needs, a 3 MHz base clock frequency to work without
> > problems with this driver.
> > 
> > This patch series adds support for an optinal property that makes it able
> > to override the default base clock frequency and enables the ir interface 
> > on the a83t and the Bananapi M3.
> > 
> > changes since v2:
> > * reorder cir pin (alphabetical)
> > * fix typo in documentation
> > 
> > changes since v1:
> > * fix typos, reword Documentation
> > * initialize 'b_clk_freq' to 'SUNXI_IR_BASE_CLK' & remove if statement
> > * change dev_info() to dev_dbg()
> > * change naming to cir* in dts/dtsi
> > * Added acked Ackedi-by to related patch
> > * use whole memory block instead of registers needed + fix for h3/h5
> > 
> > changes since rfc:
> > * The property is now optinal. If the property is not available in 
> >   the dtb the driver uses the default base clock frequency.
> > * the driver prints out the the selected base clock frequency.
> > * changed devicetree property from base-clk-frequency to clock-frequency
> > 
> > Regards,
> > Philipp
> > 
> > 
> > Philipp Rossak (6):
> >   media: rc: update sunxi-ir driver to get base clock frequency from
> > devicetree
> >   media: dt: bindings: Update binding documentation for sunxi IR
> > controller
> >   arm: dts: sun8i: a83t: Add the cir pin for the A83T
> >   arm: dts: sun8i: a83t: Add support for the cir interface
> >   arm: dts: sun8i: a83t: bananapi-m3: Enable IR controller
> >   arm: dts: sun8i: h3-h8: ir register size should be the whole memory
> > block
> 
> I can take this series (through rc-core, i.e. linux-media), but I need an
> maintainer Acked-by: for the sun[x8]i dts changes (all four patches).

We'll merge them through our tree. We usually have a rather big number
of patches around, so we'd be better off avoiding conflicts :)

Philipp, can you resubmit the DTs as soon as -rc1 is out?

Thanks!
Maxime

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


signature.asc
Description: PGP signature

[PATCH 0/2] lgdt3306a: fix bugs in usb disconnect/reconnect

[Brad Love]

There are a couple problems currently in this driver, when used as
an i2c_device. This patch set addresses the issues one at a time.

First during process of dvb frontend detach release is called, then
if CONFIG_MEDIA_ATTACH is enabled, the usage count is decremented.
Remove is then called, further decrementing the usage count, to negative
if a single device was attached. Patch one uses dvb_attach to keep the
usage count in sync on removal. I'm not sure if there is a less
'hacky' way to handle this. On a previous attempt I just NULL'd out
release in probe, which just hid the issue. Another way of sorting
out was doing a symbol_get on _attach, but the included patch seems
most consistent behaviour.

Next, there is a double kfree of state which can cause oops/GPF/etc
randomly on removal. In the process of dvb frontend detach release
is called before remove. The problem is _release kfree's state, then
right after _remove cleans up members of state, before kfree'ing
state itself. Patch 2 does not kfree state in _release if the
driver was probed and therefore _remove will be called.


Brad Love (2):
  lgdt3306a: Fix module count mismatch on usb unplug
  lgdt3306a: Fix a double kfree on i2c device remove

 drivers/media/dvb-frontends/lgdt3306a.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH 2/2] lgdt3306a: Fix a double kfree on i2c device remove

[Brad Love]

Both lgdt33606a_release and lgdt3306a_remove kfree state, but _release is
called first, then _remove operates on states members before kfree'ing it.
This can lead to random oops/GPF/etc on USB disconnect.

Signed-off-by: Brad Love 
---
 drivers/media/dvb-frontends/lgdt3306a.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/lgdt3306a.c 
b/drivers/media/dvb-frontends/lgdt3306a.c
index d370671..3642e6e 100644
--- a/drivers/media/dvb-frontends/lgdt3306a.c
+++ b/drivers/media/dvb-frontends/lgdt3306a.c
@@ -1768,7 +1768,13 @@ static void lgdt3306a_release(struct dvb_frontend *fe)
struct lgdt3306a_state *state = fe->demodulator_priv;
 
dbg_info("\n");
-   kfree(state);
+
+   /*
+* If state->muxc is not NULL, then we are an i2c device
+* and lgdt3306a_remove will clean up state
+*/
+   if (!state->muxc)
+   kfree(state);
 }
 
 static const struct dvb_frontend_ops lgdt3306a_ops;
-- 
2.7.4

[PATCH 1/2] lgdt3306a: Fix module count mismatch on usb unplug

[Brad Love]

When used as an i2c device there is a module usage count mismatch on
removal, preventing the driver from being used thereafter. dvb_attach
increments the usage count so it is properly balanced on removal.

On disconnect of Hauppauge SoloHD/DualHD before:

lsmod | grep lgdt3306a
lgdt3306a  28672  -1
i2c_mux16384  1 lgdt3306a

On disconnect of Hauppauge SoloHD/DualHD after:

lsmod | grep lgdt3306a
lgdt3306a  28672  0
i2c_mux16384  1 lgdt3306a

Signed-off-by: Brad Love 
---
 drivers/media/dvb-frontends/lgdt3306a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/lgdt3306a.c 
b/drivers/media/dvb-frontends/lgdt3306a.c
index 6356815..d370671 100644
--- a/drivers/media/dvb-frontends/lgdt3306a.c
+++ b/drivers/media/dvb-frontends/lgdt3306a.c
@@ -2169,7 +2169,7 @@ static int lgdt3306a_probe(struct i2c_client *client,
sizeof(struct lgdt3306a_config));
 
config->i2c_addr = client->addr;
-   fe = lgdt3306a_attach(config, client->adapter);
+   fe = dvb_attach(lgdt3306a_attach, config, client->adapter);
if (fe == NULL) {
ret = -ENODEV;
goto err_fe;
-- 
2.7.4

[PATCH 2/2] media: rc: do not remove first bit if leader pulse is present

[Sean Young]

The rc5 protocol does not have a leading pulse or space, but we encode
the first bit using a single leading pulse. For other protocols, the
leading pulse or space does not represent any bit. So, don't remove the
first bit if a leading pulse is present.

Cc: Antti Seppälä 
Signed-off-by: Sean Young 
---
 drivers/media/rc/ir-mce_kbd-decoder.c |  4 ++--
 drivers/media/rc/ir-rc5-decoder.c | 13 -
 drivers/media/rc/ir-rc6-decoder.c |  4 ++--
 drivers/media/rc/rc-ir-raw.c  |  1 -
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/media/rc/ir-mce_kbd-decoder.c 
b/drivers/media/rc/ir-mce_kbd-decoder.c
index 2a279b3b9c0a..2c3df02e05ff 100644
--- a/drivers/media/rc/ir-mce_kbd-decoder.c
+++ b/drivers/media/rc/ir-mce_kbd-decoder.c
@@ -452,11 +452,11 @@ static int ir_mce_kbd_encode(enum rc_proto protocol, u32 
scancode,
if (protocol == RC_PROTO_MCIR2_KBD) {
raw = scancode |
  ((u64)MCIR2_KEYBOARD_HEADER << MCIR2_KEYBOARD_NBITS);
-   len = MCIR2_KEYBOARD_NBITS + MCIR2_HEADER_NBITS + 1;
+   len = MCIR2_KEYBOARD_NBITS + MCIR2_HEADER_NBITS;
} else {
raw = scancode |
  ((u64)MCIR2_MOUSE_HEADER << MCIR2_MOUSE_NBITS);
-   len = MCIR2_MOUSE_NBITS + MCIR2_HEADER_NBITS + 1;
+   len = MCIR2_MOUSE_NBITS + MCIR2_HEADER_NBITS;
}
 
ret = ir_raw_gen_manchester(, max, _mce_kbd_timings, len, raw);
diff --git a/drivers/media/rc/ir-rc5-decoder.c 
b/drivers/media/rc/ir-rc5-decoder.c
index a1d6c955ffc8..11a28f8772da 100644
--- a/drivers/media/rc/ir-rc5-decoder.c
+++ b/drivers/media/rc/ir-rc5-decoder.c
@@ -225,9 +225,9 @@ static int ir_rc5_encode(enum rc_proto protocol, u32 
scancode,
/* encode data */
data = !commandx << 12 | system << 6 | command;
 
-   /* Modulate the data */
+   /* First bit is encoded by leader_pulse */
ret = ir_raw_gen_manchester(, max, _rc5_timings,
-   RC5_NBITS, data);
+   RC5_NBITS - 1, data);
if (ret < 0)
return ret;
} else if (protocol == RC_PROTO_RC5X_20) {
@@ -240,10 +240,11 @@ static int ir_rc5_encode(enum rc_proto protocol, u32 
scancode,
/* encode data */
data = commandx << 18 | system << 12 | command << 6 | xdata;
 
-   /* Modulate the data */
+   /* First bit is encoded by leader_pulse */
pre_space_data = data >> (RC5X_NBITS - CHECK_RC5X_NBITS);
ret = ir_raw_gen_manchester(, max, _rc5x_timings[0],
-   CHECK_RC5X_NBITS, pre_space_data);
+   CHECK_RC5X_NBITS - 1,
+   pre_space_data);
if (ret < 0)
return ret;
ret = ir_raw_gen_manchester(, max - (e - events),
@@ -254,8 +255,10 @@ static int ir_rc5_encode(enum rc_proto protocol, u32 
scancode,
return ret;
} else if (protocol == RC_PROTO_RC5_SZ) {
/* RC5-SZ scancode is raw enough for Manchester as it is */
+   /* First bit is encoded by leader_pulse */
ret = ir_raw_gen_manchester(, max, _rc5_sz_timings,
-   RC5_SZ_NBITS, scancode & 0x2fff);
+   RC5_SZ_NBITS - 1,
+   scancode & 0x2fff);
if (ret < 0)
return ret;
} else {
diff --git a/drivers/media/rc/ir-rc6-decoder.c 
b/drivers/media/rc/ir-rc6-decoder.c
index 422dec08738c..55bb19bbd4e9 100644
--- a/drivers/media/rc/ir-rc6-decoder.c
+++ b/drivers/media/rc/ir-rc6-decoder.c
@@ -327,7 +327,7 @@ static int ir_rc6_encode(enum rc_proto protocol, u32 
scancode,
/* Modulate the header (Start Bit & Mode-0) */
ret = ir_raw_gen_manchester(, max - (e - events),
_rc6_timings[0],
-   RC6_HEADER_NBITS + 1, (1 << 3));
+   RC6_HEADER_NBITS, (1 << 3));
if (ret < 0)
return ret;
 
@@ -365,7 +365,7 @@ static int ir_rc6_encode(enum rc_proto protocol, u32 
scancode,
/* Modulate the header (Start Bit & Header-version 6 */
ret = ir_raw_gen_manchester(, max - (e - events),
_rc6_timings[0],
-   RC6_HEADER_NBITS + 1, (1 << 3 | 6));
+   RC6_HEADER_NBITS, (1 << 3 | 6));
if (ret < 0)
return ret;
 
diff --git 

[PATCH 1/2] media: rc: clean up leader pulse/space for manchester encoding

[Sean Young]

The IR rc6 encoder sends the header using manchester encoding using 0
bits, which causes the following:

UBSAN: Undefined behaviour in drivers/media/rc/rc-ir-raw.c:247:6
shift exponent 4294967295 is too large for 64-bit type 'long long unsigned int'

So, allow the leader code to send a pulse and space and remove the unused
pulse_space_start field.

Cc: Antti Seppälä 
Signed-off-by: Sean Young 
---
 drivers/media/rc/ir-mce_kbd-decoder.c |  2 +-
 drivers/media/rc/ir-rc5-decoder.c |  9 +++--
 drivers/media/rc/ir-rc6-decoder.c | 35 ++-
 drivers/media/rc/rc-core-priv.h   | 10 +-
 drivers/media/rc/rc-ir-raw.c  | 12 +---
 5 files changed, 24 insertions(+), 44 deletions(-)

diff --git a/drivers/media/rc/ir-mce_kbd-decoder.c 
b/drivers/media/rc/ir-mce_kbd-decoder.c
index 8cf4cf358052..2a279b3b9c0a 100644
--- a/drivers/media/rc/ir-mce_kbd-decoder.c
+++ b/drivers/media/rc/ir-mce_kbd-decoder.c
@@ -424,7 +424,7 @@ static int ir_mce_kbd_unregister(struct rc_dev *dev)
 }
 
 static const struct ir_raw_timings_manchester ir_mce_kbd_timings = {
-   .leader = MCIR2_PREFIX_PULSE,
+   .leader_pulse   = MCIR2_PREFIX_PULSE,
.invert = 1,
.clock  = MCIR2_UNIT,
.trailer_space  = MCIR2_UNIT * 10,
diff --git a/drivers/media/rc/ir-rc5-decoder.c 
b/drivers/media/rc/ir-rc5-decoder.c
index f589d99245eb..a1d6c955ffc8 100644
--- a/drivers/media/rc/ir-rc5-decoder.c
+++ b/drivers/media/rc/ir-rc5-decoder.c
@@ -173,16 +173,14 @@ static int ir_rc5_decode(struct rc_dev *dev, struct 
ir_raw_event ev)
 }
 
 static const struct ir_raw_timings_manchester ir_rc5_timings = {
-   .leader = RC5_UNIT,
-   .pulse_space_start  = 0,
+   .leader_pulse   = RC5_UNIT,
.clock  = RC5_UNIT,
.trailer_space  = RC5_UNIT * 10,
 };
 
 static const struct ir_raw_timings_manchester ir_rc5x_timings[2] = {
{
-   .leader = RC5_UNIT,
-   .pulse_space_start  = 0,
+   .leader_pulse   = RC5_UNIT,
.clock  = RC5_UNIT,
.trailer_space  = RC5X_SPACE,
},
@@ -193,8 +191,7 @@ static const struct ir_raw_timings_manchester 
ir_rc5x_timings[2] = {
 };
 
 static const struct ir_raw_timings_manchester ir_rc5_sz_timings = {
-   .leader = RC5_UNIT,
-   .pulse_space_start  = 0,
+   .leader_pulse   = RC5_UNIT,
.clock  = RC5_UNIT,
.trailer_space  = RC5_UNIT * 10,
 };
diff --git a/drivers/media/rc/ir-rc6-decoder.c 
b/drivers/media/rc/ir-rc6-decoder.c
index 665025303c28..422dec08738c 100644
--- a/drivers/media/rc/ir-rc6-decoder.c
+++ b/drivers/media/rc/ir-rc6-decoder.c
@@ -288,13 +288,8 @@ static int ir_rc6_decode(struct rc_dev *dev, struct 
ir_raw_event ev)
 
 static const struct ir_raw_timings_manchester ir_rc6_timings[4] = {
{
-   .leader = RC6_PREFIX_PULSE,
-   .pulse_space_start  = 0,
-   .clock  = RC6_UNIT,
-   .invert = 1,
-   .trailer_space  = RC6_PREFIX_SPACE,
-   },
-   {
+   .leader_pulse   = RC6_PREFIX_PULSE,
+   .leader_space   = RC6_PREFIX_SPACE,
.clock  = RC6_UNIT,
.invert = 1,
},
@@ -329,27 +324,22 @@ static int ir_rc6_encode(enum rc_proto protocol, u32 
scancode,
struct ir_raw_event *e = events;
 
if (protocol == RC_PROTO_RC6_0) {
-   /* Modulate the preamble */
-   ret = ir_raw_gen_manchester(, max, _rc6_timings[0], 0, 0);
-   if (ret < 0)
-   return ret;
-
/* Modulate the header (Start Bit & Mode-0) */
ret = ir_raw_gen_manchester(, max - (e - events),
-   _rc6_timings[1],
-   RC6_HEADER_NBITS, (1 << 3));
+   _rc6_timings[0],
+   RC6_HEADER_NBITS + 1, (1 << 3));
if (ret < 0)
return ret;
 
/* Modulate Trailer Bit */
ret = ir_raw_gen_manchester(, max - (e - events),
-   _rc6_timings[2], 1, 0);
+   _rc6_timings[1], 1, 0);
if (ret < 0)
return ret;
 
/* Modulate rest of the data */
ret = ir_raw_gen_manchester(, max - (e - events),
-   _rc6_timings[3], RC6_0_NBITS,
+   _rc6_timings[2], 

Re: [PATCH 2/9] em28xx: Bulk transfer implementation fix

[Devin Heitmueller]

Hi Brad,

My documents indicate that Register 0x5D and 0x5E are read-only, and
populated based on the eeprom programming.

On your device, what is the value of those registers prior to you changing them?

If you write to those registers, do they reflect the new values if you
read them back?

Does changing these values result in any change to the device's
endpoint configuration (which is typically statically defined when the
device is probed)?

What precisely is the behavior you were seeing prior to this patch?

Devin

On Thu, Jan 4, 2018 at 7:22 PM, Michael Ira Krufky  wrote:
> On Thu, Jan 4, 2018 at 7:04 PM, Brad Love  wrote:
>> Set appropriate bulk/ISOC transfer multiplier on capture start.
>> This sets ISOC transfer to 940 bytes (188 * 5)
>> This sets bulk transfer to 48128 bytes (188 * 256)
>>
>> The above values are maximum allowed according to Empia.
>>
>> Signed-off-by: Brad Love 
>
> :+1
>
> Reviewed-by: Michael Ira Krufky 
>
>> ---
>>  drivers/media/usb/em28xx/em28xx-core.c | 12 
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/media/usb/em28xx/em28xx-core.c 
>> b/drivers/media/usb/em28xx/em28xx-core.c
>> index ef38e56..67ed6a3 100644
>> --- a/drivers/media/usb/em28xx/em28xx-core.c
>> +++ b/drivers/media/usb/em28xx/em28xx-core.c
>> @@ -638,6 +638,18 @@ int em28xx_capture_start(struct em28xx *dev, int start)
>> dev->chip_id == CHIP_ID_EM28174 ||
>> dev->chip_id == CHIP_ID_EM28178) {
>> /* The Transport Stream Enable Register moved in em2874 */
>> +   if (dev->dvb_xfer_bulk) {
>> +   /* Max Tx Size = 188 * 256 = 48128 - LCM(188,512) * 
>> 2 */
>> +   em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>> +   EM2874_R5D_TS1_PKT_SIZE :
>> +   EM2874_R5E_TS2_PKT_SIZE,
>> +   0xFF);
>> +   } else {
>> +   /* TS2 Maximum Transfer Size = 188 * 5 */
>> +   em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
>> +   EM2874_R5D_TS1_PKT_SIZE :
>> +   EM2874_R5E_TS2_PKT_SIZE, 0x05);
>> +   }
>> if (dev->ts == PRIMARY_TS)
>> rc = em28xx_write_reg_bits(dev,
>> EM2874_R5F_TS_ENABLE,
>> --
>> 2.7.4
>>



-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH] media: uvcvideo: support multiple frame descriptors with the same dimensions

[Laurent Pinchart]

Hi Philipp,

Thank you for the patch.

On Friday, 5 January 2018 00:51:29 EET Philipp Zabel wrote:
> The Microsoft HoloLens Sensors device has two separate frame descriptors
> with the same dimensions, each with a single different frame interval:
> 
>   VideoStreaming Interface Descriptor:
> bLength30
> bDescriptorType36
> bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> bFrameIndex 1
> bmCapabilities   0x00
>   Still image unsupported
> wWidth   1280
> wHeight   481
> dwMinBitRate147763200
> dwMaxBitRate147763200
> dwMaxVideoFrameBufferSize  615680
> dwDefaultFrameInterval 33
> bFrameIntervalType  1
> dwFrameInterval( 0)33
>   VideoStreaming Interface Descriptor:
> bLength30
> bDescriptorType36
> bDescriptorSubtype  5 (FRAME_UNCOMPRESSED)
> bFrameIndex 2
> bmCapabilities   0x00
>   Still image unsupported
> wWidth   1280
> wHeight   481
> dwMinBitRate443289600
> dwMaxBitRate443289600
> dwMaxVideoFrameBufferSize  615680
> dwDefaultFrameInterval 11
> bFrameIntervalType  1
> dwFrameInterval( 0)11
> 
> Skip duplicate dimensions in enum_framesizes, let enum_frameintervals list
> the intervals from both frame descriptors. Change set_streamparm to switch
> to the correct frame index when changing the interval. This enables 90 fps
> capture on a Lenovo Explorer Windows Mixed Reality headset.
> 
> Signed-off-by: Philipp Zabel 
> ---
> Changes since v1 [1]:
> - Break out of frame size loop if maxd == 0 in uvc_v4l2_set_streamparm.
> - Moved d and tmp variables in uvc_v4l2_set_streamparm into loop,
>   renamed tmp variable to tmp_ival.
> - Changed i loop variables to unsigned int.
> - Changed index variables to unsigned int.
> - One line per variable declaration.
> 
> [1] https://patchwork.linuxtv.org/patch/46109/
> ---
>  drivers/media/usb/uvc/uvc_v4l2.c | 71
> +++- 1 file changed, 55 insertions(+),
> 16 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> b/drivers/media/usb/uvc/uvc_v4l2.c index f5ab8164bca5..d9ee400bf47c 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -373,7 +373,10 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming
> *stream, {
>   struct uvc_streaming_control probe;
>   struct v4l2_fract timeperframe;
> - uint32_t interval;
> + struct uvc_format *format;
> + struct uvc_frame *frame;
> + __u32 interval, maxd;
> + unsigned int i;
>   int ret;
> 
>   if (parm->type != stream->type)
> @@ -396,9 +399,33 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming
> *stream, return -EBUSY;
>   }
> 
> + format = stream->cur_format;
> + frame = stream->cur_frame;
>   probe = stream->ctrl;
> - probe.dwFrameInterval =
> - uvc_try_frame_interval(stream->cur_frame, interval);
> + probe.dwFrameInterval = uvc_try_frame_interval(frame, interval);
> + maxd = abs((__s32)probe.dwFrameInterval - interval);
> +
> + /* Try frames with matching size to find the best frame interval. */
> + for (i = 0; i < format->nframes && maxd != 0; i++) {
> + __u32 d, tmp_ival;

How about ival instead of tmp_ival ?

Apart from that,

Reviewed-by: Laurent Pinchart 

If you're fine with the change there's no need to resubmit.

> +
> + if (>frame[i] == stream->cur_frame)
> + continue;
> +
> + if (format->frame[i].wWidth != stream->cur_frame->wWidth ||
> + format->frame[i].wHeight != stream->cur_frame->wHeight)
> + continue;
> +
> + tmp_ival = uvc_try_frame_interval(>frame[i], interval);
> + d = abs((__s32)tmp_ival - interval);
> + if (d >= maxd)
> + continue;
> +
> + frame = >frame[i];
> + probe.bFrameIndex = frame->bFrameIndex;
> + probe.dwFrameInterval = tmp_ival;
> + maxd = d;
> + }
> 
>   /* Probe the device with the new settings. */
>   ret = uvc_probe_video(stream, );
> @@ -408,6 +435,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming
> *stream, }
> 
>   stream->ctrl = probe;
> + stream->cur_frame = frame;
>   mutex_unlock(>mutex);
> 
>   /* Return the actual frame period. */
> 

Re: [PATCH] media: staging: tegra-vde: select DMA_SHARED_BUFFER

[Dmitry Osipenko]

On 05.01.2018 12:43, Arnd Bergmann wrote:
> Without CONFIG_DMA_SHARED_BUFFER we run into a link error for the
> dma_buf_* APIs:
> 
> ERROR: "dma_buf_map_attachment" 
> [drivers/staging/media/tegra-vde/tegra-vde.ko] undefined!
> ERROR: "dma_buf_attach" [drivers/staging/media/tegra-vde/tegra-vde.ko] 
> undefined!
> ERROR: "dma_buf_get" [drivers/staging/media/tegra-vde/tegra-vde.ko] undefined!
> ERROR: "dma_buf_put" [drivers/staging/media/tegra-vde/tegra-vde.ko] undefined!
> ERROR: "dma_buf_detach" [drivers/staging/media/tegra-vde/tegra-vde.ko] 
> undefined!
> ERROR: "dma_buf_unmap_attachment" 
> [drivers/staging/media/tegra-vde/tegra-vde.ko] undefined!
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/staging/media/tegra-vde/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/tegra-vde/Kconfig 
> b/drivers/staging/media/tegra-vde/Kconfig
> index ec3ebdaa..5c4914674468 100644
> --- a/drivers/staging/media/tegra-vde/Kconfig
> +++ b/drivers/staging/media/tegra-vde/Kconfig
> @@ -1,6 +1,7 @@
>  config TEGRA_VDE
>   tristate "NVIDIA Tegra Video Decoder Engine driver"
>   depends on ARCH_TEGRA || COMPILE_TEST
> + select DMA_SHARED_BUFFER
>   select SRAM
>   help
>   Say Y here to enable support for the NVIDIA Tegra video decoder
> 

Thanks!

Acked-by: Dmitry Osipenko 

Re: [PATCH v3 0/6] arm: sunxi: IR support for A83T

[Sean Young]

On Tue, Dec 19, 2017 at 09:07:41AM +0100, Philipp Rossak wrote:
> This patch series adds support for the sunxi A83T ir module and enhances 
> the sunxi-ir driver. Right now the base clock frequency for the ir driver
> is a hard coded define and is set to 8 MHz.
> This works for the most common ir receivers. On the Sinovoip Bananapi M3 
> the ir receiver needs, a 3 MHz base clock frequency to work without
> problems with this driver.
> 
> This patch series adds support for an optinal property that makes it able
> to override the default base clock frequency and enables the ir interface 
> on the a83t and the Bananapi M3.
> 
> changes since v2:
> * reorder cir pin (alphabetical)
> * fix typo in documentation
> 
> changes since v1:
> * fix typos, reword Documentation
> * initialize 'b_clk_freq' to 'SUNXI_IR_BASE_CLK' & remove if statement
> * change dev_info() to dev_dbg()
> * change naming to cir* in dts/dtsi
> * Added acked Ackedi-by to related patch
> * use whole memory block instead of registers needed + fix for h3/h5
> 
> changes since rfc:
> * The property is now optinal. If the property is not available in 
>   the dtb the driver uses the default base clock frequency.
> * the driver prints out the the selected base clock frequency.
> * changed devicetree property from base-clk-frequency to clock-frequency
> 
> Regards,
> Philipp
> 
> 
> Philipp Rossak (6):
>   media: rc: update sunxi-ir driver to get base clock frequency from
> devicetree
>   media: dt: bindings: Update binding documentation for sunxi IR
> controller
>   arm: dts: sun8i: a83t: Add the cir pin for the A83T
>   arm: dts: sun8i: a83t: Add support for the cir interface
>   arm: dts: sun8i: a83t: bananapi-m3: Enable IR controller
>   arm: dts: sun8i: h3-h8: ir register size should be the whole memory
> block

I can take this series (through rc-core, i.e. linux-media), but I need an
maintainer Acked-by: for the sun[x8]i dts changes (all four patches).

>  Documentation/devicetree/bindings/media/sunxi-ir.txt |  3 +++
>  arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts |  7 +++
>  arch/arm/boot/dts/sun8i-a83t.dtsi| 15 +++
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi   |  2 +-
>  drivers/media/rc/sunxi-cir.c | 19 +++
>  5 files changed, 37 insertions(+), 9 deletions(-)


Thanks
Sean

Re: [PATCH v2 8/9] lgdt3306a: QAM streaming improvement

[Michael Ira Krufky]

On Thu, Jan 4, 2018 at 8:30 PM, Brad Love  wrote:
> Add some register updates required for stable viewing
> on Cablevision in NY. Does not adversely affect other providers.
>
> Changes since v1:
> - Change upper case hex to lower case.
>
> Signed-off-by: Brad Love 

Looks good - Thanks.

Reviewed-by: Michael Ira Krufky 

> ---
>  drivers/media/dvb-frontends/lgdt3306a.c | 22 ++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/media/dvb-frontends/lgdt3306a.c 
> b/drivers/media/dvb-frontends/lgdt3306a.c
> index d2477ed..2f540f1 100644
> --- a/drivers/media/dvb-frontends/lgdt3306a.c
> +++ b/drivers/media/dvb-frontends/lgdt3306a.c
> @@ -598,6 +598,28 @@ static int lgdt3306a_set_qam(struct lgdt3306a_state 
> *state, int modulation)
> if (lg_chkerr(ret))
> goto fail;
>
> +   /* 5.1 V0.36 SRDCHKALWAYS : For better QAM detection */
> +   ret = lgdt3306a_read_reg(state, 0x000a, );
> +   val &= 0xfd;
> +   val |= 0x02;
> +   ret = lgdt3306a_write_reg(state, 0x000a, val);
> +   if (lg_chkerr(ret))
> +   goto fail;
> +
> +   /* 5.2 V0.36 Control of "no signal" detector function */
> +   ret = lgdt3306a_read_reg(state, 0x2849, );
> +   val &= 0xdf;
> +   ret = lgdt3306a_write_reg(state, 0x2849, val);
> +   if (lg_chkerr(ret))
> +   goto fail;
> +
> +   /* 5.3 Fix for Blonder Tongue HDE-2H-QAM and AQM modulators */
> +   ret = lgdt3306a_read_reg(state, 0x302b, );
> +   val &= 0x7f;  /* SELFSYNCFINDEN_CQS=0; disable auto reset */
> +   ret = lgdt3306a_write_reg(state, 0x302b, val);
> +   if (lg_chkerr(ret))
> +   goto fail;
> +
> /* 6. Reset */
> ret = lgdt3306a_soft_reset(state);
> if (lg_chkerr(ret))
> --
> 2.7.4
>

[PATCH] media: cobalt: select CONFIG_SND_PCM

[Arnd Bergmann]

The cobalt sound driver has a dependency on ALSA, but not
on the PCM helper code, so this can lead to an extremely
rare link error in randconfig builds:

ERROR: "snd_pcm_period_elapsed" [drivers/media/pci/cobalt/cobalt.ko] undefined!
ERROR: "_snd_pcm_stream_lock_irqsave" [drivers/media/pci/cobalt/cobalt.ko] 
undefined!
ERROR: "snd_pcm_hw_constraint_integer" [drivers/media/pci/cobalt/cobalt.ko] 
undefined!
ERROR: "snd_pcm_set_ops" [drivers/media/pci/cobalt/cobalt.ko] undefined!
ERROR: "snd_pcm_stream_unlock_irqrestore" [drivers/media/pci/cobalt/cobalt.ko] 
undefined!
ERROR: "snd_pcm_lib_ioctl" [drivers/media/pci/cobalt/cobalt.ko] undefined!
ERROR: "snd_pcm_new" [drivers/media/pci/cobalt/cobalt.ko] undefined!

The other audio drivers select 'SND_PCM' for this, so let's
do the same.

Signed-off-by: Arnd Bergmann 
---
 drivers/media/pci/cobalt/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/pci/cobalt/Kconfig b/drivers/media/pci/cobalt/Kconfig
index 70343829a125..aa35cbc0a904 100644
--- a/drivers/media/pci/cobalt/Kconfig
+++ b/drivers/media/pci/cobalt/Kconfig
@@ -6,6 +6,7 @@ config VIDEO_COBALT
depends on SND
depends on MTD
select I2C_ALGOBIT
+   select SND_PCM
select VIDEO_ADV7604
select VIDEO_ADV7511
select VIDEO_ADV7842
-- 
2.9.0

[PATCH] media: staging: tegra-vde: select DMA_SHARED_BUFFER

[Arnd Bergmann]

Without CONFIG_DMA_SHARED_BUFFER we run into a link error for the
dma_buf_* APIs:

ERROR: "dma_buf_map_attachment" [drivers/staging/media/tegra-vde/tegra-vde.ko] 
undefined!
ERROR: "dma_buf_attach" [drivers/staging/media/tegra-vde/tegra-vde.ko] 
undefined!
ERROR: "dma_buf_get" [drivers/staging/media/tegra-vde/tegra-vde.ko] undefined!
ERROR: "dma_buf_put" [drivers/staging/media/tegra-vde/tegra-vde.ko] undefined!
ERROR: "dma_buf_detach" [drivers/staging/media/tegra-vde/tegra-vde.ko] 
undefined!
ERROR: "dma_buf_unmap_attachment" 
[drivers/staging/media/tegra-vde/tegra-vde.ko] undefined!

Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/tegra-vde/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/tegra-vde/Kconfig 
b/drivers/staging/media/tegra-vde/Kconfig
index ec3ebdaa..5c4914674468 100644
--- a/drivers/staging/media/tegra-vde/Kconfig
+++ b/drivers/staging/media/tegra-vde/Kconfig
@@ -1,6 +1,7 @@
 config TEGRA_VDE
tristate "NVIDIA Tegra Video Decoder Engine driver"
depends on ARCH_TEGRA || COMPILE_TEST
+   select DMA_SHARED_BUFFER
select SRAM
help
Say Y here to enable support for the NVIDIA Tegra video decoder
-- 
2.9.0

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

[Geert Uytterhoeven]

On Thu, Jan 4, 2018 at 11:28 PM, Laurent Pinchart
 wrote:
> On Thursday, 4 January 2018 18:03:09 EET Jacopo Mondi wrote:
>> Add bindings documentation for Renesas Capture Engine Unit (CEU).

>> +++ b/Documentation/devicetree/bindings/media/renesas,ceu.txt
>> @@ -0,0 +1,85 @@
>> +Renesas Capture Engine Unit (CEU)
>> +--
>> +
>> +The Capture Engine Unit is the image capture interface found in the Renesas
>> +SH Mobile and RZ SoCs.
>> +
>> +The interface supports a single parallel input with data bus width of 8 or
>> 16
>> +bits.
>> +
>> +Required properties:
>> +- compatible: Must be one of the following.
>> + - "renesas,r7s72100-ceu" for CEU units found in R7S72100 (RZ/A1) SoCs.
>> + - "renesas,ceu" as a generic fallback.
>
> As asked in my review of patch 3/9, what's your policy for compatible strings
> ? As far as I understand there's no generic CEU, as the SH4 and RZ versions
> are not compatible. Should the "renesas,ceu" compatible string then be
> replaced by "renesas,rz-ceu" ?

If they're not compatible, it indeed doesn't make much sense to have a
generic "renesas,ceu".

Note that anything with "rz-" is a bad idea, as after RZ/A1, Renesas introduced
RZ/G1, RZ/N1, and RZ/T1, which are completely different (yes I know
we have a few of these in use, unfortunately).

Gr{oetje,eeting}s,

Geert

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

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

Re: [RESEND PATCH 0/2] dw9714 fixes, cleanups

[Sakari Ailus]

On Fri, Jan 05, 2018 at 01:22:54AM +, Mani, Rajmohan wrote:
> > The two patches address a small bug in dw9714 driver and clean it up a 
> > little,
> > too.
> > 
> > Raj: could you let me know if they work for you? Thanks.
> > 
> 
> These patches work fine.

Thanks! I'll add:

Tested-by: Rajmohan Mani 

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