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:   Wed Feb 28 05:00:18 CET 2018
media-tree git hash:e3e389f931a14ddf43089c7db92fc5d74edf93a4
media_build git hash:   a9ea3d056e5ce50d37dd6129126f776c3a8ec2d7
v4l-utils git hash: c969495f2788837134ceda5b7a68bc0d4d10f8d0
gcc version:i686-linux-gcc (GCC) 7.3.0
sparse version: v0.5.0-3994-g45eb2282
smatch version: v0.5.0-3994-g45eb2282
host hardware:  x86_64
host os:4.14.0-3-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-arm64: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.98-i686: ERRORS
linux-3.2.98-x86_64: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-i686: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-i686: ERRORS
linux-3.9.2-x86_64: ERRORS
linux-3.10.1-i686: ERRORS
linux-3.10.1-x86_64: ERRORS
linux-3.11.1-i686: ERRORS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.53-i686: ERRORS
linux-3.16.53-x86_64: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.93-i686: ERRORS
linux-3.18.93-x86_64: ERRORS
linux-3.19-i686: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.49-i686: ERRORS
linux-4.1.49-x86_64: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.115-i686: ERRORS
linux-4.4.115-x86_64: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-i686: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9.80-i686: ERRORS
linux-4.9.80-x86_64: ERRORS
linux-4.10.14-i686: OK
linux-4.10.14-x86_64: WARNINGS
linux-4.11-i686: OK
linux-4.11-x86_64: WARNINGS
linux-4.12.1-i686: OK
linux-4.12.1-x86_64: WARNINGS
linux-4.13-i686: OK
linux-4.13-x86_64: OK
linux-4.14.17-i686: OK
linux-4.14.17-x86_64: OK
linux-4.15.2-i686: OK
linux-4.15.2-x86_64: OK
linux-4.16-rc1-i686: OK
linux-4.16-rc1-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS
smatch: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH for v3.2 00/12] v4l2-compat-ioctl32.c: remove set_fs(KERNEL_DS)

[Ben Hutchings]

On Wed, 2018-02-14 at 13:03 +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> This patch series fixes a number of bugs and culminates in the removal
> of the set_fs(KERNEL_DS) call in v4l2-compat-ioctl32.c.
> 
> This was tested with a VM running 3.2, the vivi driver (a poor substitute for
> the much improved vivid driver that's available in later kernels, but it's the
> best I had) since that emulates the more common V4L2 ioctls that need to pass
> through v4l2-compat-ioctl32.c) and the 32-bit v4l2-compliance + 32-bit 
> v4l2-ctl
> utilities that together exercised the most common ioctls.
> 
> Most of the v4l2-compat-ioctl32.c do cleanups and fix subtle issues that
> v4l2-compliance complained about. The purpose is to 1) make it easy to
> verify that the final patch didn't introduce errors by first eliminating
> errors caused by other known bugs, and 2) keep the final patch at least
> somewhat readable.

Thanks, I've queued up all of these.  Again, I rebased these on top
of some earlier fixes to v4l2-compat-ioctl32.c which you incorporated
into your backports.

Ben.

> Regards,
> 
>   Hans
> 
> Daniel Mentz (2):
>   media: v4l2-compat-ioctl32: Copy v4l2_window->global_alpha
>   media: v4l2-compat-ioctl32.c: refactor compat ioctl32 logic
> 
> Hans Verkuil (10):
>   media: v4l2-ioctl.c: don't copy back the result for -ENOTTY
>   media: v4l2-compat-ioctl32.c: add missing VIDIOC_PREPARE_BUF
>   media: v4l2-compat-ioctl32.c: fix the indentation
>   media: v4l2-compat-ioctl32.c: move 'helper' functions to
> __get/put_v4l2_format32
>   media: v4l2-compat-ioctl32.c: avoid sizeof(type)
>   media: v4l2-compat-ioctl32.c: copy m.userptr in put_v4l2_plane32
>   media: v4l2-compat-ioctl32.c: fix ctrl_is_pointer
>   media: v4l2-compat-ioctl32.c: copy clip list in put_v4l2_window32
>   media: v4l2-compat-ioctl32.c: drop pr_info for unknown buffer type
>   media: v4l2-compat-ioctl32.c: don't copy back the result for certain
> errors
> 
>  drivers/media/video/Makefile  |   7 +-
>  drivers/media/video/v4l2-compat-ioctl32.c | 966 
> ++
>  drivers/media/video/v4l2-ioctl.c  |   6 +-
>  3 files changed, 597 insertions(+), 382 deletions(-)
> 
-- 
Ben Hutchings
If the facts do not conform to your theory, they must be disposed of.



signature.asc
Description: This is a digitally signed message part

Re: [PATCH for v3.16 00/14] v4l2-compat-ioctl32.c: remove set_fs(KERNEL_DS)

[Ben Hutchings]

On Wed, 2018-02-14 at 12:59 +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> This patch series fixes a number of bugs and culminates in the removal
> of the set_fs(KERNEL_DS) call in v4l2-compat-ioctl32.c.
> 
> This was tested with a VM running 3.16, the vivi driver (a poor substitute for
> the much improved vivid driver that's available in later kernels, but it's the
> best I had) since that emulates the more common V4L2 ioctls that need to pass
> through v4l2-compat-ioctl32.c) and the 32-bit v4l2-compliance + 32-bit 
> v4l2-ctl
> utilities that together exercised the most common ioctls.
> 
> Most of the v4l2-compat-ioctl32.c do cleanups and fix subtle issues that
> v4l2-compliance complained about. The purpose is to 1) make it easy to
> verify that the final patch didn't introduce errors by first eliminating
> errors caused by other known bugs, and 2) keep the final patch at least
> somewhat readable.
> 
> While compiling the media drivers for 3.16 I also came across a bug
> introduced in the 3.16 stable series that caused a compile error in the
> adv7604 driver. That's fixed in the first patch. Call it a bonus patch :-)

Thanks, I've queued up all of these.  However, I rebased these on top
of some earlier fixes to v4l2-compat-ioctl32.c which you incorporated
into your backports.

Ben.

-- 
Ben Hutchings
If the facts do not conform to your theory, they must be disposed of.



signature.asc
Description: This is a digitally signed message part

[linuxtv-media:fixes 3/12] drivers/media/dvb-core/dvb_vb2.c:183: undefined reference to `vb2_vmalloc_memops'

[kbuild test robot]

tree:   git://linuxtv.org/media_tree.git fixes
head:   7dbdd16a79a9d27d7dca0a49029fc8966dcfecc5
commit: ec5b100462543aee1f3e139e168699fd3b05cdc6 [3/12] media: dvb: fix 
DVB_MMAP symbol name
config: x86_64-randconfig-s2-02280514 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
git checkout ec5b100462543aee1f3e139e168699fd3b05cdc6
# save the attached .config to linux build tree
make ARCH=x86_64 

Note: the linuxtv-media/fixes HEAD 7dbdd16a79a9d27d7dca0a49029fc8966dcfecc5 
builds fine.
  It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/media/dvb-core/dvb_vb2.o: In function `dvb_vb2_init':
>> drivers/media/dvb-core/dvb_vb2.c:183: undefined reference to 
>> `vb2_vmalloc_memops'

vim +183 drivers/media/dvb-core/dvb_vb2.c

57868acc Satendra Singh Thakur 2017-12-18  164  
57868acc Satendra Singh Thakur 2017-12-18  165  /*
57868acc Satendra Singh Thakur 2017-12-18  166   * Videobuf operations
57868acc Satendra Singh Thakur 2017-12-18  167   */
57868acc Satendra Singh Thakur 2017-12-18  168  int dvb_vb2_init(struct 
dvb_vb2_ctx *ctx, const char *name, int nonblocking)
57868acc Satendra Singh Thakur 2017-12-18  169  {
57868acc Satendra Singh Thakur 2017-12-18  170  struct vb2_queue *q = 
>vb_q;
57868acc Satendra Singh Thakur 2017-12-18  171  int ret;
57868acc Satendra Singh Thakur 2017-12-18  172  
57868acc Satendra Singh Thakur 2017-12-18  173  memset(ctx, 0, 
sizeof(struct dvb_vb2_ctx));
57868acc Satendra Singh Thakur 2017-12-18  174  q->type = 
DVB_BUF_TYPE_CAPTURE;
57868acc Satendra Singh Thakur 2017-12-18  175  /**capture type*/
57868acc Satendra Singh Thakur 2017-12-18  176  q->is_output = 0;
57868acc Satendra Singh Thakur 2017-12-18  177  /**only mmap is 
supported currently*/
57868acc Satendra Singh Thakur 2017-12-18  178  q->io_modes = VB2_MMAP;
57868acc Satendra Singh Thakur 2017-12-18  179  q->drv_priv = ctx;
57868acc Satendra Singh Thakur 2017-12-18  180  q->buf_struct_size = 
sizeof(struct dvb_buffer);
57868acc Satendra Singh Thakur 2017-12-18  181  q->min_buffers_needed = 
1;
57868acc Satendra Singh Thakur 2017-12-18  182  q->ops = _vb2_qops;
57868acc Satendra Singh Thakur 2017-12-18 @183  q->mem_ops = 
_vmalloc_memops;
57868acc Satendra Singh Thakur 2017-12-18  184  q->buf_ops = 
_vb2_buf_ops;
57868acc Satendra Singh Thakur 2017-12-18  185  q->num_buffers = 0;
57868acc Satendra Singh Thakur 2017-12-18  186  ret = 
vb2_core_queue_init(q);
57868acc Satendra Singh Thakur 2017-12-18  187  if (ret) {
57868acc Satendra Singh Thakur 2017-12-18  188  ctx->state = 
DVB_VB2_STATE_NONE;
57868acc Satendra Singh Thakur 2017-12-18  189  dprintk(1, 
"[%s] errno=%d\n", ctx->name, ret);
57868acc Satendra Singh Thakur 2017-12-18  190  return ret;
57868acc Satendra Singh Thakur 2017-12-18  191  }
57868acc Satendra Singh Thakur 2017-12-18  192  
57868acc Satendra Singh Thakur 2017-12-18  193  mutex_init(>mutex);
57868acc Satendra Singh Thakur 2017-12-18  194  
spin_lock_init(>slock);
57868acc Satendra Singh Thakur 2017-12-18  195  
INIT_LIST_HEAD(>dvb_q);
57868acc Satendra Singh Thakur 2017-12-18  196  
e73f9f68 Mauro Carvalho Chehab 2017-12-29  197  strlcpy(ctx->name, 
name, DVB_VB2_NAME_MAX);
57868acc Satendra Singh Thakur 2017-12-18  198  ctx->nonblocking = 
nonblocking;
57868acc Satendra Singh Thakur 2017-12-18  199  ctx->state = 
DVB_VB2_STATE_INIT;
57868acc Satendra Singh Thakur 2017-12-18  200  
57868acc Satendra Singh Thakur 2017-12-18  201  dprintk(3, "[%s]\n", 
ctx->name);
57868acc Satendra Singh Thakur 2017-12-18  202  
57868acc Satendra Singh Thakur 2017-12-18  203  return 0;
57868acc Satendra Singh Thakur 2017-12-18  204  }
57868acc Satendra Singh Thakur 2017-12-18  205  

:: The code at line 183 was first introduced by commit
:: 57868acc369ab73ec8f6b43a0c6749077376b189 media: videobuf2: Add new uAPI 
for DVB streaming I/O

:: TO: Satendra Singh Thakur 
:: CC: Mauro Carvalho Chehab 

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


.config.gz
Description: application/gzip

Re: [PATCH 1/2] usbtv: Use same decoder sequence as Windows driver

[Hugo "Bonstra" Grostabussiat]

Le 26/02/2018 à 15:12, Hans Verkuil a écrit :
> Thanks for this patch, but I am a bit hesitant to apply it. Did you test
> that PAL and NTSC still work after this change?

I did test with both a PAL and a NTSC source before submitting the
patch. It seemed to work fine.

However, after reading your reply, I needed to be certain, so I
double-checked using the Windows driver USB dumps I collected.

When it sets the TV standard, the Windows driver:

  1. plays the decoder configuration sequence, in the exact same order
 as the one specified in the .INF file.
 -> My patch 1 does that, the unpatched driver doesn't

  2. sets registers 0xc24e and 0xc24f to value 0x02.
 -> The unpatched driver does this, my patch 1 doesn't, which is
a regression

  3. sets register 0xc16f to a different value for PAL, NTSC and
 SECAM.
 -> No write is ever done to that register by the patched
or the unpatched drivers

About step 3, I added it to the usbtv driver, and quickly understood
what register 0xc16f is for. It actually sets the standard for color
encoding!
It would seem that by default, the decoder auto-detects the standard
used to encode the color signal, and that writing a value to that
register forces selection of a specific standard.

So, with the current unpatched driver, capturing a PAL source while
setting the V4L2 norm to NTSC will still get the colors right (mostly),
but the image will be clipped at the bottom because of the lower
vertical resolution used for NTSC.

With the Windows driver or the usbtv driver patched to set the
norm in register 0xc16f, capturing a PAL source with the adapter
configured for NTSC would give the result you would expect from
misconfigured hardware: incorrect resolution and messed-up colors.

Here are some screenshots to illustrate the matter:

* Unpatched driver, PAL source, adapter configured for PAL.
  Picture is fully displayed, and colors are ok. Used as reference:

https://www.bonstra.fr.eu.org/pub/img/usbtv_unpatched-PAL_source-PAL_setting.png

* Unpatched driver, PAL source, adapter configured for NTSC.
  A part of the picture is cut at the bottom, colors are ok:

https://www.bonstra.fr.eu.org/pub/img/usbtv_unpatched-PAL_source-NTSC_setting.png

* Patched driver, PAL source, adapter configured for NTSC:
  Picture bottom is clipped, vertical stripes with the wrong colors
  are present over the colored areas:

https://www.bonstra.fr.eu.org/pub/img/usbtv_patched-PAL_source-NTSC_setting.png

Now about part 1; the sequence of register writes before actually
setting the color system register is there to set the image correction
parameters, such as color correction or image sharpness, appropriate
for the selected standard.

> Unless you've tested it then I'm inclined to just apply the second patch that
> adds the SECAM sequence.

Applying only patch 2 would get some values of the image correction
registers overwritten with PAL/NTSC values which were put in common
since they were identical (registers 0xc100, 0xc115, 0xc220, 0xc225 and
0xc24e).

I think I should make a v2 of this patch series which:
  1. fixes the mistakes I made in patch 1
  2. add SECAM image correction settings sequence
  3. writes the standard setting to register 0xc16f so that we get as
 closes as possible to the Windows driver behavior
  4. handles the PAL_60 case (NTSC resolution with PAL-like color
 subcarrier) which was working "by accident" until now

What do you think of all this?

Regards
--
Hugo

> On 02/24/2018 07:24 PM, Hugo Grostabussiat wrote:
>> Re-format the register {address, value} pairs so they follow the same
>> order as the decoder configuration sequences in the Windows driver's .INF
>> file.
>>
>> For instance, for PAL, the "AVPAL" sequence in the .INF file is:
>> 0x04,0x68,0xD3,0x72,0xA2,0xB0,0x15,0x01,0x2C,0x10,0x20,0x2e,0x08,0x02,
>> 0x02,0x59,0x16,0x35,0x17,0x16,0x36
>>
>> Signed-off-by: Hugo Grostabussiat 
>> ---
>>  drivers/media/usb/usbtv/usbtv-video.c | 26 +-
>>  1 file changed, 17 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/media/usb/usbtv/usbtv-video.c 
>> b/drivers/media/usb/usbtv/usbtv-video.c
>> index 3668a04359e8..52d06b30fabb 100644
>> --- a/drivers/media/usb/usbtv/usbtv-video.c
>> +++ b/drivers/media/usb/usbtv/usbtv-video.c
>> @@ -124,15 +124,26 @@ static int usbtv_select_input(struct usbtv *usbtv, int 
>> input)
>>  static int usbtv_select_norm(struct usbtv *usbtv, v4l2_std_id norm)
>>  {
>>  int ret;
>> +/* These are the series of register values used to configure the
>> + * decoder for a specific standard.
>> + * They are copied from the Settings\DecoderDefaults registry keys
>> + * present in the Windows driver .INF file for each norm.
>> + */
>>  static const u16 pal[][2] = {
>> +{ USBTV_BASE + 0x0003, 0x0004 },
>>  { USBTV_BASE + 0x001a, 0x0068 },
>> +{ USBTV_BASE + 0x0100, 0x00d3 },
>>  { 

Re: [v5 2/2] media: dt-bindings: Add bindings for Dongwoon DW9807 voice coil

[Rob Herring]

On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh  wrote:
> From: Alan Chiang 
>
> Dongwoon DW9807 is a voice coil lens driver.
>
> Also add a vendor prefix for Dongwoon for one did not exist previously.

Where's that?

>
> Signed-off-by: Andy Yeh 
> ---
>  Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 +

DACs generally go in bindings/iio/dac/

>  1 file changed, 9 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt 
> b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
> new file mode 100644
> index 000..0a1a860
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt
> @@ -0,0 +1,9 @@
> +Dongwoon Anatech DW9807 voice coil lens driver
> +
> +DW9807 is a 10-bit DAC with current sink capability. It is intended for
> +controlling voice coil lenses.
> +
> +Mandatory properties:
> +
> +- compatible: "dongwoon,dw9807"
> +- reg: I2C slave address
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

With All Due Respect!!

[Mrs.Louisa Benicio]

Greetings,

I am Mrs. Louisa Benicio; I have decided to donate what I have to
You/Churches/ Motherless babies/Less privileged/Widows’ because I am
dying and diagnosed for cancer for about 2 years ago. I have been
touched by God Almighty to donate from what I have inherited from my
late husband to you for good work of God. I have asked Almighty God to
forgive me and believe he has because he is a Merciful God I will be
going in for an operation soon.

I decided to will/donate the sum of $5.5 million dollars to you for
the good work of God, and also to help the motherless and less
privilege and also forth assistance of the widows. At the moment I
cannot take any telephone calls right now due to the fact that my
relatives (that have squandered the funds gave them for this purpose
before) are around me and my health status also. I have adjusted my
will and my lawyer is aware.

I wish you all the best and May the good Lord bless you abundantly,
and please use the funds judiciously and always extend the good work
to others. As soon you get back to me, I shall give you information on
what I need from you then you will contact the bank and tell them I
have willed those properties to you by quoting my personal file
routing and account information. And I have also notified the bank
that I am willing that properties to you for a good, effective and
prudent work. I know I don't know you but I have been directed to do
this by God Almighty.

If you are interested in carrying out this task please contact my
private email at ( mrslbeni...@mail.com ) for more details on this
noble project of mine.

Yours faithfully,
Mrs. Louisa Benicio.

[PATCH RFC] media: em28xx: don't use coherent buffer for DMA transfers

[Mauro Carvalho Chehab]

While coherent memory is cheap on x86, it has problems on
arm. So, stop using it.

Signed-off-by: Mauro Carvalho Chehab 
---

I wrote this patch in order to check if this would make things better
for ISOCH transfers on Raspberry Pi3. It didn't. Yet, keep using
coherent memory at USB drivers seem an overkill.

So, I'm actually not sure if we should either go ahead and merge it
or not.

Comments? Tests?

 drivers/media/usb/em28xx/em28xx-core.c | 49 +++---
 drivers/media/usb/em28xx/em28xx.h  |  2 +-
 2 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-core.c 
b/drivers/media/usb/em28xx/em28xx-core.c
index 1d0d8cc06103..6fadcb03093f 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -800,7 +800,6 @@ void em28xx_uninit_usb_xfer(struct em28xx *dev, enum 
em28xx_mode mode)
 {
struct urb *urb;
struct em28xx_usb_bufs *usb_bufs;
-   struct usb_device *udev = interface_to_usbdev(dev->intf);
int i;
 
em28xx_isocdbg("em28xx: called em28xx_uninit_usb_xfer in mode %d\n",
@@ -819,23 +818,16 @@ void em28xx_uninit_usb_xfer(struct em28xx *dev, enum 
em28xx_mode mode)
else
usb_unlink_urb(urb);
 
-   if (usb_bufs->transfer_buffer[i]) {
-   usb_free_coherent(udev,
- urb->transfer_buffer_length,
- usb_bufs->transfer_buffer[i],
- urb->transfer_dma);
-   }
usb_free_urb(urb);
usb_bufs->urb[i] = NULL;
}
-   usb_bufs->transfer_buffer[i] = NULL;
}
 
kfree(usb_bufs->urb);
-   kfree(usb_bufs->transfer_buffer);
+   kfree(usb_bufs->buf);
 
usb_bufs->urb = NULL;
-   usb_bufs->transfer_buffer = NULL;
+   usb_bufs->buf = NULL;
usb_bufs->num_bufs = 0;
 
em28xx_capture_start(dev, 0);
@@ -912,14 +904,13 @@ int em28xx_alloc_urbs(struct em28xx *dev, enum 
em28xx_mode mode, int xfer_bulk,
 
usb_bufs->num_bufs = num_bufs;
 
-   usb_bufs->urb = kzalloc(sizeof(void *)*num_bufs,  GFP_KERNEL);
+   usb_bufs->urb = kcalloc(sizeof(void *), num_bufs,  GFP_KERNEL);
if (!usb_bufs->urb)
return -ENOMEM;
 
-   usb_bufs->transfer_buffer = kzalloc(sizeof(void *)*num_bufs,
-GFP_KERNEL);
-   if (!usb_bufs->transfer_buffer) {
-   kfree(usb_bufs->urb);
+   usb_bufs->buf = kcalloc(sizeof(void *), num_bufs, GFP_KERNEL);
+   if (!usb_bufs->buf) {
+   kfree(usb_bufs->buf);
return -ENOMEM;
}
 
@@ -942,37 +933,39 @@ int em28xx_alloc_urbs(struct em28xx *dev, enum 
em28xx_mode mode, int xfer_bulk,
}
usb_bufs->urb[i] = urb;
 
-   usb_bufs->transfer_buffer[i] = usb_alloc_coherent(udev,
-   sb_size, GFP_KERNEL, >transfer_dma);
-   if (!usb_bufs->transfer_buffer[i]) {
+   usb_bufs->buf[i] = kzalloc(sb_size, GFP_KERNEL);
+   if (!usb_bufs->buf[i]) {
dev_err(>intf->dev,
"unable to allocate %i bytes for transfer 
buffer %i%s\n",
   sb_size, i,
   in_interrupt() ? " while in int" : "");
em28xx_uninit_usb_xfer(dev, mode);
+
+   while (i) {
+   kfree(usb_bufs->buf[i]);
+   i--;
+   }
+   kfree(usb_bufs->buf);
+
return -ENOMEM;
}
-   memset(usb_bufs->transfer_buffer[i], 0, sb_size);
 
if (xfer_bulk) { /* bulk */
pipe = usb_rcvbulkpipe(udev,
   mode == EM28XX_ANALOG_MODE ?
   dev->analog_ep_bulk :
   dev->dvb_ep_bulk);
-   usb_fill_bulk_urb(urb, udev, pipe,
- usb_bufs->transfer_buffer[i], sb_size,
- em28xx_irq_callback, dev);
-   urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;
+   usb_fill_bulk_urb(urb, udev, pipe, usb_bufs->buf[i],
+ sb_size, em28xx_irq_callback, dev);
+   urb->transfer_flags = URB_FREE_BUFFER;
} else { /* isoc */
pipe = usb_rcvisocpipe(udev,
   mode == EM28XX_ANALOG_MODE ?
   

[PATCH] media: platform: renesas-ceu: Fix CSTRST_CPON mask

[Jacopo Mondi]

The CSTRST_CPON mask was wrongly assigned to BIT(1) instead of BIT(0).
Fix that by changing the mask opportunely.

Reported-by: Dylan Laduranty 
Signed-off-by: Jacopo Mondi 

---
Mauro: could you please pick up this patch since you already applied the
CEU series to your tree if I'm not wrong?

Laurent, in this email exchange:
https://www.mail-archive.com/linux-media@vger.kernel.org/msg123779.html
you asked me to measure the number of cycles required to fully reset the
interface in order to quantify the proper delay loops. I was testing this
using the wrong bit, and I always got 0, and I assumed 1usec was enough.
Good news is that I re-tested this on SH and RZ and I still have 0, so
no driver change is required apart from the bitmask one \o/

---
 drivers/media/platform/renesas-ceu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas-ceu.c 
b/drivers/media/platform/renesas-ceu.c
index cfabe1a..c7d3659 100644
--- a/drivers/media/platform/renesas-ceu.c
+++ b/drivers/media/platform/renesas-ceu.c
@@ -95,7 +95,7 @@

 /* CEU operating flag bit. */
 #define CEU_CAPCR_CTNCPBIT(16)
-#define CEU_CSTRST_CPTON   BIT(1)
+#define CEU_CSTRST_CPTON   BIT(0)

 /* Platform specific IRQ source flags. */
 #define CEU_CETCR_ALL_IRQS_RZ  0x397f313
--
2.7.4

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Claudiu Beznea]

On 27.02.2018 17:38, Daniel Thompson wrote:
> On Tue, Feb 27, 2018 at 01:40:58PM +0200, Claudiu Beznea wrote:
>> On 27.02.2018 12:54, Daniel Thompson wrote:
>>> On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
 On 26.02.2018 11:57, Jani Nikula wrote:
> On Thu, 22 Feb 2018, Daniel Thompson  wrote:
>> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
>>> Add PWM mode to pwm_config() function. The drivers which uses 
>>> pwm_config()
>>> were adapted to this change.
>>>
>>> Signed-off-by: Claudiu Beznea 
>>> ---
>>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
>>>  drivers/bus/ts-nbus.c|  2 +-
>>>  drivers/clk/clk-pwm.c|  3 ++-
>>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
>>>  drivers/hwmon/pwm-fan.c  |  2 +-
>>>  drivers/input/misc/max77693-haptic.c |  2 +-
>>>  drivers/input/misc/max8997_haptic.c  |  6 +-
>>>  drivers/leds/leds-pwm.c  |  5 -
>>>  drivers/media/rc/ir-rx51.c   |  5 -
>>>  drivers/media/rc/pwm-ir-tx.c |  5 -
>>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>>>  drivers/video/backlight/lp8788_bl.c  |  5 -
>>>  drivers/video/backlight/pwm_bl.c | 11 +--
>>>  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
>>>  include/linux/pwm.h  |  6 --
>>>  16 files changed, 70 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/video/backlight/lm3630a_bl.c 
>>> b/drivers/video/backlight/lm3630a_bl.c
>>> index 2030a6b77a09..696fa25dafd2 100644
>>> --- a/drivers/video/backlight/lm3630a_bl.c
>>> +++ b/drivers/video/backlight/lm3630a_bl.c
>>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip 
>>> *pchip, int br, int br_max)
>>>  {
>>> unsigned int period = pchip->pdata->pwm_period;
>>> unsigned int duty = br * period / br_max;
>>> +   struct pwm_caps caps = { };
>>>  
>>> -   pwm_config(pchip->pwmd, duty, period);
>>> +   pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, );
>>> +   pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
>>
>> Well... I admit I've only really looked at the patches that impact 
>> backlight but dispersing this really odd looking bit twiddling 
>> throughout the kernel doesn't strike me a great API design.
>>
>> IMHO callers should not be required to find the first set bit in
>> some specially crafted set of capability bits simply to get sane 
>> default behaviour.
>
> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
> error prone.

 Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be 
 OK
 from your side?

 Or, what about using a function like pwm_mode_first() to get the first 
 supported
 mode by PWM channel?

 Or, would you prefer to solve this inside pwm_config() function, let's 
 say, in
 case an invalid mode is passed as argument, to let pwm_config() to choose 
 the
 first available PWM mode for PWM channel passed as argument?
>>>
>>> What is it that actually needs solving?
>>>
>>> If a driver requests normal mode and the PWM driver cannot support it
>>> why not just return an error an move on.
>> Because, simply, I wasn't aware of what these PWM client drivers needs for.
> 
> I'm afraid you have confused me here.
> 
> Didn't you just *add* the whole concept of PWM caps with your patches?
> How could any existing call site expect anything except normal mode.
> Until now there has been no possiblity to request anything else.
Agree. And agree I was confusing in previous email, sorry about that. And
agree that there was nothing before and everything should work with PWM
normal mode.

When I choose to have BIT(ffs(caps.modes)) instead of PWM_MODE(NORMAL) I
was thinking at having these pwm_config() calls working all the time having
in mind that in future the PWM controllers that these drivers use, might
change in terms of PWM supported modes.

Thank you,
Claudiu Beznea

> 
> 
>>> Put another way, what is the use case for secretly adopting a mode the
>>> caller didn't want? Under what circumstances is this a good thing?
>> No one... But I wasn't aware of what the PWM clients needs for from their PWM
>> controllers. At this moment having BIT(ffs(caps.modes)) instead of
>> PWM_MODE(NORMAL) is mostly the same since all the driver that has not 
>> explicitly
>> registered PWM caps will use PWM normal mode.
>>
>> I will use PWM_MODE(NORMAL) instead of this in all the cases if this is OK 
>> from
>> your side.
>>
>> Thank you,
>> Claudiu Beznea
>>>
>>>
>>> Daniel.
>>>
> 

[PATCH 12/13] media: ov772x: Re-order variables declaration

[Jacopo Mondi]

Re-order variables declaration to respect 'reverse christmas tree'
ordering whenever possible.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/ov772x.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 4f464ac..1fd6d4b 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1098,8 +1098,8 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
  struct v4l2_subdev_format *format)
 {
-   struct ov772x_priv *priv = to_ov772x(sd);
struct v4l2_mbus_framefmt *mf = >format;
+   struct ov772x_priv *priv = to_ov772x(sd);
const struct ov772x_color_format *cfmt;
const struct ov772x_win_size *win;
int ret;
@@ -1135,10 +1135,11 @@ 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;
-   const char *devname;
-   int ret;
+   struct i2c_client *client = v4l2_get_subdevdata(>subdev);
+   const char *devname;
+   int ret;
+   u8 pid;
+   u8 ver;
 
ret = ov772x_s_power(>subdev, 1);
if (ret < 0)
@@ -1246,9 +1247,9 @@ static const struct v4l2_subdev_ops ov772x_subdev_ops = {
 static int ov772x_probe(struct i2c_client *client,
const struct i2c_device_id *did)
 {
-   struct ov772x_priv  *priv;
-   struct i2c_adapter  *adapter = client->adapter;
-   int ret;
+   struct i2c_adapter *adapter = client->adapter;
+   struct ov772x_priv *priv;
+   int ret;
 
if (!client->dev.platform_data) {
dev_err(>dev, "Missing ov772x platform data\n");
-- 
2.7.4

[PATCH 11/13] media: ov772x: Empty line before end-of-function return

[Jacopo Mondi]

Add an empty line before return at the end of functions.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/ov772x.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 8849da1..4f464ac 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1129,6 +1129,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
 
priv->win = win;
priv->cfmt = cfmt;
+
return 0;
 }
 
@@ -1172,6 +1173,7 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
 
 done:
ov772x_s_power(>subdev, 0);
+
return ret;
 }
 
@@ -1213,6 +1215,7 @@ static int ov772x_enum_mbus_code(struct v4l2_subdev *sd,
return -EINVAL;
 
code->code = ov772x_cfmts[code->index].code;
+
return 0;
 }
 
@@ -1327,6 +1330,7 @@ static int ov772x_remove(struct i2c_client *client)
gpiod_put(priv->pwdn_gpio);
v4l2_device_unregister_subdev(>subdev);
v4l2_ctrl_handler_free(>hdl);
+
return 0;
 }
 
-- 
2.7.4

[PATCH 05/13] media: tw9910: Re-organize in-code comments

[Jacopo Mondi]

A lot of comments that would fit a single line were spread on two or
more lines. Also fix capitalization and punctuation where appropriate.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/tw9910.c | 44 +---
 1 file changed, 13 insertions(+), 31 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index f88cc93..f082f6d 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -388,7 +388,7 @@ static int tw9910_set_hsync(struct i2c_client *client)
if (ret < 0)
return ret;
 
-   /* So far only revisions 0 and 1 have been seen */
+   /* So far only revisions 0 and 1 have been seen. */
/* bit 2 - 0 */
if (priv->revision == 1)
ret = tw9910_mask_set(client, HSLOWCTL, 0x77,
@@ -653,21 +653,15 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32 
*width, u32 *height)
int ret = -EINVAL;
u8 val;
 
-   /*
-* select suitable norm
-*/
+   /* Select suitable norm. */
priv->scale = tw9910_select_norm(priv->norm, *width, *height);
if (!priv->scale)
goto tw9910_set_fmt_error;
 
-   /*
-* reset hardware
-*/
+   /* Reset hardware. */
tw9910_reset(client);
 
-   /*
-* set bus width
-*/
+   /* Set bus width. */
val = 0x00;
if (priv->info->buswidth == 16)
val = LEN;
@@ -676,9 +670,7 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32 
*width, u32 *height)
if (ret < 0)
goto tw9910_set_fmt_error;
 
-   /*
-* select MPOUT behavior
-*/
+   /* Select MPOUT behavior. */
switch (priv->info->mpout) {
case TW9910_MPO_VLOSS:
val = RTSEL_VLOSS; break;
@@ -704,16 +696,12 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32 
*width, u32 *height)
if (ret < 0)
goto tw9910_set_fmt_error;
 
-   /*
-* set scale
-*/
+   /* Set scale. */
ret = tw9910_set_scale(client, priv->scale);
if (ret < 0)
goto tw9910_set_fmt_error;
 
-   /*
-* set hsync
-*/
+   /* Set hsync. */
ret = tw9910_set_hsync(client);
if (ret < 0)
goto tw9910_set_fmt_error;
@@ -740,7 +728,7 @@ static int tw9910_get_selection(struct v4l2_subdev *sd,
 
if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
return -EINVAL;
-   /* Only CROP, CROP_DEFAULT and CROP_BOUNDS are supported */
+   /* Only CROP, CROP_DEFAULT and CROP_BOUNDS are supported. */
if (sel->target > V4L2_SEL_TGT_CROP_BOUNDS)
return -EINVAL;
 
@@ -792,9 +780,7 @@ static int tw9910_s_fmt(struct v4l2_subdev *sd,
WARN_ON(mf->field != V4L2_FIELD_ANY &&
mf->field != V4L2_FIELD_INTERLACED_BT);
 
-   /*
-* check color format
-*/
+   /* Check color format. */
if (mf->code != MEDIA_BUS_FMT_UYVY8_2X8)
return -EINVAL;
 
@@ -831,9 +817,7 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
mf->colorspace = V4L2_COLORSPACE_SMPTE170M;
 
-   /*
-* select suitable norm
-*/
+   /* Select suitable norm. */
scale = tw9910_select_norm(priv->norm, mf->width, mf->height);
if (!scale)
return -EINVAL;
@@ -854,9 +838,7 @@ static int tw9910_video_probe(struct i2c_client *client)
int ret;
s32 id;
 
-   /*
-* tw9910 only use 8 or 16 bit bus width
-*/
+   /* TW9910 only use 8 or 16 bit bus width. */
if (priv->info->buswidth != 16 && priv->info->buswidth != 8) {
dev_err(>dev, "bus width error\n");
return -ENODEV;
@@ -867,8 +849,8 @@ static int tw9910_video_probe(struct i2c_client *client)
return ret;
 
/*
-* check and show Product ID
-* So far only revisions 0 and 1 have been seen
+* Check and show Product ID.
+* So far only revisions 0 and 1 have been seen.
 */
id = i2c_smbus_read_byte_data(client, ID);
priv->revision = GET_REV(id);
-- 
2.7.4

[PATCH 13/13] media: ov772x: Replace msleep(1) with usleep_range

[Jacopo Mondi]

msleep() can sleep up to 20ms.

As suggested by Documentation/timers/timers_howto.txt replace it with
usleep_range() with up to 5ms delay.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/ov772x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 1fd6d4b..2d5281a 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -574,7 +574,7 @@ static int ov772x_reset(struct i2c_client *client)
if (ret < 0)
return ret;
 
-   msleep(1);
+   usleep_range(1000, 5000);
 
return ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, SOFT_SLEEP_MODE);
 }
-- 
2.7.4

[PATCH 06/13] media: tw9910: Mixed style fixes

[Jacopo Mondi]

Two minor style fixes, align function parameter and remove un-necessary
spaces.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/tw9910.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index f082f6d..f9b75c1 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -445,7 +445,7 @@ static const struct tw9910_scale_ctrl 
*tw9910_select_norm(v4l2_std_id norm,
 
for (i = 0; i < size; i++) {
tmp = abs(width - scale[i].width) +
-   abs(height - scale[i].height);
+ abs(height - scale[i].height);
if (tmp < diff) {
diff = tmp;
ret = scale + i;
@@ -953,7 +953,7 @@ static int tw9910_probe(struct i2c_client *client,
if (!priv)
return -ENOMEM;
 
-   priv->info   = info;
+   priv->info = info;
 
v4l2_i2c_subdev_init(>subdev, client, _subdev_ops);
 
-- 
2.7.4

[PATCH 02/13] media: tw9910: Empty line before end-of-function return

[Jacopo Mondi]

Add an empty line before return at the end of functions.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/tw9910.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 70e0ae2..3e4b530 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -753,6 +753,7 @@ static int tw9910_get_selection(struct v4l2_subdev *sd,
sel->r.width= 768;
sel->r.height   = 576;
}
+
return 0;
 }
 
@@ -804,6 +805,7 @@ static int tw9910_s_fmt(struct v4l2_subdev *sd,
mf->width   = width;
mf->height  = height;
}
+
return ret;
 }
 
@@ -842,6 +844,7 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
if (format->which == V4L2_SUBDEV_FORMAT_ACTIVE)
return tw9910_s_fmt(sd, mf);
cfg->try_fmt = *mf;
+
return 0;
 }
 
@@ -887,6 +890,7 @@ static int tw9910_video_probe(struct i2c_client *client)
 
 done:
tw9910_s_power(>subdev, 0);
+
return ret;
 }
 
@@ -906,12 +910,14 @@ static int tw9910_enum_mbus_code(struct v4l2_subdev *sd,
return -EINVAL;
 
code->code = MEDIA_BUS_FMT_UYVY8_2X8;
+
return 0;
 }
 
 static int tw9910_g_tvnorms(struct v4l2_subdev *sd, v4l2_std_id *norm)
 {
*norm = V4L2_STD_NTSC | V4L2_STD_PAL;
+
return 0;
 }
 
-- 
2.7.4

[PATCH 04/13] media: tw9910: Re-order variables declaration

[Jacopo Mondi]

Re-order variables declaration to respect 'reverse christmas tree'
ordering whenever possible.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/tw9910.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 45afdb9..f88cc93 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client)
 
 static int tw9910_power(struct i2c_client *client, int enable)
 {
-   int ret;
u8 acntl1;
u8 acntl2;
+   int ret;
 
if (enable) {
acntl1 = 0;
@@ -428,8 +428,8 @@ static int tw9910_power(struct i2c_client *client, int 
enable)
 static const struct tw9910_scale_ctrl *tw9910_select_norm(v4l2_std_id norm,
  u32 width, u32 height)
 {
-   const struct tw9910_scale_ctrl *scale;
const struct tw9910_scale_ctrl *ret = NULL;
+   const struct tw9910_scale_ctrl *scale;
__u32 diff = 0x, tmp;
int size, i;
 
@@ -462,8 +462,8 @@ static int tw9910_s_stream(struct v4l2_subdev *sd, int 
enable)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct tw9910_priv *priv = to_tw9910(client);
-   u8 val;
int ret;
+   u8 val;
 
if (!enable) {
switch (priv->revision) {
@@ -512,10 +512,10 @@ static int tw9910_s_std(struct v4l2_subdev *sd, 
v4l2_std_id norm)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct tw9910_priv *priv = to_tw9910(client);
-   const unsigned int hact = 720;
const unsigned int hdelay = 15;
-   unsigned int vact;
+   const unsigned int hact = 720;
unsigned int vdelay;
+   unsigned int vact;
int ret;
 
if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL)))
@@ -761,8 +761,8 @@ static int tw9910_get_fmt(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
  struct v4l2_subdev_format *format)
 {
-   struct v4l2_mbus_framefmt *mf = >format;
struct i2c_client *client = v4l2_get_subdevdata(sd);
+   struct v4l2_mbus_framefmt *mf = >format;
struct tw9910_priv *priv = to_tw9910(client);
 
if (format->pad)
@@ -813,8 +813,8 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
  struct v4l2_subdev_format *format)
 {
-   struct v4l2_mbus_framefmt *mf = >format;
struct i2c_client *client = v4l2_get_subdevdata(sd);
+   struct v4l2_mbus_framefmt *mf = >format;
struct tw9910_priv *priv = to_tw9910(client);
const struct tw9910_scale_ctrl *scale;
 
@@ -851,8 +851,8 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
 static int tw9910_video_probe(struct i2c_client *client)
 {
struct tw9910_priv *priv = to_tw9910(client);
-   s32 id;
int ret;
+   s32 id;
 
/*
 * tw9910 only use 8 or 16 bit bus width
@@ -949,10 +949,9 @@ static int tw9910_probe(struct i2c_client *client,
const struct i2c_device_id *did)
 
 {
-   struct tw9910_priv  *priv;
-   struct tw9910_video_info*info;
-   struct i2c_adapter  *adapter =
-   to_i2c_adapter(client->dev.parent);
+   struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+   struct tw9910_video_info *info;
+   struct tw9910_priv *priv;
int ret;
 
if (!client->dev.platform_data) {
-- 
2.7.4

[PATCH 01/13] media: tw9910: Fix parameter alignment issue

[Jacopo Mondi]

Align parameters to first open brace.

Signed-off-by: Jacopo Mondi 

---
 drivers/media/i2c/tw9910.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index cc5d383..70e0ae2 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -533,10 +533,10 @@ static int tw9910_s_std(struct v4l2_subdev *sd, 
v4l2_std_id norm)
}
if (!ret)
ret = i2c_smbus_write_byte_data(client, CROP_HI,
-   ((vdelay >> 2) & 0xc0) |
-   ((vact >> 4) & 0x30) |
-   ((hdelay >> 6) & 0x0c) |
-   ((hact >> 8) & 0x03));
+   ((vdelay >> 2) & 0xc0)  |
+   ((vact >> 4) & 0x30)|
+   ((hdelay >> 6) & 0x0c)  |
+   ((hact >> 8) & 0x03));
if (!ret)
ret = i2c_smbus_write_byte_data(client, VDELAY_LO,
vdelay & 0xff);
--
2.7.4

[PATCH 03/13] media: tw9910: Align function parameters

[Jacopo Mondi]

Align all function parameters to first open brace when declaring
functions.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/tw9910.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 3e4b530..45afdb9 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -733,7 +733,7 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32 
*width, u32 *height)
 
 static int tw9910_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_selection *sel)
+   struct v4l2_subdev_selection *sel)
 {
struct i2c_client *client = v4l2_get_subdevdata(sd);
struct tw9910_priv *priv = to_tw9910(client);
@@ -759,7 +759,7 @@ static int tw9910_get_selection(struct v4l2_subdev *sd,
 
 static int tw9910_get_fmt(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_format *format)
+ struct v4l2_subdev_format *format)
 {
struct v4l2_mbus_framefmt *mf = >format;
struct i2c_client *client = v4l2_get_subdevdata(sd);
@@ -811,7 +811,7 @@ static int tw9910_s_fmt(struct v4l2_subdev *sd,
 
 static int tw9910_set_fmt(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_format *format)
+ struct v4l2_subdev_format *format)
 {
struct v4l2_mbus_framefmt *mf = >format;
struct i2c_client *client = v4l2_get_subdevdata(sd);
@@ -904,7 +904,7 @@ static const struct v4l2_subdev_core_ops 
tw9910_subdev_core_ops = {
 
 static int tw9910_enum_mbus_code(struct v4l2_subdev *sd,
 struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_mbus_code_enum *code)
+struct v4l2_subdev_mbus_code_enum *code)
 {
if (code->pad || code->index)
return -EINVAL;
-- 
2.7.4

[PATCH 08/13] media: tw9910: Replace msleep(1) with usleep_range

[Jacopo Mondi]

msleep() can sleep up to 20ms.

As suggested by Documentation/timers/timers_howto.txt replace it with
usleep_range() with up to 5ms delay.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/tw9910.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 1043f7d..920877a 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -401,7 +401,7 @@ static int tw9910_set_hsync(struct i2c_client *client)
 static void tw9910_reset(struct i2c_client *client)
 {
tw9910_mask_set(client, ACNTL1, SRESET, SRESET);
-   msleep(1);
+   usleep_range(1000, 5000);
 }
 
 static int tw9910_power(struct i2c_client *client, int enable)
-- 
2.7.4

[PATCH 09/13] media: ov772x: Align function parameters

[Jacopo Mondi]

Align all function parameters to first open brace when declaring
functions.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/ov772x.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 16665af..a418455 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1064,7 +1064,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 
 static int ov772x_get_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_selection *sel)
+   struct v4l2_subdev_selection *sel)
 {
struct ov772x_priv *priv = to_ov772x(sd);
 
@@ -1087,7 +1087,7 @@ static int ov772x_get_selection(struct v4l2_subdev *sd,
 
 static int ov772x_get_fmt(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_format *format)
+ struct v4l2_subdev_format *format)
 {
struct v4l2_mbus_framefmt *mf = >format;
struct ov772x_priv *priv = to_ov772x(sd);
@@ -1106,7 +1106,7 @@ static int ov772x_get_fmt(struct v4l2_subdev *sd,
 
 static int ov772x_set_fmt(struct v4l2_subdev *sd,
  struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_format *format)
+ struct v4l2_subdev_format *format)
 {
struct ov772x_priv *priv = to_ov772x(sd);
struct v4l2_mbus_framefmt *mf = >format;
@@ -1219,7 +1219,7 @@ static int ov772x_enum_frame_interval(struct v4l2_subdev 
*sd,
 
 static int ov772x_enum_mbus_code(struct v4l2_subdev *sd,
 struct v4l2_subdev_pad_config *cfg,
-   struct v4l2_subdev_mbus_code_enum *code)
+struct v4l2_subdev_mbus_code_enum *code)
 {
if (code->pad || code->index >= ARRAY_SIZE(ov772x_cfmts))
return -EINVAL;
-- 
2.7.4

[PATCH 07/13] media: tw9910: Sort includes alphabetically

[Jacopo Mondi]

Sort include directives alphabetically to ease maintenance.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/tw9910.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index f9b75c1..1043f7d 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -16,13 +16,13 @@
  */
 
 #include 
+#include 
 #include 
+#include 
 #include 
+#include 
 #include 
-#include 
 #include 
-#include 
-#include 
 #include 
 #include 
 
-- 
2.7.4

[PATCH 10/13] media: ov772x: Re-organize in-code comments

[Jacopo Mondi]

A lot of comments that would fit a single line were spread on two or
more lines. Also fix capitalization and punctuation where appropriate.

Signed-off-by: Jacopo Mondi 
---
 drivers/media/i2c/ov772x.c | 32 ++--
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index a418455..8849da1 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -910,17 +910,13 @@ static int ov772x_set_params(struct ov772x_priv *priv,
int ret;
u8  val;
 
-   /*
-* reset hardware
-*/
+   /* Reset hardware. */
ov772x_reset(client);
 
-   /*
-* Edge Ctrl
-*/
+   /* Edge Ctrl. */
if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) {
/*
-* Manual Edge Control Mode
+* Manual Edge Control Mode.
 *
 * Edge auto strength bit is set by default.
 * Remove it when manual mode.
@@ -944,9 +940,9 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 
} else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) {
/*
-* Auto Edge Control Mode
+* Auto Edge Control Mode.
 *
-* set upper and lower limit
+* Set upper and lower limit.
 */
ret = ov772x_mask_set(client,
  EDGE_UPPER, OV772X_EDGE_UPPER_MASK,
@@ -961,7 +957,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
goto ov772x_set_fmt_error;
}
 
-   /* Format and window size */
+   /* Format and window size. */
ret = ov772x_write(client, HSTART, win->rect.left >> 2);
if (ret < 0)
goto ov772x_set_fmt_error;
@@ -993,9 +989,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
if (ret < 0)
goto ov772x_set_fmt_error;
 
-   /*
-* set DSP_CTRL3
-*/
+   /* Set DSP_CTRL3. */
val = cfmt->dsp3;
if (val) {
ret = ov772x_mask_set(client,
@@ -1011,9 +1005,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
goto ov772x_set_fmt_error;
}
 
-   /*
-* set COM3
-*/
+   /* Set COM3. */
val = cfmt->com3;
if (priv->info->flags & OV772X_FLAG_VFLIP)
val |= VFLIP_IMG;
@@ -1041,9 +1033,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
if (ret < 0)
goto ov772x_set_fmt_error;
 
-   /*
-* set COM8
-*/
+   /* Set COM8. */
if (priv->band_filter) {
ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
if (!ret)
@@ -1153,9 +1143,7 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
if (ret < 0)
return ret;
 
-   /*
-* check and show product ID and manufacturer ID
-*/
+   /* Check and show product ID and manufacturer ID. */
pid = ov772x_read(client, PID);
ver = ov772x_read(client, VER);
 
-- 
2.7.4

[PATCH 00/13] media: ov772x/tw9910 cleanup

[Jacopo Mondi]

Hi Mauro,
  as you have started cleaning up those two drivers as they've been now
moved away from soc_camera I have added a few style fixes for both of them
on top of your two patches:

commit ae24b8a1d5f9 ("media: tw9910: solve coding style issues")
commit 2d595d14fe8b ("media: ov772x: fix whitespace issues")

checkpatch now returns no error apart from a > 80 columns in ov772x I did not
break for sake of readability.

Thanks
   j

Jacopo Mondi (13):
  media: tw9910: Fix parameter alignment issue
  media: tw9910: Empty line before end-of-function return
  media: tw9910: Align function parameters
  media: tw9910: Re-order variables declaration
  media: tw9910: Re-organize in-code comments
  media: tw9910: Mixed style fixes
  media: tw9910: Sort includes alphabetically
  media: tw9910: Replace msleep(1) with usleep_range
  media: ov772x: Align function parameters
  media: ov772x: Re-organize in-code comments
  media: ov772x: Empty line before end-of-function return
  media: ov772x: Re-order variables declaration
  media: ov772x: Replace msleep(1) with usleep_range

 drivers/media/i2c/ov772x.c |  63 +---
 drivers/media/i2c/tw9910.c | 101 -
 2 files changed, 72 insertions(+), 92 deletions(-)

--
2.7.4

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Daniel Thompson]

On Tue, Feb 27, 2018 at 01:40:58PM +0200, Claudiu Beznea wrote:
> On 27.02.2018 12:54, Daniel Thompson wrote:
> > On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
> >> On 26.02.2018 11:57, Jani Nikula wrote:
> >>> On Thu, 22 Feb 2018, Daniel Thompson  wrote:
>  On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> > Add PWM mode to pwm_config() function. The drivers which uses 
> > pwm_config()
> > were adapted to this change.
> >
> > Signed-off-by: Claudiu Beznea 
> > ---
> >  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
> >  drivers/bus/ts-nbus.c|  2 +-
> >  drivers/clk/clk-pwm.c|  3 ++-
> >  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
> >  drivers/hwmon/pwm-fan.c  |  2 +-
> >  drivers/input/misc/max77693-haptic.c |  2 +-
> >  drivers/input/misc/max8997_haptic.c  |  6 +-
> >  drivers/leds/leds-pwm.c  |  5 -
> >  drivers/media/rc/ir-rx51.c   |  5 -
> >  drivers/media/rc/pwm-ir-tx.c |  5 -
> >  drivers/video/backlight/lm3630a_bl.c |  4 +++-
> >  drivers/video/backlight/lp855x_bl.c  |  4 +++-
> >  drivers/video/backlight/lp8788_bl.c  |  5 -
> >  drivers/video/backlight/pwm_bl.c | 11 +--
> >  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
> >  include/linux/pwm.h  |  6 --
> >  16 files changed, 70 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/video/backlight/lm3630a_bl.c 
> > b/drivers/video/backlight/lm3630a_bl.c
> > index 2030a6b77a09..696fa25dafd2 100644
> > --- a/drivers/video/backlight/lm3630a_bl.c
> > +++ b/drivers/video/backlight/lm3630a_bl.c
> > @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip 
> > *pchip, int br, int br_max)
> >  {
> > unsigned int period = pchip->pdata->pwm_period;
> > unsigned int duty = br * period / br_max;
> > +   struct pwm_caps caps = { };
> >  
> > -   pwm_config(pchip->pwmd, duty, period);
> > +   pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, );
> > +   pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
> 
>  Well... I admit I've only really looked at the patches that impact 
>  backlight but dispersing this really odd looking bit twiddling 
>  throughout the kernel doesn't strike me a great API design.
> 
>  IMHO callers should not be required to find the first set bit in
>  some specially crafted set of capability bits simply to get sane 
>  default behaviour.
> >>>
> >>> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
> >>> error prone.
> >>
> >> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be 
> >> OK
> >> from your side?
> >>
> >> Or, what about using a function like pwm_mode_first() to get the first 
> >> supported
> >> mode by PWM channel?
> >>
> >> Or, would you prefer to solve this inside pwm_config() function, let's 
> >> say, in
> >> case an invalid mode is passed as argument, to let pwm_config() to choose 
> >> the
> >> first available PWM mode for PWM channel passed as argument?
> > 
> > What is it that actually needs solving?
> > 
> > If a driver requests normal mode and the PWM driver cannot support it
> > why not just return an error an move on.
> Because, simply, I wasn't aware of what these PWM client drivers needs for.

I'm afraid you have confused me here.

Didn't you just *add* the whole concept of PWM caps with your patches?
How could any existing call site expect anything except normal mode.
Until now there has been no possiblity to request anything else.


> > Put another way, what is the use case for secretly adopting a mode the
> > caller didn't want? Under what circumstances is this a good thing?
> No one... But I wasn't aware of what the PWM clients needs for from their PWM
> controllers. At this moment having BIT(ffs(caps.modes)) instead of
> PWM_MODE(NORMAL) is mostly the same since all the driver that has not 
> explicitly
> registered PWM caps will use PWM normal mode.
> 
> I will use PWM_MODE(NORMAL) instead of this in all the cases if this is OK 
> from
> your side.
> 
> Thank you,
> Claudiu Beznea
> > 
> > 
> > Daniel.
> > 

Re: [Patch v8 12/12] Documention: v4l: Documentation for HEVC CIDs

[Nicolas Dufresne]

Le vendredi 02 février 2018 à 17:55 +0530, Smitha T Murthy a écrit :
> Added V4l2 controls for HEVC encoder
> 
> Signed-off-by: Smitha T Murthy 
> ---
>  Documentation/media/uapi/v4l/extended-controls.rst | 410
> +
>  1 file changed, 410 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst
> b/Documentation/media/uapi/v4l/extended-controls.rst
> index dfe49ae..cb0a64a 100644
> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> @@ -1960,6 +1960,416 @@ enum v4l2_vp8_golden_frame_sel -
>  1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
>  
>  
> +High Efficiency Video Coding (HEVC/H.265) Control Reference
> +---
> +
> +The HEVC/H.265 controls include controls for encoding parameters of
> HEVC/H.265
> +video codec.
> +
> +
> +.. _hevc-control-id:
> +
> +HEVC/H.265 Control IDs
> +^^
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (integer)``
> +Minimum quantization parameter for HEVC.
> +Valid range: from 0 to 51.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP (integer)``
> +Maximum quantization parameter for HEVC.
> +Valid range: from 0 to 51.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP (integer)``
> +Quantization parameter for an I frame for HEVC.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP (integer)``
> +Quantization parameter for a P frame for HEVC.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP (integer)``
> +Quantization parameter for a B frame for HEVC.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_QP (boolean)``
> +HIERARCHICAL_QP allows the host to specify the quantization
> parameter
> +values for each temporal layer through HIERARCHICAL_QP_LAYER.
> This is
> +valid only if HIERARCHICAL_CODING_LAYER is greater than 1.
> Setting the
> +control value to 1 enables setting of the QP values for the
> layers.
> +
> +.. _v4l2-hevc-hier-coding-type:
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_TYPE``
> +(enum)
> +
> +enum v4l2_mpeg_video_hevc_hier_coding_type -
> +Selects the hierarchical coding type for encoding. Possible
> values are:
> +
> +.. raw:: latex
> +
> +\begin{adjustbox}{width=\columnwidth}
> +
> +.. tabularcolumns:: |p{11.0cm}|p{10.0cm}|
> +
> +.. flat-table::
> +:header-rows:  0
> +:stub-columns: 0
> +
> +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_B``
> +  - Use the B frame for hierarchical coding.
> +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_P``
> +  - Use the P frame for hierarchical coding.
> +
> +.. raw:: latex
> +
> +\end{adjustbox}
> +
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER (integer)``
> +Selects the hierarchical coding layer. In normal encoding
> +(non-hierarchial coding), it should be zero. Possible values are
> [0, 6].
> +0 indicates HIERARCHICAL CODING LAYER 0, 1 indicates
> HIERARCHICAL CODING
> +LAYER 1 and so on.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L0_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 0.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L1_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 1.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L2_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 2.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L3_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 3.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L4_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 4.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L5_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 5.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 6.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +.. _v4l2-hevc-profile:
> +
> 

Re: [Patch v8 12/12] Documention: v4l: Documentation for HEVC CIDs

[Nicolas Dufresne]

Le vendredi 02 février 2018 à 17:55 +0530, Smitha T Murthy a écrit :
> Added V4l2 controls for HEVC encoder
> 
> Signed-off-by: Smitha T Murthy 
> ---
>  Documentation/media/uapi/v4l/extended-controls.rst | 410
> +
>  1 file changed, 410 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/extended-controls.rst
> b/Documentation/media/uapi/v4l/extended-controls.rst
> index dfe49ae..cb0a64a 100644
> --- a/Documentation/media/uapi/v4l/extended-controls.rst
> +++ b/Documentation/media/uapi/v4l/extended-controls.rst
> @@ -1960,6 +1960,416 @@ enum v4l2_vp8_golden_frame_sel -
>  1, 2 and 3 corresponding to encoder profiles 0, 1, 2 and 3.
>  
>  
> +High Efficiency Video Coding (HEVC/H.265) Control Reference
> +---
> +
> +The HEVC/H.265 controls include controls for encoding parameters of
> HEVC/H.265
> +video codec.
> +
> +
> +.. _hevc-control-id:
> +
> +HEVC/H.265 Control IDs
> +^^
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP (integer)``
> +Minimum quantization parameter for HEVC.
> +Valid range: from 0 to 51.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP (integer)``
> +Maximum quantization parameter for HEVC.
> +Valid range: from 0 to 51.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_I_FRAME_QP (integer)``
> +Quantization parameter for an I frame for HEVC.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_P_FRAME_QP (integer)``
> +Quantization parameter for a P frame for HEVC.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP (integer)``
> +Quantization parameter for a B frame for HEVC.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_QP (boolean)``
> +HIERARCHICAL_QP allows the host to specify the quantization
> parameter
> +values for each temporal layer through HIERARCHICAL_QP_LAYER.
> This is
> +valid only if HIERARCHICAL_CODING_LAYER is greater than 1.
> Setting the
> +control value to 1 enables setting of the QP values for the
> layers.
> +
> +.. _v4l2-hevc-hier-coding-type:
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_TYPE``
> +(enum)
> +
> +enum v4l2_mpeg_video_hevc_hier_coding_type -
> +Selects the hierarchical coding type for encoding. Possible
> values are:
> +
> +.. raw:: latex
> +
> +\begin{adjustbox}{width=\columnwidth}
> +
> +.. tabularcolumns:: |p{11.0cm}|p{10.0cm}|
> +
> +.. flat-table::
> +:header-rows:  0
> +:stub-columns: 0
> +
> +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_B``
> +  - Use the B frame for hierarchical coding.
> +* - ``V4L2_MPEG_VIDEO_HEVC_HIERARCHICAL_CODING_P``
> +  - Use the P frame for hierarchical coding.
> +
> +.. raw:: latex
> +
> +\end{adjustbox}
> +
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_LAYER (integer)``
> +Selects the hierarchical coding layer. In normal encoding
> +(non-hierarchial coding), it should be zero. Possible values are
> [0, 6].
> +0 indicates HIERARCHICAL CODING LAYER 0, 1 indicates
> HIERARCHICAL CODING
> +LAYER 1 and so on.
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L0_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 0.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L1_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 1.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L2_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 2.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L3_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 3.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L4_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 4.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L5_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 5.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +``V4L2_CID_MPEG_VIDEO_HEVC_HIER_CODING_L6_QP (integer)``
> +Indicates quantization parameter for hierarchical coding layer
> 6.
> +Valid range: [V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP,
> +V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP].
> +
> +.. _v4l2-hevc-profile:
> +
> 

Re: [PATCH V2 1/2] [media] Add Rockchip RK1608 driver

[Heiko Stuebner]

Hi Leo,

Am Dienstag, 27. Februar 2018, 04:15:46 CET schrieb 温暖:
> Thank you for your advice! I will revise it according to your suggestion.

please also keep an eye on my reply to Linus' mail pointing out some
other issues where the driver should not tie into soc-specific areas
like the clock controller etc.


Thanks
Heiko

Am Dienstag, 27. Februar 2018, 04:15:46 CET schrieb 温暖:
> On 2/26/2018 18:12，Linus Walleij  wrote：
> On Mon, Feb 26, 2018 at 9:16 AM, Wen Nuan  wrote:
> +   pdata->grf_gpio2b_iomux = ioremap((resource_size_t)
>  + (GRF_BASE_ADDR +
>  +  GRF_GPIO2B_IOMUX), 4);
>  +   grf_val = __raw_readl(pdata->grf_gpio2b_iomux);
>  +   __raw_writel(((grf_val) | (1 << 6) | (1 << (6 + 16))),
>  +pdata->grf_gpio2b_iomux);
>  +
>  +   pdata->grf_io_vsel = ioremap((resource_size_t)
>  + (GRF_BASE_ADDR + GRF_IO_VSEL), 
> 4);
>  +   grf_val = __raw_readl(pdata->grf_io_vsel);
>  +   __raw_writel(((grf_val) | (1 << 1) | (1 << (1 + 16))),
>  +pdata->grf_io_vsel);
> 
> You are doing pin control on the side of the pin control subsystem
> it looks like?
> 
> I think David Wu and Heiko Stubner needs to have a look at what you
> are doing here to suggest other solutions.
> 
> Apart from that, why use __raw_writel instead of just writel()?
> 
> This pin is iomux, default GPIO, need to be changed to CLK. 
> This CLK is provided to external sensor for use.
> I'll use writel().

As stated, please don't directly access soc blocks like the clock
controller or iomuxes, there are standard APIs like the general
clock API and also assigned-clock* devicetree properties.

Similarly for pinctrl access.

So there should not be any writel (or ioremap) at all in this spi driver
I'd think.


Thanks
Heiko

[PATCH v2 1/3] media: i2c: adv748x: Simplify regmap configuration

[Kieran Bingham]

From: Kieran Bingham 

The ADV748x has identical map configurations for each register map. The
duplication of each map can be simplified using a helper macro such that
each map is represented on a single line.

Define ADV748X_REGMAP_CONF for this purpose use it to create the tables.

Signed-off-by: Kieran Bingham 

---
v2:
 - Remove unnecessary #undef

 drivers/media/i2c/adv748x/adv748x-core.c | 109 ++-
 1 file changed, 20 insertions(+), 89 deletions(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-core.c 
b/drivers/media/i2c/adv748x/adv748x-core.c
index fd92c9e4b519..faf73949962b 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -35,96 +35,27 @@
  * Register manipulation
  */
 
-static const struct regmap_config adv748x_regmap_cnf[] = {
-   {
-   .name   = "io",
-   .reg_bits   = 8,
-   .val_bits   = 8,
-
-   .max_register   = 0xff,
-   .cache_type = REGCACHE_NONE,
-   },
-   {
-   .name   = "dpll",
-   .reg_bits   = 8,
-   .val_bits   = 8,
-
-   .max_register   = 0xff,
-   .cache_type = REGCACHE_NONE,
-   },
-   {
-   .name   = "cp",
-   .reg_bits   = 8,
-   .val_bits   = 8,
-
-   .max_register   = 0xff,
-   .cache_type = REGCACHE_NONE,
-   },
-   {
-   .name   = "hdmi",
-   .reg_bits   = 8,
-   .val_bits   = 8,
-
-   .max_register   = 0xff,
-   .cache_type = REGCACHE_NONE,
-   },
-   {
-   .name   = "edid",
-   .reg_bits   = 8,
-   .val_bits   = 8,
-
-   .max_register   = 0xff,
-   .cache_type = REGCACHE_NONE,
-   },
-   {
-   .name   = "repeater",
-   .reg_bits   = 8,
-   .val_bits   = 8,
-
-   .max_register   = 0xff,
-   .cache_type = REGCACHE_NONE,
-   },
-   {
-   .name   = "infoframe",
-   .reg_bits   = 8,
-   .val_bits   = 8,
-
-   .max_register   = 0xff,
-   .cache_type = REGCACHE_NONE,
-   },
-   {
-   .name   = "cec",
-   .reg_bits   = 8,
-   .val_bits   = 8,
-
-   .max_register   = 0xff,
-   .cache_type = REGCACHE_NONE,
-   },
-   {
-   .name   = "sdp",
-   .reg_bits   = 8,
-   .val_bits   = 8,
-
-   .max_register   = 0xff,
-   .cache_type = REGCACHE_NONE,
-   },
-
-   {
-   .name   = "txb",
-   .reg_bits   = 8,
-   .val_bits   = 8,
-
-   .max_register   = 0xff,
-   .cache_type = REGCACHE_NONE,
-   },
-   {
-   .name   = "txa",
-   .reg_bits   = 8,
-   .val_bits   = 8,
+#define ADV748X_REGMAP_CONF(n) \
+{ \
+   .name = n, \
+   .reg_bits = 8, \
+   .val_bits = 8, \
+   .max_register = 0xff, \
+   .cache_type = REGCACHE_NONE, \
+}
 
-   .max_register   = 0xff,
-   .cache_type = REGCACHE_NONE,
-   },
+static const struct regmap_config adv748x_regmap_cnf[] = {
+   ADV748X_REGMAP_CONF("io"),
+   ADV748X_REGMAP_CONF("dpll"),
+   ADV748X_REGMAP_CONF("cp"),
+   ADV748X_REGMAP_CONF("hdmi"),
+   ADV748X_REGMAP_CONF("edid"),
+   ADV748X_REGMAP_CONF("repeater"),
+   ADV748X_REGMAP_CONF("infoframe"),
+   ADV748X_REGMAP_CONF("cec"),
+   ADV748X_REGMAP_CONF("sdp"),
+   ADV748X_REGMAP_CONF("txa"),
+   ADV748X_REGMAP_CONF("txb"),
 };
 
 static int adv748x_configure_regmap(struct adv748x_state *state, int region)
-- 
2.7.4

[PATCH v2 0/3] media: i2c: adv748x: Fix CBUS page issue

[Kieran Bingham]

The ADV748x has 12 pages mapped on to I2C addresses.

The existing implementation only has 11 of these in the map enumeration, and
this can cause an off-by-one error when programming the map addresses.

This short series simplifies the regmap configuration tables, and adds the
missing CBUS page to better model the device, and remove the off by one error.

Once the tables are corrected, we add support for overriding the default map
addresses through the device tree using i2c_new_secondary_device().

Kieran Bingham (3):
  media: i2c: adv748x: Simplify regmap configuration
  media: i2c: adv748x: Add missing CBUS page
  media: i2c: adv748x: Add support for i2c_new_secondary_device

 drivers/media/i2c/adv748x/adv748x-core.c | 185 ++-
 drivers/media/i2c/adv748x/adv748x.h  |  14 +--
 2 files changed, 58 insertions(+), 141 deletions(-)

-- 
2.7.4

[PATCH v2 2/3] media: i2c: adv748x: Add missing CBUS page

[Kieran Bingham]

From: Kieran Bingham 

The ADV748x has 12 pages mapped onto I2C addresses.

In the existing implementation only 11 are mapped correctly in the page
enumerations, which causes an off-by-one fault on pages above the
infoframe definition due to a missing 'CBUS' page.

This causes the address for the CEC, SDP, TXA, and TXB to be incorrectly
programmed during the iterations in adv748x_initialise_clients().

Until now this has gone un-noticed due to the fact that following the
creation of the clients - the device is reset and the addresses are
reprogrammed in manually by the call to "adv748x_write_regs(state,
adv748x_set_slave_address);"

As part of moving to dynamic i2c address allocations repair this by
providing the missing CBUS page definition.

Signed-off-by: Kieran Bingham 
Reviewed-by: Niklas Söderlund 
---
v2
 - Remove period from commit title.
 - Collect Niklas' RB tag.


 drivers/media/i2c/adv748x/adv748x-core.c | 3 +++
 drivers/media/i2c/adv748x/adv748x.h  | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/media/i2c/adv748x/adv748x-core.c 
b/drivers/media/i2c/adv748x/adv748x-core.c
index faf73949962b..9dcaadaca217 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -52,6 +52,7 @@ static const struct regmap_config adv748x_regmap_cnf[] = {
ADV748X_REGMAP_CONF("edid"),
ADV748X_REGMAP_CONF("repeater"),
ADV748X_REGMAP_CONF("infoframe"),
+   ADV748X_REGMAP_CONF("cbus"),
ADV748X_REGMAP_CONF("cec"),
ADV748X_REGMAP_CONF("sdp"),
ADV748X_REGMAP_CONF("txa"),
@@ -89,6 +90,7 @@ static int adv748x_i2c_addresses[ADV748X_PAGE_MAX] = {
ADV748X_I2C_EDID,
ADV748X_I2C_REPEATER,
ADV748X_I2C_INFOFRAME,
+   ADV748X_I2C_CBUS,
ADV748X_I2C_CEC,
ADV748X_I2C_SDP,
ADV748X_I2C_TXB,
@@ -352,6 +354,7 @@ static const struct adv748x_reg_value 
adv748x_set_slave_address[] = {
{ADV748X_PAGE_IO, 0xf6, ADV748X_I2C_EDID << 1},
{ADV748X_PAGE_IO, 0xf7, ADV748X_I2C_REPEATER << 1},
{ADV748X_PAGE_IO, 0xf8, ADV748X_I2C_INFOFRAME << 1},
+   {ADV748X_PAGE_IO, 0xf9, ADV748X_I2C_CBUS << 1},
{ADV748X_PAGE_IO, 0xfa, ADV748X_I2C_CEC << 1},
{ADV748X_PAGE_IO, 0xfb, ADV748X_I2C_SDP << 1},
{ADV748X_PAGE_IO, 0xfc, ADV748X_I2C_TXB << 1},
diff --git a/drivers/media/i2c/adv748x/adv748x.h 
b/drivers/media/i2c/adv748x/adv748x.h
index 6789e2f3bc8c..725662edc4b8 100644
--- a/drivers/media/i2c/adv748x/adv748x.h
+++ b/drivers/media/i2c/adv748x/adv748x.h
@@ -35,6 +35,7 @@
 #define ADV748X_I2C_EDID   0x36/* EDID Map */
 #define ADV748X_I2C_REPEATER   0x32/* HDMI RX Repeater Map */
 #define ADV748X_I2C_INFOFRAME  0x31/* HDMI RX InfoFrame Map */
+#define ADV748X_I2C_CBUS   0x30/* CBUS MHL Map */
 #define ADV748X_I2C_CEC0x41/* CEC Map */
 #define ADV748X_I2C_SDP0x79/* SDP Map */
 #define ADV748X_I2C_TXB0x48/* CSI-TXB Map */
@@ -48,6 +49,7 @@ enum adv748x_page {
ADV748X_PAGE_EDID,
ADV748X_PAGE_REPEATER,
ADV748X_PAGE_INFOFRAME,
+   ADV748X_PAGE_CBUS,
ADV748X_PAGE_CEC,
ADV748X_PAGE_SDP,
ADV748X_PAGE_TXB,
-- 
2.7.4

[PATCH v2 3/3] media: i2c: adv748x: Add support for i2c_new_secondary_device

[Kieran Bingham]

From: Kieran Bingham 

The ADV748x has twelve 256-byte maps that can be accessed via the main
I2C ports. Each map has it own I2C address and acts as a standard slave
device on the I2C bus.

Allow a device tree node to override the default addresses so that
address conflicts with other devices on the same bus may be resolved at
the board description level.

Signed-off-by: Kieran Bingham 
---
 drivers/media/i2c/adv748x/adv748x-core.c | 77 +++-
 drivers/media/i2c/adv748x/adv748x.h  | 14 --
 2 files changed, 36 insertions(+), 55 deletions(-)

diff --git a/drivers/media/i2c/adv748x/adv748x-core.c 
b/drivers/media/i2c/adv748x/adv748x-core.c
index 9dcaadaca217..911ccf6eb68c 100644
--- a/drivers/media/i2c/adv748x/adv748x-core.c
+++ b/drivers/media/i2c/adv748x/adv748x-core.c
@@ -80,21 +80,24 @@ static int adv748x_configure_regmap(struct adv748x_state 
*state, int region)
 
return 0;
 }
+struct adv748x_register_map {
+   const char *name;
+   u8 default_addr;
+};
 
-/* Default addresses for the I2C pages */
-static int adv748x_i2c_addresses[ADV748X_PAGE_MAX] = {
-   ADV748X_I2C_IO,
-   ADV748X_I2C_DPLL,
-   ADV748X_I2C_CP,
-   ADV748X_I2C_HDMI,
-   ADV748X_I2C_EDID,
-   ADV748X_I2C_REPEATER,
-   ADV748X_I2C_INFOFRAME,
-   ADV748X_I2C_CBUS,
-   ADV748X_I2C_CEC,
-   ADV748X_I2C_SDP,
-   ADV748X_I2C_TXB,
-   ADV748X_I2C_TXA,
+static const struct adv748x_register_map adv748x_default_addresses[] = {
+   [ADV748X_PAGE_IO] = { "main", 0x70 },
+   [ADV748X_PAGE_DPLL] = { "dpll", 0x26 },
+   [ADV748X_PAGE_CP] = { "cp", 0x22 },
+   [ADV748X_PAGE_HDMI] = { "hdmi", 0x34 },
+   [ADV748X_PAGE_EDID] = { "edid", 0x36 },
+   [ADV748X_PAGE_REPEATER] = { "repeater", 0x32 },
+   [ADV748X_PAGE_INFOFRAME] = { "infoframe", 0x31 },
+   [ADV748X_PAGE_CBUS] = { "cbus", 0x30 },
+   [ADV748X_PAGE_CEC] = { "cec", 0x41 },
+   [ADV748X_PAGE_SDP] = { "sdp", 0x79 },
+   [ADV748X_PAGE_TXB] = { "txb", 0x48 },
+   [ADV748X_PAGE_TXA] = { "txa", 0x4a },
 };
 
 static int adv748x_read_check(struct adv748x_state *state,
@@ -143,15 +146,20 @@ int adv748x_write_block(struct adv748x_state *state, int 
client_page,
return regmap_raw_write(regmap, init_reg, val, val_len);
 }
 
-static struct i2c_client *adv748x_dummy_client(struct adv748x_state *state,
-  u8 addr, u8 io_reg)
+static int adv748x_set_slave_addresses(struct adv748x_state *state)
 {
-   struct i2c_client *client = state->client;
+   struct i2c_client *client;
+   unsigned int i;
+   u8 io_reg;
+
+   for (i = ADV748X_PAGE_DPLL; i < ADV748X_PAGE_MAX; ++i) {
+   io_reg = ADV748X_IO_SLAVE_ADDR_BASE + i;
+   client = state->i2c_clients[i];
 
-   if (addr)
-   io_write(state, io_reg, addr << 1);
+   io_write(state, io_reg, client->addr << 1);
+   }
 
-   return i2c_new_dummy(client->adapter, io_read(state, io_reg) >> 1);
+   return 0;
 }
 
 static void adv748x_unregister_clients(struct adv748x_state *state)
@@ -164,13 +172,15 @@ static void adv748x_unregister_clients(struct 
adv748x_state *state)
 
 static int adv748x_initialise_clients(struct adv748x_state *state)
 {
-   int i;
+   unsigned int i;
int ret;
 
for (i = ADV748X_PAGE_DPLL; i < ADV748X_PAGE_MAX; ++i) {
-   state->i2c_clients[i] =
-   adv748x_dummy_client(state, adv748x_i2c_addresses[i],
-ADV748X_IO_SLAVE_ADDR_BASE + i);
+   state->i2c_clients[i] = i2c_new_secondary_device(
+   state->client,
+   adv748x_default_addresses[i].name,
+   adv748x_default_addresses[i].default_addr);
+
if (state->i2c_clients[i] == NULL) {
adv_err(state, "failed to create i2c client %u\n", i);
return -ENOMEM;
@@ -181,7 +191,7 @@ static int adv748x_initialise_clients(struct adv748x_state 
*state)
return ret;
}
 
-   return 0;
+   return adv748x_set_slave_addresses(state);
 }
 
 /**
@@ -347,21 +357,6 @@ static const struct adv748x_reg_value adv748x_sw_reset[] = 
{
{ADV748X_PAGE_EOR, 0xff, 0xff}  /* End of register table */
 };
 
-static const struct adv748x_reg_value adv748x_set_slave_address[] = {
-   {ADV748X_PAGE_IO, 0xf3, ADV748X_I2C_DPLL << 1},
-   {ADV748X_PAGE_IO, 0xf4, ADV748X_I2C_CP << 1},
-   {ADV748X_PAGE_IO, 0xf5, ADV748X_I2C_HDMI << 1},
-   {ADV748X_PAGE_IO, 0xf6, ADV748X_I2C_EDID << 1},
-   {ADV748X_PAGE_IO, 0xf7, ADV748X_I2C_REPEATER << 1},
-   {ADV748X_PAGE_IO, 0xf8, ADV748X_I2C_INFOFRAME << 1},
-   {ADV748X_PAGE_IO, 0xf9, ADV748X_I2C_CBUS << 1},
-   {ADV748X_PAGE_IO, 0xfa, 

Re: [PATCH 2/2] media: tw9910: solve coding style issues

[jacopo mondi]

Hi Mauro,
   thanks for doing this.

I didn't dare to touch this driver style issues as it was mainline
already, but since you addressed this I now would have more changes to
apply...

On Mon, Feb 26, 2018 at 09:28:08AM -0500, Mauro Carvalho Chehab wrote:
> As we're adding this as a new driver, make checkpatch happier by
> solving several style issues, using --fix-inplace at strict mode.
>
> Some issues required manual work.
>
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/i2c/tw9910.c | 32 
>  1 file changed, 16 insertions(+), 16 deletions(-)

[snip]

>
>   if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL)))
> @@ -532,16 +533,16 @@ static int tw9910_s_std(struct v4l2_subdev *sd, 
> v4l2_std_id norm)
>   }
>   if (!ret)
>   ret = i2c_smbus_write_byte_data(client, CROP_HI,
> - ((vdelay >> 2) & 0xc0) |
> + ((vdelay >> 2) & 0xc0) |
>   ((vact >> 4) & 0x30) |
>   ((hdelay >> 6) & 0x0c) |
>   ((hact >> 8) & 0x03));

If you move the first line, all the following ones should be be moved
too.

I can send a few patches more on top of this two, to address this and
a few other style issues on this driver.

Would this work for you?

Thanks
  j

>   if (!ret)
>   ret = i2c_smbus_write_byte_data(client, VDELAY_LO,
> - vdelay & 0xff);
> + vdelay & 0xff);
>   if (!ret)
>   ret = i2c_smbus_write_byte_data(client, VACTIVE_LO,
> - vact & 0xff);
> + vact & 0xff);
>
>   return ret;
>  }
> @@ -731,7 +732,7 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32 
> *width, u32 *height)
>  }
>
>  static int tw9910_get_selection(struct v4l2_subdev *sd,
> - struct v4l2_subdev_pad_config *cfg,
> + struct v4l2_subdev_pad_config *cfg,
>   struct v4l2_subdev_selection *sel)
>  {
>   struct i2c_client *client = v4l2_get_subdevdata(sd);
> @@ -756,7 +757,7 @@ static int tw9910_get_selection(struct v4l2_subdev *sd,
>  }
>
>  static int tw9910_get_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_pad_config *cfg,
> +   struct v4l2_subdev_pad_config *cfg,
>   struct v4l2_subdev_format *format)
>  {
>   struct v4l2_mbus_framefmt *mf = >format;
> @@ -807,7 +808,7 @@ static int tw9910_s_fmt(struct v4l2_subdev *sd,
>  }
>
>  static int tw9910_set_fmt(struct v4l2_subdev *sd,
> - struct v4l2_subdev_pad_config *cfg,
> +   struct v4l2_subdev_pad_config *cfg,
>   struct v4l2_subdev_format *format)
>  {
>   struct v4l2_mbus_framefmt *mf = >format;
> @@ -818,9 +819,9 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
>   if (format->pad)
>   return -EINVAL;
>
> - if (V4L2_FIELD_ANY == mf->field) {
> + if (mf->field == V4L2_FIELD_ANY) {
>   mf->field = V4L2_FIELD_INTERLACED_BT;
> - } else if (V4L2_FIELD_INTERLACED_BT != mf->field) {
> + } else if (mf->field != V4L2_FIELD_INTERLACED_BT) {
>   dev_err(>dev, "Field type %d invalid.\n", mf->field);
>   return -EINVAL;
>   }
> @@ -870,8 +871,7 @@ static int tw9910_video_probe(struct i2c_client *client)
>   priv->revision = GET_REV(id);
>   id = GET_ID(id);
>
> - if (0x0B != id ||
> - 0x01 < priv->revision) {
> + if (id != 0x0b || priv->revision > 0x01) {
>   dev_err(>dev,
>   "Product ID error %x:%x\n",
>   id, priv->revision);
> @@ -899,7 +899,7 @@ static const struct v4l2_subdev_core_ops 
> tw9910_subdev_core_ops = {
>  };
>
>  static int tw9910_enum_mbus_code(struct v4l2_subdev *sd,
> - struct v4l2_subdev_pad_config *cfg,
> +  struct v4l2_subdev_pad_config *cfg,
>   struct v4l2_subdev_mbus_code_enum *code)
>  {
>   if (code->pad || code->index)
> --
> 2.14.3
>

[PATCH] media: rc: lirc does not use LIRC_CAN_SEND_SCANCODE feature

[Sean Young]

Since commit 02d742f4b209 ("media: lirc: lirc daemon fails to detect raw
IR device"), the feature LIRC_CAN_SEND_SCANCODE is no longer used as it
tripped up lircd. The ability to send scancodes for IR Tx is implied by
LIRC_CAN_SEND_PULSE (i.e. any device that can send can use IR Tx encoders).

So, remove LIRC_CAN_SEND_SCANCODE since it never used. This fixes:

Documentation/output/lirc.h.rst:6: WARNING: undefined label:
lirc-can-send-scancode (if the link has no caption the label must precede
a section header

Signed-off-by: Sean Young 
---
 include/uapi/linux/lirc.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/uapi/linux/lirc.h b/include/uapi/linux/lirc.h
index 4fe580d36e41..f5bf06ecd87d 100644
--- a/include/uapi/linux/lirc.h
+++ b/include/uapi/linux/lirc.h
@@ -54,7 +54,6 @@
 #define LIRC_CAN_SEND_RAW  LIRC_MODE2SEND(LIRC_MODE_RAW)
 #define LIRC_CAN_SEND_PULSELIRC_MODE2SEND(LIRC_MODE_PULSE)
 #define LIRC_CAN_SEND_MODE2LIRC_MODE2SEND(LIRC_MODE_MODE2)
-#define LIRC_CAN_SEND_SCANCODE LIRC_MODE2SEND(LIRC_MODE_SCANCODE)
 #define LIRC_CAN_SEND_LIRCCODE LIRC_MODE2SEND(LIRC_MODE_LIRCCODE)
 
 #define LIRC_CAN_SEND_MASK 0x003f
-- 
2.14.3

Re: [PATCH v2] Staging: bcm2048: Fix function argument alignment in radio-bcm2048.c.

[Greg KH]

On Tue, Feb 27, 2018 at 08:32:30AM +0100, Hans Verkuil wrote:
> On 02/27/2018 02:53 AM, Quytelda Kahja wrote:
> > Hans,
> > 
> > Thank you very much for your input on the patch; however this patch
> > has already been applied to the staging tree.  Additionally:
> 
> I have no record of this being applied through linux-media. Did someone
> else pick this up? Greg perhaps?

Did I pick this up?  Oops, sorry, didn't mean to, I'll go revert it now,
sorry...

greg k-h

[GIT PULL] Remove metag architecture

[James Hogan]

Hi Arnd,

On Fri, Feb 23, 2018 at 01:26:09PM +0100, Arnd Bergmann wrote:
> On Fri, Feb 23, 2018 at 12:02 PM, James Hogan  wrote:
> > I'm happy to put v2 in linux-next now (only patch 4 has changed, I just
> > sent an updated version), and send you a pull request early next week so
> > you can take it from there. The patches can't be directly applied with
> > git-am anyway thanks to the -D option to make them more concise.
> >
> > Sound okay?
> 
> Yes, sounds good, thanks!

As discussed, here is a tagged branch to remove arch/metag and dependent
drivers. Its basically v2 with some acks added.

Cheers
James

The following changes since commit 91ab883eb21325ad80f3473633f794c78ac87f51:

  Linux 4.16-rc2 (2018-02-18 17:29:42 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jhogan/metag.git 
tags/metag_remove

for you to fetch changes up to ef9fb83815db7d7e03da9a0904b4ef352e633922:

  i2c: img-scb: Drop METAG dependency (2018-02-26 14:58:09 +)


Remove metag architecture

These patches remove the metag architecture and tightly dependent
drivers from the kernel. With the 4.16 kernel the ancient gcc 4.2.4
based metag toolchain we have been using is hitting compiler bugs, so
now seems a good time to drop it altogether.


James Hogan (13):
  metag: Remove arch/metag/
  docs: Remove metag docs
  docs: Remove remaining references to metag
  Drop a bunch of metag references
  irqchip: Remove metag irqchip drivers
  clocksource: Remove metag generic timer driver
  tty: Remove metag DA TTY and console driver
  MAINTAINERS/CREDITS: Drop METAG ARCHITECTURE
  pinctrl: Drop TZ1090 drivers
  gpio: Drop TZ1090 drivers
  watchdog: imgpdc: Drop METAG dependency
  media: img-ir: Drop METAG dependency
  i2c: img-scb: Drop METAG dependency

 CREDITS|5 +
 Documentation/00-INDEX |2 -
 Documentation/admin-guide/kernel-parameters.txt|4 -
 Documentation/dev-tools/kmemleak.rst   |2 +-
 .../devicetree/bindings/gpio/gpio-tz1090-pdc.txt   |   45 -
 .../devicetree/bindings/gpio/gpio-tz1090.txt   |   88 -
 Documentation/devicetree/bindings/metag/meta.txt   |   30 -
 .../bindings/pinctrl/img,tz1090-pdc-pinctrl.txt|  127 --
 .../bindings/pinctrl/img,tz1090-pinctrl.txt|  227 ---
 .../features/core/BPF-JIT/arch-support.txt |1 -
 .../core/generic-idle-thread/arch-support.txt  |1 -
 .../features/core/jump-labels/arch-support.txt |1 -
 .../features/core/tracehook/arch-support.txt   |1 -
 .../features/debug/KASAN/arch-support.txt  |1 -
 .../debug/gcov-profile-all/arch-support.txt|1 -
 Documentation/features/debug/kgdb/arch-support.txt |1 -
 .../debug/kprobes-on-ftrace/arch-support.txt   |1 -
 .../features/debug/kprobes/arch-support.txt|1 -
 .../features/debug/kretprobes/arch-support.txt |1 -
 .../features/debug/optprobes/arch-support.txt  |1 -
 .../features/debug/stackprotector/arch-support.txt |1 -
 .../features/debug/uprobes/arch-support.txt|1 -
 .../debug/user-ret-profiler/arch-support.txt   |1 -
 .../features/io/dma-api-debug/arch-support.txt |1 -
 .../features/io/dma-contiguous/arch-support.txt|1 -
 .../features/io/sg-chain/arch-support.txt  |1 -
 .../features/lib/strncasecmp/arch-support.txt  |1 -
 .../locking/cmpxchg-local/arch-support.txt |1 -
 .../features/locking/lockdep/arch-support.txt  |1 -
 .../locking/queued-rwlocks/arch-support.txt|1 -
 .../locking/queued-spinlocks/arch-support.txt  |1 -
 .../locking/rwsem-optimized/arch-support.txt   |1 -
 .../features/perf/kprobes-event/arch-support.txt   |1 -
 .../features/perf/perf-regs/arch-support.txt   |1 -
 .../features/perf/perf-stackdump/arch-support.txt  |1 -
 .../sched/membarrier-sync-core/arch-support.txt|1 -
 .../features/sched/numa-balancing/arch-support.txt |1 -
 .../seccomp/seccomp-filter/arch-support.txt|1 -
 .../time/arch-tick-broadcast/arch-support.txt  |1 -
 .../features/time/clockevents/arch-support.txt |1 -
 .../time/context-tracking/arch-support.txt |1 -
 .../features/time/irq-time-acct/arch-support.txt   |1 -
 .../time/modern-timekeeping/arch-support.txt   |1 -
 .../features/time/virt-cpuacct/arch-support.txt|1 -
 .../features/vm/ELF-ASLR/arch-support.txt  |1 -
 .../features/vm/PG_uncached/arch-support.txt   |1 -
 Documentation/features/vm/THP/arch-support.txt |1 -
 Documentation/features/vm/TLB/arch-support.txt |1 -
 .../features/vm/huge-vmap/arch-support.txt |1 -
 

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Claudiu Beznea]

On 27.02.2018 12:54, Daniel Thompson wrote:
> On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
>> On 26.02.2018 11:57, Jani Nikula wrote:
>>> On Thu, 22 Feb 2018, Daniel Thompson  wrote:
 On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
> were adapted to this change.
>
> Signed-off-by: Claudiu Beznea 
> ---
>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
>  drivers/bus/ts-nbus.c|  2 +-
>  drivers/clk/clk-pwm.c|  3 ++-
>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
>  drivers/hwmon/pwm-fan.c  |  2 +-
>  drivers/input/misc/max77693-haptic.c |  2 +-
>  drivers/input/misc/max8997_haptic.c  |  6 +-
>  drivers/leds/leds-pwm.c  |  5 -
>  drivers/media/rc/ir-rx51.c   |  5 -
>  drivers/media/rc/pwm-ir-tx.c |  5 -
>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
>  drivers/video/backlight/lp8788_bl.c  |  5 -
>  drivers/video/backlight/pwm_bl.c | 11 +--
>  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
>  include/linux/pwm.h  |  6 --
>  16 files changed, 70 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/video/backlight/lm3630a_bl.c 
> b/drivers/video/backlight/lm3630a_bl.c
> index 2030a6b77a09..696fa25dafd2 100644
> --- a/drivers/video/backlight/lm3630a_bl.c
> +++ b/drivers/video/backlight/lm3630a_bl.c
> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip 
> *pchip, int br, int br_max)
>  {
>   unsigned int period = pchip->pdata->pwm_period;
>   unsigned int duty = br * period / br_max;
> + struct pwm_caps caps = { };
>  
> - pwm_config(pchip->pwmd, duty, period);
> + pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, );
> + pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));

 Well... I admit I've only really looked at the patches that impact 
 backlight but dispersing this really odd looking bit twiddling 
 throughout the kernel doesn't strike me a great API design.

 IMHO callers should not be required to find the first set bit in
 some specially crafted set of capability bits simply to get sane 
 default behaviour.
>>>
>>> Agreed. IMHO the regular use case becomes rather tedious, ugly, and
>>> error prone.
>>
>> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
>> from your side?
>>
>> Or, what about using a function like pwm_mode_first() to get the first 
>> supported
>> mode by PWM channel?
>>
>> Or, would you prefer to solve this inside pwm_config() function, let's say, 
>> in
>> case an invalid mode is passed as argument, to let pwm_config() to choose the
>> first available PWM mode for PWM channel passed as argument?
> 
> What is it that actually needs solving?
> 
> If a driver requests normal mode and the PWM driver cannot support it
> why not just return an error an move on.
Because, simply, I wasn't aware of what these PWM client drivers needs for.

> 
> Put another way, what is the use case for secretly adopting a mode the
> caller didn't want? Under what circumstances is this a good thing?
No one... But I wasn't aware of what the PWM clients needs for from their PWM
controllers. At this moment having BIT(ffs(caps.modes)) instead of
PWM_MODE(NORMAL) is mostly the same since all the driver that has not explicitly
registered PWM caps will use PWM normal mode.

I will use PWM_MODE(NORMAL) instead of this in all the cases if this is OK from
your side.

Thank you,
Claudiu Beznea
> 
> 
> Daniel.
> 

Re: [PATCH v3 05/10] pwm: add PWM mode to pwm_config()

[Daniel Thompson]

On Mon, Feb 26, 2018 at 04:24:15PM +0200, Claudiu Beznea wrote:
> On 26.02.2018 11:57, Jani Nikula wrote:
> > On Thu, 22 Feb 2018, Daniel Thompson  wrote:
> >> On Thu, Feb 22, 2018 at 02:01:16PM +0200, Claudiu Beznea wrote:
> >>> Add PWM mode to pwm_config() function. The drivers which uses pwm_config()
> >>> were adapted to this change.
> >>>
> >>> Signed-off-by: Claudiu Beznea 
> >>> ---
> >>>  arch/arm/mach-s3c24xx/mach-rx1950.c  | 11 +--
> >>>  drivers/bus/ts-nbus.c|  2 +-
> >>>  drivers/clk/clk-pwm.c|  3 ++-
> >>>  drivers/gpu/drm/i915/intel_panel.c   | 17 ++---
> >>>  drivers/hwmon/pwm-fan.c  |  2 +-
> >>>  drivers/input/misc/max77693-haptic.c |  2 +-
> >>>  drivers/input/misc/max8997_haptic.c  |  6 +-
> >>>  drivers/leds/leds-pwm.c  |  5 -
> >>>  drivers/media/rc/ir-rx51.c   |  5 -
> >>>  drivers/media/rc/pwm-ir-tx.c |  5 -
> >>>  drivers/video/backlight/lm3630a_bl.c |  4 +++-
> >>>  drivers/video/backlight/lp855x_bl.c  |  4 +++-
> >>>  drivers/video/backlight/lp8788_bl.c  |  5 -
> >>>  drivers/video/backlight/pwm_bl.c | 11 +--
> >>>  drivers/video/fbdev/ssd1307fb.c  |  3 ++-
> >>>  include/linux/pwm.h  |  6 --
> >>>  16 files changed, 70 insertions(+), 21 deletions(-)
> >>>
> >>> diff --git a/drivers/video/backlight/lm3630a_bl.c 
> >>> b/drivers/video/backlight/lm3630a_bl.c
> >>> index 2030a6b77a09..696fa25dafd2 100644
> >>> --- a/drivers/video/backlight/lm3630a_bl.c
> >>> +++ b/drivers/video/backlight/lm3630a_bl.c
> >>> @@ -165,8 +165,10 @@ static void lm3630a_pwm_ctrl(struct lm3630a_chip 
> >>> *pchip, int br, int br_max)
> >>>  {
> >>>   unsigned int period = pchip->pdata->pwm_period;
> >>>   unsigned int duty = br * period / br_max;
> >>> + struct pwm_caps caps = { };
> >>>  
> >>> - pwm_config(pchip->pwmd, duty, period);
> >>> + pwm_get_caps(pchip->pwmd->chip, pchip->pwmd, );
> >>> + pwm_config(pchip->pwmd, duty, period, BIT(ffs(caps.modes) - 1));
> >>
> >> Well... I admit I've only really looked at the patches that impact 
> >> backlight but dispersing this really odd looking bit twiddling 
> >> throughout the kernel doesn't strike me a great API design.
> >>
> >> IMHO callers should not be required to find the first set bit in
> >> some specially crafted set of capability bits simply to get sane 
> >> default behaviour.
> > 
> > Agreed. IMHO the regular use case becomes rather tedious, ugly, and
> > error prone.
> 
> Using simply PWM_MODE(NORMAL) instead of BIT(ffs(caps.modes) - 1) would be OK
> from your side?
>
> Or, what about using a function like pwm_mode_first() to get the first 
> supported
> mode by PWM channel?
> 
> Or, would you prefer to solve this inside pwm_config() function, let's say, in
> case an invalid mode is passed as argument, to let pwm_config() to choose the
> first available PWM mode for PWM channel passed as argument?

What is it that actually needs solving?

If a driver requests normal mode and the PWM driver cannot support it
why not just return an error an move on.

Put another way, what is the use case for secretly adopting a mode the
caller didn't want? Under what circumstances is this a good thing?


Daniel.

[GIT PULL FOR v4.17] Minor RC fixes

[Sean Young]

Hi Mauro,

Just two minor changes for RC.

Thanks,
Sean

The following changes since commit 15ea2df9143729a2b722d4ca2b52cfa14a819d8e:

  media: ov2685: mark PM functions as __maybe_unused (2018-02-26 10:38:56 -0500)

are available in the Git repository at:

  git://linuxtv.org/syoung/media_tree.git for-v4.17b

for you to fetch changes up to 66b3840b6bdbbef998741122bd19e6a54c32793d:

  media: rc: fix race condition in ir_raw_event_store_edge() handling 
(2018-02-27 09:00:21 +)


Sean Young (2):
  media: rc: no need to announce major number
  media: rc: fix race condition in ir_raw_event_store_edge() handling

 Documentation/media/uapi/rc/lirc-dev-intro.rst |  1 -
 drivers/media/rc/lirc_dev.c|  4 ++--
 drivers/media/rc/rc-core-priv.h|  5 +++--
 drivers/media/rc/rc-ir-raw.c   | 24 +---
 4 files changed, 26 insertions(+), 8 deletions(-)

Re: [PATCH 01/13] media: v4l2-fwnode: Let parse_endpoint callback decide if no remote is error

[Laurent Pinchart]

Hi Philipp,

On Tuesday, 27 February 2018 11:13:04 EET Philipp Zabel wrote:
> On Fri, 2018-02-23 at 14:47 +0200, Sakari Ailus wrote:
> > On Fri, Feb 23, 2018 at 12:16:17PM +0100, Philipp Zabel wrote:
> >> On Fri, 2018-02-23 at 12:05 +0200, Laurent Pinchart wrote:
> >>> On Friday, 23 February 2018 11:56:52 EET Philipp Zabel wrote:
>  On Fri, 2018-02-23 at 11:29 +0200, Laurent Pinchart wrote:
> > On Thursday, 22 February 2018 03:39:37 EET Steve Longerbeam wrote:
> >> For some subdevices, a fwnode endpoint that has no connection to
> >> a remote endpoint may not be an error. Let the parse_endpoint
> >> callback
> > 
> > make that decision in v4l2_async_notifier_fwnode_parse_endpoint().
> > If
> > 
> >> the callback indicates that is not an error, skip adding the asd
> >> to the notifier and return 0.
> >> 
> >> For the current users of v4l2_async_notifier_parse_fwnode_endpoints()
> >> (omap3isp, rcar-vin, intel-ipu3), return -EINVAL in the callback
> >> for unavailable remote fwnodes to maintain the previous behavior.
> > 
> > I'm not sure this should be a per-driver decision.
> > 
> > Generally speaking, if an endpoint node has no remote-endpoint
> > property, the endpoint node is not needed. I've always considered such
> > an endpoint node as invalid. The OF graphs DT bindings are however not
> > clear on this subject.
>  
>  Documentation/devicetree/bindings/graph.txt says:
>    Each endpoint should contain a 'remote-endpoint' phandle property
>    that points to the corresponding endpoint in the port of the
>    remote device.
>  
>  ("should", not "must").
> >>> 
> >>> The DT bindings documentation has historically used "should" to mean
> >>> "must" in many places :-( That was a big mistake.
> >> 
> >> Maybe I could have worded that better? The intention was to let "should"
> >> be read as a strong suggestion, like "it is recommended", but not as a
> >> requirement.
> > 
> > Is there a reason for have an endpoint without a remote-endpoint property?
> 
> It allows to slightly reduce boilerplate in board device trees at the
> cost of empty endpoint nodes included from the dtsi in board DTs that
> don't use them.
> 
> > The problem with should (in general when it is used when the intention is
> > "shall") is that it lets the developer to write broken DT source that is
> > still conforming to the spec.
> > 
> > I don't have a strong preference to change should to shall in this
> > particular case now but I would have used "shall" to begin with.
> 
> I used "should" on purpose, but I'd be fine with giving up on it when
> all current users of this loophole are transitioned away from it:
> 
>   git grep -A1 "endpoint {" arch/ | grep -B1 "};"
> 
> I'm very much against enforcing a required remote-endpoint property in
> core DT parsing code, though.

Just to make it clear, I'm fine with making the property either optional or 
mandatory, but I would like the rule to be global, not per-bindings. When the 
OF graphs bindings were developed we reasoned that there was no use for empty 
endpoints and that they should thus be forbidden. Now, if we have good use 
cases for empty endpoints, I don't object them.

Regardless of what we decide I agree that we need to support existing device 
trees and must thus not reject empty endpoints as invalid. We could, however, 
if we decide to forbid empty endpoints, print a warning to encourage DT 
authors to fix the problem.

>  Later, the remote-node property explicitly lists the remote-endpoint
>  property as optional.
> >>> 
> >>> I've seen that too, and that's why I mentioned that the documentation
> >>> isn't clear on the subject.
> >> 
> >> Do you have a suggestion how to improve the documentation? I thought
> >> listing the remote-endpoint property under a header called "Optional
> >> endpoint properties" was pretty clear.
> >> 
> >>> This could also be achieved by adding the endpoints in the board DT
> >>> files. See for instance the hdmi@fead node in
> >>> arch/arm64/boot/dts/renesas/ r8a7795.dtsi and how it gets extended in
> >>> arch/arm64/boot/dts/renesas/r8a7795- salvator-x.dts. On the other
> >>> hand, I also have empty endpoints in the display@feb0 node of
> >>> arch/arm64/boot/dts/renesas/r8a7795.dtsi.
> >> 
> >> Right, that would be possible.
> >> 
> >>> I think we should first decide what we want to do going forward
> >>> (allowing for empty endpoints or not), clarify the documentation, and
> >>> then update the code. In any case I don't think it should be a
> >>> per-device decision.
> >> 
> >> There are device trees in the wild that have those empty endpoints, so I
> >> don't think retroactively declaring the remote-endpoint property
> >> required is a good idea.
> > 
> > You could IMO, but the kernel (and existing drivers) would still need to
> > work with DT binaries without those bits. And leave comments in the code
> > why it's 

Re: [PATCH] media: platform: Drop OF dependency of VIDEO_RENESAS_VSP1

[Laurent Pinchart]

Hi Geert,

Thank you for the patch.

On Monday, 26 February 2018 20:09:10 EET Geert Uytterhoeven wrote:
> VIDEO_RENESAS_VSP1 depends on ARCH_RENESAS && OF.
> As ARCH_RENESAS implies OF, the latter can be dropped.
> 
> Signed-off-by: Geert Uytterhoeven 

Acked-by: Laurent Pinchart 

and taken in my tree.

> ---
>  drivers/media/platform/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 614fbef08ddcabb0..2b8b1ad0edd9eb31 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -448,7 +448,7 @@ config VIDEO_RENESAS_FCP
>  config VIDEO_RENESAS_VSP1
>   tristate "Renesas VSP1 Video Processing Engine"
>   depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && HAS_DMA
> - depends on (ARCH_RENESAS && OF) || COMPILE_TEST
> + depends on ARCH_RENESAS || COMPILE_TEST
>   depends on (!ARM64 && !VIDEO_RENESAS_FCP) || VIDEO_RENESAS_FCP
>   select VIDEOBUF2_DMA_CONTIG
>   select VIDEOBUF2_VMALLOC

-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/13] media: v4l2-fwnode: Let parse_endpoint callback decide if no remote is error

[Philipp Zabel]

Hi Sakari,

On Fri, 2018-02-23 at 14:47 +0200, Sakari Ailus wrote:
> Hi Philipp,
> 
> On Fri, Feb 23, 2018 at 12:16:17PM +0100, Philipp Zabel wrote:
> > Hi Laurent,
> > 
> > On Fri, 2018-02-23 at 12:05 +0200, Laurent Pinchart wrote:
> > > Hi Philipp,
> > > 
> > > On Friday, 23 February 2018 11:56:52 EET Philipp Zabel wrote:
> > > > On Fri, 2018-02-23 at 11:29 +0200, Laurent Pinchart wrote:
> > > > > On Thursday, 22 February 2018 03:39:37 EET Steve Longerbeam wrote:
> > > > > > For some subdevices, a fwnode endpoint that has no connection to a
> > > > > > remote endpoint may not be an error. Let the parse_endpoint callback
> > > > > 
> > > > > make that decision in v4l2_async_notifier_fwnode_parse_endpoint(). If
> > > > > > the callback indicates that is not an error, skip adding the asd to 
> > > > > > the
> > > > > > notifier and return 0.
> > > > > > 
> > > > > > For the current users of 
> > > > > > v4l2_async_notifier_parse_fwnode_endpoints()
> > > > > > (omap3isp, rcar-vin, intel-ipu3), return -EINVAL in the callback for
> > > > > > unavailable remote fwnodes to maintain the previous behavior.
> > > > > 
> > > > > I'm not sure this should be a per-driver decision.
> > > > > 
> > > > > Generally speaking, if an endpoint node has no remote-endpoint 
> > > > > property,
> > > > > the endpoint node is not needed. I've always considered such an 
> > > > > endpoint
> > > > > node as invalid. The OF graphs DT bindings are however not clear on 
> > > > > this
> > > > > subject.
> > > > 
> > > > Documentation/devicetree/bindings/graph.txt says:
> > > > 
> > > >   Each endpoint should contain a 'remote-endpoint' phandle property
> > > >   that points to the corresponding endpoint in the port of the remote
> > > >   device.
> > > > 
> > > > ("should", not "must").
> > > 
> > > The DT bindings documentation has historically used "should" to mean 
> > > "must" in 
> > > many places :-( That was a big mistake.
> > 
> > Maybe I could have worded that better? The intention was to let "should"
> > be read as a strong suggestion, like "it is recommended", but not as a
> > requirement.
> 
> Is there a reason for have an endpoint without a remote-endpoint property?

It allows to slightly reduce boilerplate in board device trees at the
cost of empty endpoint nodes included from the dtsi in board DTs that
don't use them.

> The problem with should (in general when it is used when the intention is
> "shall") is that it lets the developer to write broken DT source that is
> still conforming to the spec.
>
> I don't have a strong preference to change should to shall in this
> particular case now but I would have used "shall" to begin with.

I used "should" on purpose, but I'd be fine with giving up on it when
all current users of this loophole are transitioned away from it:

  git grep -A1 "endpoint {" arch/ | grep -B1 "};"

I'm very much against enforcing a required remote-endpoint property in
core DT parsing code, though.

> > > > Later, the remote-node property explicitly lists the remote-endpoint
> > > > property as optional.
> > > 
> > > I've seen that too, and that's why I mentioned that the documentation 
> > > isn't 
> > > clear on the subject.
> > 
> > Do you have a suggestion how to improve the documentation? I thought
> > listing the remote-endpoint property under a header called "Optional
> > endpoint properties" was pretty clear.
> > 
> > > This could also be achieved by adding the endpoints in the board DT 
> > > files. See 
> > > for instance the hdmi@fead node in arch/arm64/boot/dts/renesas/
> > > r8a7795.dtsi and how it gets extended in 
> > > arch/arm64/boot/dts/renesas/r8a7795-
> > > salvator-x.dts. On the other hand, I also have empty endpoints in the 
> > > display@feb0 node of arch/arm64/boot/dts/renesas/r8a7795.dtsi.
> > 
> > Right, that would be possible.
> > 
> > > I think we should first decide what we want to do going forward (allowing 
> > > for 
> > > empty endpoints or not), clarify the documentation, and then update the 
> > > code. 
> > > In any case I don't think it should be a per-device decision.
> > 
> > There are device trees in the wild that have those empty endpoints, so I
> > don't think retroactively declaring the remote-endpoint property
> > required is a good idea.
> 
> You could IMO, but the kernel (and existing drivers) would still need to
> work with DT binaries without those bits. And leave comments in the code
> why it's there.
>
> > 
> > Is there any driver that currently benefits from throwing an error on
> > empty endpoints in any way? I'd prefer to just let the core ignore empty
> > endpoints for all drivers.
> 
> Not necessarily, but it's overhead in parsing the DT as well as in the
> DT source and in the DT binary.

True. I suppose whether or not that is enough reason to change the
wording in the existing of-graph bindings is something to be decided on
the devicetree list (in Cc).

regards
Philipp

Re: [PATCH 02/15] v4l: vsp1: Remove outdated comment

[Laurent Pinchart]

Hi Sergei,

On Tuesday, 27 February 2018 10:22:25 EET Sergei Shtylyov wrote:
> On 2/27/2018 12:45 AM, Laurent Pinchart wrote:
> > The entities in the pipeline are all started when the LIF is setup.
> > Remove the outdated comment that state otherwise.
> 
> States?

You're right, will fix in v2.

> > Signed-off-by: Laurent Pinchart
> > 

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] media: imx: Don't initialize vars that won't be used

[Philipp Zabel]

On Mon, 2018-02-26 at 08:40 -0500, Mauro Carvalho Chehab wrote:
> As reported by gcc:
> 
>   + drivers/staging/media/imx/imx-media-csi.c: warning: variable 'input_fi' 
> set but not used [-Wunused-but-set-variable]:  => 671:33
>   + drivers/staging/media/imx/imx-media-csi.c: warning: variable 'pinctrl' 
> set but not used [-Wunused-but-set-variable]:  => 1742:18
> 
> input_fi is not used, so just remove it.
> 
> However, pinctrl should be used, as it devm_pinctrl_get_select_default()
> is declared with attribute warn_unused_result. What's missing there
> is an error handling code, in case it fails. Add it.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Reviewed-by: Philipp Zabel 

regards
Philipp

Re: [PATCH] media: platform: Drop OF dependency of VIDEO_RENESAS_VSP1

[Simon Horman]

On Mon, Feb 26, 2018 at 07:09:10PM +0100, Geert Uytterhoeven wrote:
> VIDEO_RENESAS_VSP1 depends on ARCH_RENESAS && OF.
> As ARCH_RENESAS implies OF, the latter can be dropped.
> 
> Signed-off-by: Geert Uytterhoeven 

My reasoning is that ARCH_RENESAS depends on ARCH_MULTI_V7,
which in turn depends on ARCH_MULTIPLATFORM which selects OF via USE_OF

Reviewed-by: Simon Horman 

Re: [PATCH] media: imx: add 8-bit grayscale support

[Philipp Zabel]

Hi Hans,

On Thu, 2018-02-15 at 15:43 +0100, Hans Verkuil wrote:
> Hi Philipp,
> 
> Can you let me know if/when I can merge this? It looks good, so when the other
> patch is merged, then this can be merged as well.

Thank you for the reminder, the required patch is now merged into
v4.16-rc3, commit 6d36b7fec60e ("gpu: ipu-cpmem: add 8-bit grayscale
support to ipu_cpmem_set_image").

regards
Philipp

Re: [PATCH v8 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)

[Yong]

On Tue, 27 Feb 2018 09:20:39 +0100
Philipp Zabel  wrote:

> On Tue, 2018-02-27 at 10:07 +0800, Yong Deng wrote:
> > Add binding documentation for Allwinner V3s CSI.
> > 
> > Acked-by: Maxime Ripard 
> > Acked-by: Sakari Ailus 
> > Reviewed-by: Rob Herring 
> > Signed-off-by: Yong Deng 
> > ---
> >  .../devicetree/bindings/media/sun6i-csi.txt| 59 
> > ++
> >  1 file changed, 59 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt 
> > b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> > new file mode 100644
> > index ..2ff47a9507a6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> > @@ -0,0 +1,59 @@
> > +Allwinner V3s Camera Sensor Interface
> > +-
> > +
> > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
> > +interface and CSI1 is used for parallel interface.
> > +
> > +Required properties:
> > +  - compatible: value must be "allwinner,sun8i-v3s-csi"
> > +  - reg: base address and size of the memory-mapped region.
> > +  - interrupts: interrupt associated to this IP
> > +  - clocks: phandles to the clocks feeding the CSI
> > +* bus: the CSI interface clock
> > +* mod: the CSI module clock
> > +* ram: the CSI DRAM clock
> > +  - clock-names: the clock names mentioned above
> > +  - resets: phandles to the reset line driving the CSI
> > +
> > +Each CSI node should contain one 'port' child node with one child 
> > 'endpoint'
> > +node, according to the bindings defined in
> > +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned
> > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel 
> > or
> > +Bt656 for CSI1.
> > +
> > +Endpoint node properties for CSI1
> > +-
> > +
> > +- remote-endpoint  : (required) a phandle to the bus receiver's endpoint
> > +  node
> > +- bus-width:   : (required) must be 8, 10, 12 or 16
> > +- pclk-sample  : (optional) (default: sample on falling edge)
> 
> It would be helpful to state that 1 is rising edge and 0 is falling
> edge, see for example ov5640.txt

I think this is already documented at 
Documentation/devicetree/bindings/media/video-interfaces.txt.

> 
> regards
> Philipp


Thanks,
Yong

Re: [PATCH 02/15] v4l: vsp1: Remove outdated comment

[Sergei Shtylyov]

Hello!

On 2/27/2018 12:45 AM, Laurent Pinchart wrote:


The entities in the pipeline are all started when the LIF is setup.
Remove the outdated comment that state otherwise.


   States?


Signed-off-by: Laurent Pinchart 

[...]

MBR, Sergei

Re: [PATCH v8 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI)

[Philipp Zabel]

On Tue, 2018-02-27 at 10:07 +0800, Yong Deng wrote:
> Add binding documentation for Allwinner V3s CSI.
> 
> Acked-by: Maxime Ripard 
> Acked-by: Sakari Ailus 
> Reviewed-by: Rob Herring 
> Signed-off-by: Yong Deng 
> ---
>  .../devicetree/bindings/media/sun6i-csi.txt| 59 
> ++
>  1 file changed, 59 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt 
> b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> new file mode 100644
> index ..2ff47a9507a6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
> @@ -0,0 +1,59 @@
> +Allwinner V3s Camera Sensor Interface
> +-
> +
> +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
> +interface and CSI1 is used for parallel interface.
> +
> +Required properties:
> +  - compatible: value must be "allwinner,sun8i-v3s-csi"
> +  - reg: base address and size of the memory-mapped region.
> +  - interrupts: interrupt associated to this IP
> +  - clocks: phandles to the clocks feeding the CSI
> +* bus: the CSI interface clock
> +* mod: the CSI module clock
> +* ram: the CSI DRAM clock
> +  - clock-names: the clock names mentioned above
> +  - resets: phandles to the reset line driving the CSI
> +
> +Each CSI node should contain one 'port' child node with one child 'endpoint'
> +node, according to the bindings defined in
> +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned
> +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or
> +Bt656 for CSI1.
> +
> +Endpoint node properties for CSI1
> +-
> +
> +- remote-endpoint: (required) a phandle to the bus receiver's endpoint
> +node
> +- bus-width: : (required) must be 8, 10, 12 or 16
> +- pclk-sample: (optional) (default: sample on falling edge)

It would be helpful to state that 1 is rising edge and 0 is falling
edge, see for example ov5640.txt

regards
Philipp