Re: omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera

[H. Nikolaus Schaller]

Hi Pavel,

> Am 25.06.2017 um 11:18 schrieb Pavel Machek :
> 
> Hi!
> 
>> * unfortunately we still get no image :(
>> 
>> The latter is likely a setup issue of our camera interface (OMAP3 ISP = 
>> Image Signal Processor) which
>> we were not yet able to solve. Oscilloscoping signals on the interface 
>> indicated that signals and
>> sync are correct. But we do not know since mplayer only shows a green screen.
> 
> What mplayer command line do you use? How did you set up the pipeline
> with media-ctl?
> 
> On kernel.org, I have tree called camera-fw5-6 , where camera works
> for me on n900. On gitlab, there's modifed fcam-dev, which can be used
> for testing.

We did have yet another (non-DT) camera driver and media-ctl working in with 
3.12.37,
but had no success yet to update it to work with modern kernels or drivers. It
is either that the (newer) drivers missing something or the media-ctl has 
changed.

Here is the log of our scripts with Hugues' driver and our latest setup:

root@letux:~# ./camera-demo sxga
DISPLAY=:0
XAUTHORITY=tcp
Camera: /dev/v4l-subdev8
Setting mode sxga
media-ctl -r
media-ctl -l '"ov965x":0 -> "OMAP3 ISP CCDC":0[1]'
media-ctl -l '"OMAP3 ISP CCDC":1 -> "OMAP3 ISP CCDC output":0[1]'
media-ctl -V '"ov965x":0 [UYVY2X8 1280x1024]'
media-ctl -V '"OMAP3 ISP CCDC":0 [UYVY2X8 1280x1024]'
media-ctl -V '"OMAP3 ISP CCDC":1 [UYVY 1280x1024]'
### starting mplayer in sxga mode ###
mplayer tv:// -vf rotate=2 -tv 
driver=v4l2:device=/dev/video2:outfmt=uyvy:width=1280:height=1024:fps=15 -vo x11
MPlayer2 2.0-728-g2c378c7-4+b1 (C) 2000-2012 MPlayer Team

Playing tv://.
Detected file format: TV
Selected driver: v4l2
 name: Video 4 Linux 2 input
 author: Martin Olschewski 
 comment: first try, more to come ;-)
v4l2: ioctl get standard failed: Invalid argument
Selected device: OMAP3 ISP CCDC output
 Capabilities:  video capture  video output  streaming
 supported norms:
 inputs: 0 = camera;
 Current input: 0
 Current format: unknown (0x0)
tv.c: norm_from_string(pal): Bogus norm parameter, setting default.
v4l2: ioctl enum norm failed: Inappropriate ioctl for device
Error: Cannot set norm!
Selected input hasn't got a tuner!
v4l2: ioctl set mute failed: Inappropriate ioctl for device
v4l2: ioctl query control failed: Inappropriate ioctl for device
v4l2: ioctl query control failed: Inappropriate ioctl for device
v4l2: ioctl query control failed: Inappropriate ioctl for device
v4l2: ioctl query control failed: Inappropriate ioctl for device
v4l2: ioctl streamon failed: Broken pipe
[ass] auto-open
Opening video filter: [rotate=2]
VIDEO:  1280x1024  15.000 fps0.0 kbps ( 0.0 kB/s)
Could not find matching colorspace - retrying with -vf scale...
Opening video filter: [scale]
[swscaler @ 0xb5ca9980]using unscaled uyvy422 -> yuv420p special converter
VO: [x11] 1024x1280 => 1024x1280 Planar YV12
[swscaler @ 0xb5ca9980]No accelerated colorspace conversion found from yuv420p 
to bgra.
Colorspace details not fully supported by selected vo.
Selected video codec: RAW UYVY [raw]
Audio: no sound
Starting playback...
V:   0.0  10/ 10 ??% ??% ??,?% 0 0 $<3>


MPlayer interrupted by signal 2 in module: filter_video
V:   0.0  11/ 11 ??% ??% ??,?% 0 0 $<3>
v4l2: ioctl set mute failed: Inappropriate ioctl for device
v4l2: 0 frames successfully processed, 0 frames dropped.

Exiting... (Quit)
root@letux:~#

BR and thanks,
Nikolaus



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [PATCH v1 1/6] DT bindings: add bindings for ov965x camera module

[H. Nikolaus Schaller]

> Am 24.06.2017 um 00:24 schrieb Suman Anna :
> 
> On 06/23/2017 01:59 PM, H. Nikolaus Schaller wrote:
>> Hi Suman,
>> 
>>> Am 23.06.2017 um 20:05 schrieb Suman Anna :
>>> 
>> 
>> Or does it just mean that it defines the property name?
> 
> Please read the documentation link I sent - it's in the very bottom and
> should have an example.
 
 I have seen it but it does not give me a good clue how to translate that 
 into
 correct omap3isp node setup in a specific DT. Rather it raises more 
 questions.
 Maybe because I don't understand completely what it is talking about.
 
 The fundamental question is if this "assigned-clock-rates" is already
 handled by ov965x->clk = devm_clk_get(&client->dev, NULL); ?
 
 Or should we define that for the omap3isp node?
 
 Then of course we need no new code and just use the right property names.
 And N900, N9 camera DTs should be updated.
>>> 
>>> Look up of_clk_set_defaults() function in drivers/clk/clk-conf.c. This
>>> function gets invoked usually during clock registration, and also gets
>>> called in platform_drv_probe(), so the parents and clocks do get
>>> configured before your driver gets probed. So, this provides a default
>>> configuration if these properties are supplied (in either clock nodes or
>>> actual device nodes), and if your driver needs to change the rates at
>>> runtime, then you would have to do that in the driver itself.
>> 
>> Ok, now I understand. Thanks!
>> 
>> Quite hidden, but nice feature. I would never have thought that it exists.
>> Especially as there are no examples around omap3isp cameras...
>> 
>> And an fgrep assigned-clock-rates shows not many use cases outside CPU/SoC
>> include files.
>> 
>> But interestingly arch/arm/boot/dts/at91sam9g25ek.dts uses it for an 
>> ovti,ov2640 camera...
>> 
>> So it seems that we just have to write:
>> 
>>  ov9655@30 {
>>  compatible = "ovti,ov9655";
>>  reg = <0x30>;
>>  clocks = <&isp 0>;  /* cam_clka */
>>  assigned-clocks = <&isp 0>;
>>  assigned-clock-rates = <2400>;
>>  };
> 
> Yeah, that looks alright and should work.

I have tested and it works that way.

Thanks,
Nikolaus

Re: [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant

[H. Nikolaus Schaller]

> Am 22.06.2017 um 17:05 schrieb Hugues Fruchet :
> 
> Add a first support of OV9655 variant.
> Because of register set slightly different from OV9650/9652,
> not all of the driver features are supported (controls).
> Supported resolutions are limited to VGA, QVGA, QQVGA.
> Supported format is limited to RGB565.
> Controls are limited to color bar test pattern for test purpose.
> 
> Signed-off-by: H. Nikolaus Schaller 
> Signed-off-by: Hugues Fruchet 
> ---
> drivers/media/i2c/Kconfig  |   4 +-
> drivers/media/i2c/ov9650.c | 486 ++---
> 2 files changed, 457 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index efea14d..a8f638c 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -594,11 +594,11 @@ config VIDEO_OV7670
> controller.
> 
> config VIDEO_OV9650
> - tristate "OmniVision OV9650/OV9652 sensor support"
> + tristate "OmniVision OV9650/OV9652/OV9655 sensor support"
>   depends on GPIOLIB && I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
>   ---help---
> This is a V4L2 sensor-level driver for the Omnivision
> -   OV9650 and OV9652 camera sensors.
> +   OV9650 and OV9652 and OV9655 camera sensors.
> 
> config VIDEO_VS6624
>   tristate "ST VS6624 sensor support"
> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
> index a9d268d..c0819af 100644
> --- a/drivers/media/i2c/ov9650.c
> +++ b/drivers/media/i2c/ov9650.c
> @@ -1,5 +1,5 @@
> /*
> - * Omnivision OV9650/OV9652 CMOS Image Sensor driver
> + * Omnivision OV9650/OV9652/OV9655 CMOS Image Sensor driver
>  *
>  * Copyright (C) 2013, Sylwester Nawrocki 
>  *
> @@ -7,6 +7,15 @@
>  * by Vladimir Fonov.
>  * Copyright (c) 2010, Vladimir Fonov
>  *
> + *
> + * Copyright (C) STMicroelectronics SA 2017
> + * Author: Hugues Fruchet  for STMicroelectronics.
> + *
> + * OV9655 initial support based on a driver written by H. Nikolaus Schaller:
> + *   
> http://git.goldelico.com/?p=gta04-kernel.git;a=shortlog;h=refs/heads/work/hns/video/ov9655
> + * OV9655 registers sequence from STM32CubeF7 embedded software, see:
> + *   
> https://developer.mbed.org/teams/ST/code/BSP_DISCO_F746NG/file/e1d9da7fe856/Drivers/BSP/Components/ov9655/ov9655.c
> + *
>  * This program is free software; you can redistribute it and/or modify
>  * it under the terms of the GNU General Public License version 2 as
>  * published by the Free Software Foundation.
> @@ -58,14 +67,21 @@
> #define REG_PID   0x0a/* Product ID MSB */
> #define REG_VER   0x0b/* Product ID LSB */
> #define REG_COM3  0x0c
> -#define  COM3_SWAP   0x40
> +#define  COM3_COLORBAR   0x80
> +#define  COM3_RGB565 0x00
> +#define  COM3_SWAP   0x40/* Doesn't work in RGB */
> +#define  COM3_RESETB 0x08
> #define  COM3_VARIOPIXEL1 0x04
> +#define  OV9655_SINGLEFRAME  0x01
> #define REG_COM4  0x0d/* Vario Pixels  */
> #define  COM4_VARIOPIXEL2 0x80
> +#define  OV9655_TRISTATE /* seems to have a different function */
> #define REG_COM5  0x0e/* System clock options */
> #define  COM5_SLAVE_MODE  0x10
> -#define  COM5_SYSTEMCLOCK48MHZ   0x80
> +#define  COM5_SYSTEMCLOCK48MHZ   0x80/* not on OV9655 */
> +#define  OV9655_EXPOSURESTEP 0x01
> #define REG_COM6  0x0f/* HREF & ADBLC options */
> +#define  COM6_BLC_OPTICAL0x40/* Optical black */
> #define REG_AECH  0x10/* Exposure value, AEC[9:2] */
> #define REG_CLKRC 0x11/* Clock control */
> #define  CLK_EXT  0x40/* Use external clock directly */
> @@ -74,13 +90,18 @@
> #define  COM7_RESET   0x80
> #define  COM7_FMT_MASK0x38
> #define  COM7_FMT_VGA 0x40
> -#define   COM7_FMT_CIF   0x20
> +#define  COM7_FMT_CIF0x20
> #define  COM7_FMT_QVGA0x10
> #define  COM7_FMT_QCIF0x08
> -#define   COM7_RGB   0x04
> -#define   COM7_YUV   0x00
> -#define   COM7_BAYER 0x01
> -#define   COM7_PBAYER0x05
> +#define  COM7_RGB0x04
> +#define  COM7_YUV0x00
> +#define  COM7_BAYER  0x01
> +#define  COM7_PBAYER 0x05
> +#define  OV9655_COM7_VGA 0x60
> +#define  OV9655_COM7_RAWRGB  0x00/* different format encoding */
> +#define  OV9655_COM7_RAWRGBINT   0x01
> +#define  OV9655_COM7_YUV 0x02
> +#define  OV9655_COM7_RGB 0x03
> #define REG_COM8  0x13/* AGC/AEC options */
> #define  COM8_FASTAEC 0x80/* Enable fast AGC/AEC */
> #define  COM8_AECSTEP 0x40/* Unlimited AEC step size */
> @@ -89,14 +110,23 @@
> #define  COM8_AWB 0x02/* White balance enable */
> #define  COM8_AEC 0x01/* Auto exposure enable */
> #define R

Re: [PATCH v1 0/6] Add support of OV9655 camera

[H. Nikolaus Schaller]

Hi Hugues,

> Am 23.06.2017 um 12:25 schrieb H. Nikolaus Schaller :
> 
> Hi Hugues,
> 
>> Am 22.06.2017 um 17:41 schrieb H. Nikolaus Schaller :
>> 
>> 
>>> Am 22.06.2017 um 17:05 schrieb Hugues Fruchet :
>>> 
>>> This patchset enables OV9655 camera support.
>>> 
>>> OV9655 support has been tested using STM32F4DIS-CAM extension board
>>> plugged on connector P1 of STM32F746G-DISCO board.
>>> Due to lack of OV9650/52 hardware support, the modified related code
>>> could not have been checked for non-regression.
>>> 
>>> First patches upgrade current support of OV9650/52 to prepare then
>>> introduction of OV9655 variant patch.
>>> Because of OV9655 register set slightly different from OV9650/9652,
>>> not all of the driver features are supported (controls). Supported
>>> resolutions are limited to VGA, QVGA, QQVGA.
>>> Supported format is limited to RGB565.
>>> Controls are limited to color bar test pattern for test purpose.
>>> 
>>> OV9655 initial support is based on a driver written by H. Nikolaus Schaller 
>>> [1].
>> 
>> Great!
> 
> Thanks again for picking up or work and trying to get it upstream.
> 
>> 
>> I will test as soon as possible.

Here are some more test results and fixes:

> 
> I have tried and had to fix some issues first:
> * gpio properties have a different name than in our approach (but that is 
> something maintainers have to decide and is easy to follow this or that way)
> * there is no clock-frequency property which makes the driver request a clock 
> frequency (something our camera interface expects this way)

This can indeed be replaced by assigned-clock-rates and no additional
driver code. So there is no need to implement anything new here.

> * there is no vana-supply regulator and we need that to power on/off the 
> camera on demand (reset and pwdn isn't enough in our hardware)

this is something we still need to have added by patch



> * for some unknown reason the driver does not load automatically from DT 
> compatibility string and needs to be explicitly modprobed

This turned out to be because the i2c device ids are upper case while 
compatible-strings
are lower-case. See comment for patch 6/6.

BR and looking forward to v2,
Nikolaus

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:   Mon Jun 26 05:00:18 CEST 2017
media-tree git hash:430e29d9c0f65d9653a0b7f7a56f1c6cd374b84b
media_build git hash:   a5ec7f00979b6c866911fb42507770727ff5afd4
v4l-utils git hash: 1ae9a7adea3766879935dfede90d5aefd954c786
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.9.0-164

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

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH v2 08/15] [media] cxd2880: Add top level of the driver

[Mauro Carvalho Chehab]

Em Sun, 25 Jun 2017 09:15:06 -0300
Mauro Carvalho Chehab  escreveu:

> Em Fri, 23 Jun 2017 10:02:39 -0300
> Mauro Carvalho Chehab  escreveu:
> 
> > Em Mon, 19 Jun 2017 16:56:13 +0900
> > "Takiguchi, Yasunari"  escreveu:
> >   
> > > >> +static int cxd2880_get_frontend_t(struct dvb_frontend *fe,
> > > >> +  struct dtv_frontend_properties *c)
> > > >> +{
> > > >> +  enum cxd2880_ret ret = CXD2880_RESULT_OK;
> > > >> +  int result = 0;
> > > >> +  struct cxd2880_priv *priv = NULL;
> > > >> +  enum cxd2880_dvbt_mode mode = CXD2880_DVBT_MODE_2K;
> > > >> +  enum cxd2880_dvbt_guard guard = CXD2880_DVBT_GUARD_1_32;
> > > >> +  struct cxd2880_dvbt_tpsinfo tps;
> > > >> +  enum cxd2880_tnrdmd_spectrum_sense sense;
> > > >> +  u16 snr = 0;
> > > >> +  int strength = 0;
> > > >> +  u32 pre_bit_err = 0, pre_bit_count = 0;
> > > >> +  u32 post_bit_err = 0, post_bit_count = 0;
> > > >> +  u32 block_err = 0, block_count = 0;
> > > >> +
> > > >> +  if ((!fe) || (!c)) {
> > > >> +  pr_err("%s: invalid arg\n", __func__);
> > > >> +  return -EINVAL;
> > > >> +  }
> > > >> +
> > > >> +  priv = (struct cxd2880_priv *)fe->demodulator_priv;
> > > >> +
> > > >> +  mutex_lock(priv->spi_mutex);
> > > >> +  ret = cxd2880_tnrdmd_dvbt_mon_mode_guard(&priv->tnrdmd,
> > > >> +   &mode, &guard);
> > > >> +  mutex_unlock(priv->spi_mutex);
> > > >> +  if (ret == CXD2880_RESULT_OK) {
> > > >> +  switch (mode) {
> > > >> +  case CXD2880_DVBT_MODE_2K:
> > > >> +  c->transmission_mode = TRANSMISSION_MODE_2K;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_MODE_8K:
> > > >> +  c->transmission_mode = TRANSMISSION_MODE_8K;
> > > >> +  break;
> > > >> +  default:
> > > >> +  c->transmission_mode = TRANSMISSION_MODE_2K;
> > > >> +  dev_err(&priv->spi->dev, "%s: get invalid mode 
> > > >> %d\n",
> > > >> +  __func__, mode);
> > > >> +  break;
> > > >> +  }
> > > >> +  switch (guard) {
> > > >> +  case CXD2880_DVBT_GUARD_1_32:
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_32;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_GUARD_1_16:
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_16;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_GUARD_1_8:
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_8;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_GUARD_1_4:
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_4;
> > > >> +  break;
> > > >> +  default:
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_32;
> > > >> +  dev_err(&priv->spi->dev, "%s: get invalid guard 
> > > >> %d\n",
> > > >> +  __func__, guard);
> > > >> +  break;
> > > >> +  }
> > > >> +  } else {
> > > >> +  c->transmission_mode = TRANSMISSION_MODE_2K;
> > > >> +  c->guard_interval = GUARD_INTERVAL_1_32;
> > > >> +  dev_dbg(&priv->spi->dev,
> > > >> +  "%s: ModeGuard err %d\n", __func__, ret);
> > > >> +  }
> > > >> +
> > > >> +  mutex_lock(priv->spi_mutex);
> > > >> +  ret = cxd2880_tnrdmd_dvbt_mon_tps_info(&priv->tnrdmd, &tps);
> > > >> +  mutex_unlock(priv->spi_mutex);
> > > >> +  if (ret == CXD2880_RESULT_OK) {
> > > >> +  switch (tps.hierarchy) {
> > > >> +  case CXD2880_DVBT_HIERARCHY_NON:
> > > >> +  c->hierarchy = HIERARCHY_NONE;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_HIERARCHY_1:
> > > >> +  c->hierarchy = HIERARCHY_1;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_HIERARCHY_2:
> > > >> +  c->hierarchy = HIERARCHY_2;
> > > >> +  break;
> > > >> +  case CXD2880_DVBT_HIERARCHY_4:
> > > >> +  c->hierarchy = HIERARCHY_4;
> > > >> +  break;
> > > >> +  default:
> > > >> +  c->hierarchy = HIERARCHY_NONE;
> > > >> +  dev_err(&priv->spi->dev,
> > > >> +  "%s: TPSInfo hierarchy invalid %d\n",
> > > >> +  __func__, tps.hierarchy);
> > > >> +  break;
> > > >> +  }
> > > >> +
> > > >> +  switch (tps.rate_hp) {
> > > >> +  case CXD2880_DVBT_CODERATE_1_2:
> > > >> +  c->c

[PATCH v2 1/7] [media] dvb-core/dvb_ca_en50221.c: State UNINITIALISED instead of INVALID

[Jasmin J.]

From: Ralph Metzler 

In case of a linkinit failure change to state UNINITIALISED to re-init
the CAM.

Original code change by Ralph Metzler, modified by Jasmin Jessich to match
Kernel code style.

Signed-off-by: Ralph Metzler 
Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index af694f2..80edbe8 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -1176,7 +1176,8 @@ static int dvb_ca_en50221_thread(void *data)
 
pr_err("dvb_ca adapter %d: DVB CAM link 
initialisation failed :(\n",
   ca->dvbdev->adapter->num);
-   ca->slot_info[slot].slot_state = 
DVB_CA_SLOTSTATE_INVALID;
+   ca->slot_info[slot].slot_state =
+   DVB_CA_SLOTSTATE_UNINITIALISED;
dvb_ca_en50221_thread_update_delay(ca);
break;
}
-- 
2.7.4

[PATCH v2 5/7] [staging] cxd2099/cxd2099.c: Removed useless printing in cxd2099 driver

[Jasmin J.]

From: Ralph Metzler 

campoll and read_data are called very often and the printouts are very
annoying and make the driver unusable. They seem to be left over from
developing the buffer mode.

Original code change by Ralph Metzler, modified by Jasmin Jessich to
match current Kernel code.

Signed-off-by: Ralph Metzler 
Signed-off-by: Jasmin Jessich 
---
 drivers/staging/media/cxd2099/cxd2099.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/cxd2099/cxd2099.c 
b/drivers/staging/media/cxd2099/cxd2099.c
index 60d8dd0..6426ff1 100644
--- a/drivers/staging/media/cxd2099/cxd2099.c
+++ b/drivers/staging/media/cxd2099/cxd2099.c
@@ -568,14 +568,10 @@ static int campoll(struct cxd *ci)
return 0;
write_reg(ci, 0x05, istat);
 
-   if (istat & 0x40) {
+   if (istat & 0x40)
ci->dr = 1;
-   dev_info(&ci->i2c->dev, "DR\n");
-   }
-   if (istat & 0x20) {
+   if (istat & 0x20)
ci->write_busy = 0;
-   dev_info(&ci->i2c->dev, "WC\n");
-   }
 
if (istat & 2) {
u8 slotstat;
@@ -628,7 +624,6 @@ static int read_data(struct dvb_ca_en50221 *ca, int slot, 
u8 *ebuf, int ecount)
campoll(ci);
mutex_unlock(&ci->lock);
 
-   dev_info(&ci->i2c->dev, "%s\n", __func__);
if (!ci->dr)
return 0;
 
-- 
2.7.4

[PATCH v2 3/7] [media] dvb-core/dvb_ca_en50221.c: Add block read/write functions

[Jasmin J.]

From: Ralph Metzler 

Some lower level drivers may work better when sending blocks of data
instead byte per byte. For this we need new function pointers in the
dvb_ca_en50221 protocol structure (read_data, write_data) and the protocol
needs to execute them, if they are defined.
Block data transmission is done in all states except LINKINIT.

Original code change by Ralph Metzler, modified by Jasmin Jessich and
Daniel Scheller to match Kernel code style.

Signed-off-by: Ralph Metzler 
Signed-off-by: Daniel Scheller 
Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 137 
 drivers/media/dvb-core/dvb_ca_en50221.h |   7 ++
 2 files changed, 92 insertions(+), 52 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 529e7ec..17970cd 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -645,72 +645,101 @@ static int dvb_ca_en50221_read_data(struct 
dvb_ca_private *ca, int slot,
}
buf_free = dvb_ringbuffer_free(&ca->slot_info[slot].rx_buffer);
 
-   if (buf_free < (ca->slot_info[slot].link_buf_size + 
DVB_RINGBUFFER_PKTHDRSIZE)) {
+   if (buf_free < (ca->slot_info[slot].link_buf_size +
+   DVB_RINGBUFFER_PKTHDRSIZE)) {
status = -EAGAIN;
goto exit;
}
}
 
-   /* check if there is data available */
-   if ((status = ca->pub->read_cam_control(ca->pub, slot, CTRLIF_STATUS)) 
< 0)
-   goto exit;
-   if (!(status & STATUSREG_DA)) {
-   /* no data */
-   status = 0;
-   goto exit;
-   }
-
-   /* read the amount of data */
-   if ((status = ca->pub->read_cam_control(ca->pub, slot, 
CTRLIF_SIZE_HIGH)) < 0)
-   goto exit;
-   bytes_read = status << 8;
-   if ((status = ca->pub->read_cam_control(ca->pub, slot, 
CTRLIF_SIZE_LOW)) < 0)
-   goto exit;
-   bytes_read |= status;
+   if (ca->pub->read_data &&
+   (ca->slot_info[slot].slot_state != DVB_CA_SLOTSTATE_LINKINIT)) {
+   if (ebuf == NULL)
+   status = ca->pub->read_data(ca->pub, slot, buf,
+   sizeof(buf));
+   else
+   status = ca->pub->read_data(ca->pub, slot, buf, ecount);
+   if (status < 0)
+   return status;
+   bytes_read =  status;
+   if (status == 0)
+   goto exit;
+   } else {
 
-   /* check it will fit */
-   if (ebuf == NULL) {
-   if (bytes_read > ca->slot_info[slot].link_buf_size) {
-   pr_err("dvb_ca adapter %d: CAM tried to send a buffer 
larger than the link buffer size (%i > %i)!\n",
-  ca->dvbdev->adapter->num, bytes_read,
-  ca->slot_info[slot].link_buf_size);
-   ca->slot_info[slot].slot_state = 
DVB_CA_SLOTSTATE_LINKINIT;
-   status = -EIO;
+   /* check if there is data available */
+   status = ca->pub->read_cam_control(ca->pub, slot,
+  CTRLIF_STATUS);
+   if (status < 0)
goto exit;
-   }
-   if (bytes_read < 2) {
-   pr_err("dvb_ca adapter %d: CAM sent a buffer that was 
less than 2 bytes!\n",
-  ca->dvbdev->adapter->num);
-   ca->slot_info[slot].slot_state = 
DVB_CA_SLOTSTATE_LINKINIT;
-   status = -EIO;
+   if (!(status & STATUSREG_DA)) {
+   /* no data */
+   status = 0;
goto exit;
}
-   } else {
-   if (bytes_read > ecount) {
-   pr_err("dvb_ca adapter %d: CAM tried to send a buffer 
larger than the ecount size!\n",
-  ca->dvbdev->adapter->num);
-   status = -EIO;
+
+   /* read the amount of data */
+   status = ca->pub->read_cam_control(ca->pub, slot,
+  CTRLIF_SIZE_HIGH);
+   if (status < 0)
+   goto exit;
+   bytes_read = status << 8;
+   status = ca->pub->read_cam_control(ca->pub, slot,
+  CTRLIF_SIZE_LOW);
+   if (status < 0)
goto exit;
+   bytes_read |= status;
+
+   /* check it will fit */
+   if (ebuf == NULL) {
+   if (bytes_read > ca->slot_info[slot].link_buf_size) {
+   pr_err("dvb_ca adapter %d

[PATCH v2 0/7] Add block read/write to en50221 CAM functions

[Jasmin J.]

From: Jasmin Jessich 

This is now the V2 version of the patch series with preserved author and
little checkpatch fixes. I also combined some patches which needs to be
applied at once to.

Changes from v1 to v2:
 - Preserved authorship of original author.
 - All patches tested with checkpatch.pl (no errors).
 - Patch "Set maximum cxd2099 block size to 512" is now part of patch
   "Fixed buffer mode", because this needs to be applied together.
 - Patch "Removed useless printing in cxd2099 driver" is now split into a
   part which is already upstream and an additional one.
 - Rebased to media_tree/master.
 
These patch series implement a block read/write interface to the en50221
CAM control functions. The origin of this patches can be found in the
Digital Devices Git on https://github.com/DigitalDevices/dddvb maintained
by Ralph Metzler  .
 
The relevant changes concerning dvb-core/dvb_ca_en50221.c/.h and
cxd2099/cxd2099.c/.h have been extracted from the mentioned repository by
Daniel Scheller  and committed to his branch on
https://github.com/herrnst/dddvb-linux-kernel/tree/mediatree/master-cxd2099
 
I split the patch set in smaller pieces for easier review, fixed code style
issues in cxd2099/cxd2099.c/.h and dvb_ca_en50221.c (checkpatch.pl) and
tested the resulting driver on my hardware with the DD DuoFlex CI (single)
card. I tested if the CAM communication is working with VDR:
 vdr: [2414] CAM 1: module ready
 vdr: [2414] CAM 1: AlphaCrypt, 01, 4A20, 4A20
 vdr: [2414] CAM 1: system ids: 0D95 0648 1702 1722 1762 4A20 0500 0B00
0100 1833 1834 0D05 0D22
 vdr: [2414] CAM 1: replies to QUERY - multi channel decryption (MCD)
possible
 vdr: [2414] CAM 1: supports multi transponder decryption (MTD)
 vdr: [2414] CAM 1: activating MTD support
 vdr: [2405] CAM 1: ready, master (AlphaCrypt)

Please note, that the block read/write functionality is already implemented
in the currently existing cxd2099/cxd2099.c/.h driver, but deactivated. The
existing code in this driver is also not functional and has been updated by
the working implementation from the Digital Devices Git.
 
Additionally to the block read/write functions, I merged also two patches
in the en50221 CAM control state machine, which were existing in the
Digital Devices Git. This are the first two patches of this series.
 
There is another patch series coming "Fix coding style in en50221 CAM
functions" which fixes some of the style issues in
dvb-core/dvb_ca_en50221.c/.h, based on this patch series. I will send this
after this series has been accepted.

Jasmin Jessich (2):
  [staging] cxd2099/cxd2099.c: Removed printing in write_block
  [staging] cxd2099/cxd2099.c: Activate cxd2099 buffer mode

Ralph Metzler (5):
  [media] dvb-core/dvb_ca_en50221.c: State UNINITIALISED instead of
INVALID
  [media] dvb-core/dvb_ca_en50221.c: Increase timeout for link init
  [media] dvb-core/dvb_ca_en50221.c: Add block read/write functions
  [staging] cxd2099/cxd2099.c/.h: Fixed buffer mode
  [staging] cxd2099/cxd2099.c: Removed useless printing in cxd2099
driver

 drivers/media/dvb-core/dvb_ca_en50221.c| 143 +++--
 drivers/media/dvb-core/dvb_ca_en50221.h|   7 ++
 drivers/media/pci/ddbridge/ddbridge-core.c |   1 +
 drivers/staging/media/cxd2099/cxd2099.c| 165 -
 drivers/staging/media/cxd2099/cxd2099.h|   6 +-
 5 files changed, 217 insertions(+), 105 deletions(-)

-- 
2.7.4

[PATCH v2 2/7] [media] dvb-core/dvb_ca_en50221.c: Increase timeout for link init

[Jasmin J.]

From: Ralph Metzler 

Some CAMs do a really slow initialization, which requires a longer timeout
for the first response.

Original code change by Ralph Metzler, modified by Jasmin Jessich to match
Kernel code style.

Signed-off-by: Ralph Metzler 
Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-core/dvb_ca_en50221.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_ca_en50221.c 
b/drivers/media/dvb-core/dvb_ca_en50221.c
index 80edbe8..529e7ec 100644
--- a/drivers/media/dvb-core/dvb_ca_en50221.c
+++ b/drivers/media/dvb-core/dvb_ca_en50221.c
@@ -349,7 +349,8 @@ static int dvb_ca_en50221_link_init(struct dvb_ca_private 
*ca, int slot)
/* read the buffer size from the CAM */
if ((ret = ca->pub->write_cam_control(ca->pub, slot, CTRLIF_COMMAND, 
IRQEN | CMDREG_SR)) != 0)
return ret;
-   if ((ret = dvb_ca_en50221_wait_if_status(ca, slot, STATUSREG_DA, HZ / 
10)) != 0)
+   ret = dvb_ca_en50221_wait_if_status(ca, slot, STATUSREG_DA, HZ);
+   if (ret != 0)
return ret;
if ((ret = dvb_ca_en50221_read_data(ca, slot, buf, 2)) != 2)
return -EIO;
-- 
2.7.4

[PATCH v2 6/7] [staging] cxd2099/cxd2099.c: Removed printing in write_block

[Jasmin J.]

From: Jasmin Jessich 

There were remaining debug prints which haven't been found earlier due to
the disabled buffer mode (see commit "Removed useless printing in cxd2099
driver" for the already removed printings.

Signed-off-by: Jasmin Jessich 
---
 drivers/staging/media/cxd2099/cxd2099.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/media/cxd2099/cxd2099.c 
b/drivers/staging/media/cxd2099/cxd2099.c
index 6426ff1..3431cf6 100644
--- a/drivers/staging/media/cxd2099/cxd2099.c
+++ b/drivers/staging/media/cxd2099/cxd2099.c
@@ -230,7 +230,6 @@ static int write_block(struct cxd *ci, u8 adr, u8 *data, 
u16 n)
status = i2c_write_reg(ci->i2c, ci->cfg.adr, 0, adr);
if (status)
return status;
-   dev_info(&ci->i2c->dev, "write_block %d\n", n);
 
ci->lastaddress = adr;
buf[0] = 1;
@@ -239,7 +238,6 @@ static int write_block(struct cxd *ci, u8 adr, u8 *data, 
u16 n)
 
if (ci->cfg.max_i2c && (len + 1 > ci->cfg.max_i2c))
len = ci->cfg.max_i2c - 1;
-   dev_info(&ci->i2c->dev, "write %d\n", len);
memcpy(buf + 1, data, len);
status = i2c_write(ci->i2c, ci->cfg.adr, buf, len + 1);
if (status)
@@ -652,7 +650,6 @@ static int write_data(struct dvb_ca_en50221 *ca, int slot, 
u8 *ebuf, int ecount)
if (ci->write_busy)
return -EAGAIN;
mutex_lock(&ci->lock);
-   dev_info(&ci->i2c->dev, "%s %d\n", __func__, ecount);
write_reg(ci, 0x0d, ecount >> 8);
write_reg(ci, 0x0e, ecount & 0xff);
write_block(ci, 0x11, ebuf, ecount);
-- 
2.7.4

[PATCH v2 7/7] [staging] cxd2099/cxd2099.c: Activate cxd2099 buffer mode

[Jasmin J.]

From: Jasmin Jessich 

Activate the cxd2099 buffer mode.

Signed-off-by: Jasmin Jessich 
---
 drivers/staging/media/cxd2099/cxd2099.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/cxd2099/cxd2099.c 
b/drivers/staging/media/cxd2099/cxd2099.c
index 3431cf6..f28916e 100644
--- a/drivers/staging/media/cxd2099/cxd2099.c
+++ b/drivers/staging/media/cxd2099/cxd2099.c
@@ -33,7 +33,8 @@
 
 #include "cxd2099.h"
 
-/* #define BUFFER_MODE 1 */
+/* comment this line to deactivate the cxd2099ar buffer mode */
+#define BUFFER_MODE 1
 
 static int read_data(struct dvb_ca_en50221 *ca, int slot, u8 *ebuf, int 
ecount);
 
-- 
2.7.4

[PATCH v2 4/7] [staging] cxd2099/cxd2099.c/.h: Fixed buffer mode

[Jasmin J.]

From: Ralph Metzler 

The buffer mode was already implemented in this driver, but it did not work
as expected. This has been fixed now, but it is still deactivated and can
be activated by removing a comment at the begin of the file.

Original code change by Ralph Metzler, modified by Jasmin Jessich and
Daniel Scheller to match Kernel code style.

Signed-off-by: Ralph Metzler 
Signed-off-by: Daniel Scheller 
Signed-off-by: Jasmin Jessich 
---
 drivers/media/pci/ddbridge/ddbridge-core.c |   1 +
 drivers/staging/media/cxd2099/cxd2099.c| 162 +
 drivers/staging/media/cxd2099/cxd2099.h|   6 +-
 3 files changed, 123 insertions(+), 46 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
b/drivers/media/pci/ddbridge/ddbridge-core.c
index 32f4d37..cd1723e 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -1305,6 +1305,7 @@ static struct cxd2099_cfg cxd_cfg = {
.adr =  0x40,
.polarity = 1,
.clock_mode = 1,
+   .max_i2c = 512,
 };
 
 static int ddb_ci_attach(struct ddb_port *port)
diff --git a/drivers/staging/media/cxd2099/cxd2099.c 
b/drivers/staging/media/cxd2099/cxd2099.c
index 370ecb9..60d8dd0 100644
--- a/drivers/staging/media/cxd2099/cxd2099.c
+++ b/drivers/staging/media/cxd2099/cxd2099.c
@@ -1,7 +1,7 @@
 /*
  * cxd2099.c: Driver for the CXD2099AR Common Interface Controller
  *
- * Copyright (C) 2010-2011 Digital Devices GmbH
+ * Copyright (C) 2010-2013 Digital Devices GmbH
  *
  *
  * This program is free software; you can redistribute it and/or
@@ -33,7 +33,9 @@
 
 #include "cxd2099.h"
 
-#define MAX_BUFFER_SIZE 248
+/* #define BUFFER_MODE 1 */
+
+static int read_data(struct dvb_ca_en50221 *ca, int slot, u8 *ebuf, int 
ecount);
 
 struct cxd {
struct dvb_ca_en50221 en;
@@ -48,6 +50,7 @@ struct cxd {
intmode;
intready;
intdr;
+   intwrite_busy;
intslot_stat;
 
u8 amem[1024];
@@ -55,6 +58,9 @@ struct cxd {
 
intcammode;
struct mutex lock;
+
+   u8 rbuf[1028];
+   u8 wbuf[1028];
 };
 
 static int i2c_write_reg(struct i2c_adapter *adapter, u8 adr,
@@ -73,7 +79,7 @@ static int i2c_write_reg(struct i2c_adapter *adapter, u8 adr,
 }
 
 static int i2c_write(struct i2c_adapter *adapter, u8 adr,
-u8 *data, u8 len)
+u8 *data, u16 len)
 {
struct i2c_msg msg = {.addr = adr, .flags = 0, .buf = data, .len = len};
 
@@ -100,12 +106,12 @@ static int i2c_read_reg(struct i2c_adapter *adapter, u8 
adr,
 }
 
 static int i2c_read(struct i2c_adapter *adapter, u8 adr,
-   u8 reg, u8 *data, u8 n)
+   u8 reg, u8 *data, u16 n)
 {
struct i2c_msg msgs[2] = {{.addr = adr, .flags = 0,
-.buf = ®, .len = 1},
-   {.addr = adr, .flags = I2C_M_RD,
-.buf = data, .len = n} };
+  .buf = ®, .len = 1},
+ {.addr = adr, .flags = I2C_M_RD,
+  .buf = data, .len = n} };
 
if (i2c_transfer(adapter, msgs, 2) != 2) {
dev_err(&adapter->dev, "error in i2c_read\n");
@@ -114,14 +120,26 @@ static int i2c_read(struct i2c_adapter *adapter, u8 adr,
return 0;
 }
 
-static int read_block(struct cxd *ci, u8 adr, u8 *data, u8 n)
+static int read_block(struct cxd *ci, u8 adr, u8 *data, u16 n)
 {
-   int status;
+   int status = 0;
 
-   status = i2c_write_reg(ci->i2c, ci->cfg.adr, 0, adr);
+   if (ci->lastaddress != adr)
+   status = i2c_write_reg(ci->i2c, ci->cfg.adr, 0, adr);
if (!status) {
ci->lastaddress = adr;
-   status = i2c_read(ci->i2c, ci->cfg.adr, 1, data, n);
+
+   while (n) {
+   int len = n;
+
+   if (ci->cfg.max_i2c && (len > ci->cfg.max_i2c))
+   len = ci->cfg.max_i2c;
+   status = i2c_read(ci->i2c, ci->cfg.adr, 1, data, len);
+   if (status)
+   return status;
+   data += len;
+   n -= len;
+   }
}
return status;
 }
@@ -182,16 +200,16 @@ static int write_io(struct cxd *ci, u16 address, u8 val)
 
 static int write_regm(struct cxd *ci, u8 reg, u8 val, u8 mask)
 {
-   int status;
+   int status = 0;
 
-   status = i2c_write_reg(ci->i2c, ci->cfg.adr, 0, reg);
+   if (ci->lastaddress != reg)
+   status = i2c_write_reg(ci->i2c, ci->cfg.adr, 0, reg);
if (!status && reg >= 6 && reg <= 8 && mask != 0xff)
status = i2c_read_reg(ci->i2c, ci->cfg.adr, 1, &ci->regs[reg]);
+   ci->lastaddress = reg;
ci->regs[reg] = (ci->regs[reg] & (~mask)) | val;
-   if (!status) {
-   

Re: [PATCH v3 2/2] v4l: async: add subnotifier registration for subdevices

[Sylwester Nawrocki]

On 06/13/2017 04:30 PM, Niklas Söderlund wrote:

When the registered() callback of v4l2_subdev_internal_ops is called the
subdevice has access to the master devices v4l2_dev and it's called with
the async frameworks list_lock held. In this context the subdevice can
register its own notifiers to allow for incremental discovery of
subdevices.

The master device registers the subdevices closest to itself in its
notifier while the subdevice(s) register notifiers for their closest
neighboring devices when they are registered. Using this incremental
approach two problems can be solved:

1. The master device no longer has to care how many devices exist in
the pipeline. It only needs to care about its closest subdevice and
arbitrary long pipelines can be created without having to adapt the
master device for each case.

2. Subdevices which are represented as a single DT node but register
more than one subdevice can use this to improve the pipeline
discovery, since the subdevice driver is the only one who knows which
of its subdevices is linked with which subdevice of a neighboring DT
node.

To enable subdevices to register/unregister notifiers from the
registered()/unregistered() callback v4l2_async_subnotifier_register()
and v4l2_async_subnotifier_unregister() are added. These new notifier
register functions are similar to the master device equivalent functions
but run without taking the v4l2-async list_lock which already is held
when the registered()/unregistered() callbacks are called.

Signed-off-by: Niklas Söderlund
Acked-by: Hans Verkuil
Acked-by: Sakari Ailus


Acked-by: Sylwester Nawrocki 

Re: [PATCH v3 1/2] v4l: async: check for v4l2_dev in v4l2_async_notifier_register()

[Sylwester Nawrocki]

On 06/13/2017 04:30 PM, Niklas Söderlund wrote:

Add a check for v4l2_dev to v4l2_async_notifier_register() as to fail as
early as possible since this will fail later in v4l2_async_test_notify().

Signed-off-by: Niklas Söderlund
Acked-by: Sakari Ailus
Acked-by: Hans Verkuil


Acked-by: Sylwester Nawrocki 

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

[Sylwester Nawrocki]

On 06/20/2017 07:26 PM, Jose Abreu wrote:
> This is an initial submission for the Synopsys Designware HDMI RX
> Controller Driver. This driver interacts with a phy driver so that
> a communication between them is created and a video pipeline is
> configured.

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

> diff --git a/drivers/media/platform/dwc/dw-hdmi-rx.c 
> b/drivers/media/platform/dwc/dw-hdmi-rx.c
> new file mode 100644
> index 000..22ee51d
> --- /dev/null
> +++ b/drivers/media/platform/dwc/dw-hdmi-rx.c

> +static const struct of_device_id dw_hdmi_supported_phys[] = {
> + { .compatible = "snps,dw-hdmi-phy-e405", .data = DW_PHY_E405_DRVNAME, },
> + { },
> +};
> +
> +static struct device_node *dw_hdmi_get_phy_of_node(struct dw_hdmi_dev 
> *dw_dev,
> + const struct of_device_id **found_id)
> +{
> + struct device_node *child = NULL;
> + const struct of_device_id *id;
> +
> + for_each_child_of_node(dw_dev->of_node, child) {
> + id = of_match_node(dw_hdmi_supported_phys, child);
> + if (id)
> + break;
> + }
> +
> + if (found_id)
> + *found_id = id;
> +
> + return child;
> +}
> +
> +static int dw_hdmi_phy_init(struct dw_hdmi_dev *dw_dev)
> +{
> + struct dw_phy_pdata *phy = &dw_dev->phy_config;
> + const struct of_device_id *of_id;
> + struct of_dev_auxdata lookup;

struct of_dev_auxdata lookup = { };

You could initialize to 0 here and...

> + struct device_node *child;
> + const char *drvname;
> + int ret;
> +
> + child = dw_hdmi_get_phy_of_node(dw_dev, &of_id);
> + if (!child || !of_id || !of_id->data) {
> + dev_err(dw_dev->dev, "no supported phy found in DT\n");
> + return -EINVAL;
> + }
> +
> + drvname = of_id->data;
> + phy->funcs = &dw_hdmi_phy_funcs;
> + phy->funcs_arg = dw_dev;
> +
> + lookup.compatible = (char *)of_id->compatible;
> + lookup.phys_addr = 0x0;
> + lookup.name = NULL;

...drop these two assignments.

> + lookup.platform_data = phy;
> +
> + request_module(drvname);

I'd say this request_module() is not needed when you use the v4l2-async 
subnotifiers and the modules are properly installed in the file system.
I might be missing something though.

> + ret = of_platform_populate(dw_dev->of_node, NULL, &lookup, dw_dev->dev);
> + if (ret) {
> + dev_err(dw_dev->dev, "failed to populate phy driver\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void dw_hdmi_phy_exit(struct dw_hdmi_dev *dw_dev)
> +{
> + of_platform_depopulate(dw_dev->dev);
> +}

> +static int dw_hdmi_registered(struct v4l2_subdev *sd)
> +{
> + struct dw_hdmi_dev *dw_dev = to_dw_dev(sd);
> + int ret;
> +
> + ret = cec_register_adapter(dw_dev->cec_adap, dw_dev->dev);
> + if (ret) {
> + dev_err(dw_dev->dev, "failed to register CEC adapter\n");
> + cec_delete_adapter(dw_dev->cec_adap);
> + return ret;
> + }
> +
> + cec_register_cec_notifier(dw_dev->cec_adap, dw_dev->cec_notifier);
> + dw_dev->registered = true;
> + return ret;
> +}
> +
> +static void dw_hdmi_unregistered(struct v4l2_subdev *sd)
> +{
> + struct dw_hdmi_dev *dw_dev = to_dw_dev(sd);
> +
> + cec_unregister_adapter(dw_dev->cec_adap);
> + cec_notifier_put(dw_dev->cec_notifier);
> +}
> +

> +static const struct v4l2_su

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

[Sylwester Nawrocki]

Hi Jose,

Thank you for addressing my review comments. Couple more suggestions below.

On 06/20/2017 07:26 PM, Jose Abreu wrote:
> This adds support for the Synopsys Designware HDMI RX PHY e405. This
> phy receives and decodes HDMI video that is delivered to a controller.

> +static int dw_phy_config(struct dw_phy_dev *dw_dev, unsigned char 
> color_depth,
> + bool hdmi2, bool scrambling)
> +{

> + phy_reset(dw_dev, 1);
> + phy_pddq(dw_dev, 1);
> + phy_svsmode(dw_dev, 1);
> +
> + phy_zcal_reset(dw_dev);
> + do {
> + udelay(1000);

Could be mdelay(1) or better e.g. usleep_range(1000, 1100);

> + zcal_done = phy_zcal_done(dw_dev);
> + } while (!zcal_done && timeout--);
> +
> + if (!zcal_done) {
> + dev_err(dw_dev->dev, "Zcal calibration failed\n");
> + return -ETIMEDOUT;
> + }

> + return 0;
> +}

> +static int dw_phy_probe(struct platform_device *pdev)
> +{
> + struct dw_phy_pdata *pdata = pdev->dev.platform_data;
> + struct device *dev = &pdev->dev;
> + struct dw_phy_dev *dw_dev;
> + struct v4l2_subdev *sd;
> + int ret;
> +
> + dev_dbg(dev, "probe start\n");
> +
> + /* Resource allocation */

This comment is not needed.

> + dw_dev = devm_kzalloc(dev, sizeof(*dw_dev), GFP_KERNEL);
> + if (!dw_dev)
> + return -ENOMEM;
> +
> + /* Resource initialization */

Ditto.

> + if (!pdata) {
> + dev_err(dev, "no platform data suplied\n");
> + return -EINVAL;
> + }
> +
> + dw_dev->dev = dev;
> + dw_dev->config = pdata;
> + dw_dev->version = 405;

How about storing the version number in the dw_hdmi_phy_e405_id[] table
and retrieving it here with of_device_get_match_data() ?

> + mutex_init(&dw_dev->lock);
> +
> + /* Get config clock value */

The comment is not needed, it's clear from the code we are getting the clock 
and its rate.

> + dw_dev->clk = devm_clk_get(dev, "cfg-clk");

As Rob suggested, it would be good to change name of the clock to just "cfg".

> + if (IS_ERR(dw_dev->clk)) {
> + dev_err(dev, "failed to get cfg-clk\n");
> + return PTR_ERR(dw_dev->clk);
> + }
> +
> + ret = clk_prepare_enable(dw_dev->clk);
> + if (ret) {
> + dev_err(dev, "failed to enable cfg-clk\n");
> + return ret;
> + }
> +
> + dw_dev->cfg_clk = clk_get_rate(dw_dev->clk) / 100;

100U ?

> + if (!dw_dev->cfg_clk) {
> + dev_err(dev, "invalid cfg-clk frequency\n");> + ret = 
> -EINVAL;
> + goto err_clk;
> + }
> +
> + /* V4L2 initialization */
> + sd = &dw_dev->sd;
> + v4l2_subdev_init(sd, &dw_phy_sd_ops);
> + strlcpy(sd->name, dev_name(dev), sizeof(sd->name));
> + sd->dev = dev;
> +
> + /* Force phy disabling */
> + dw_dev->phy_enabled = true;
> + dw_phy_disable(dw_dev);
> +
> + /* Register subdev */
> + ret = v4l2_async_register_subdev(sd);
> + if (ret) {
> + dev_err(dev, "failed to register subdev\n");
> + goto err_clk;
> + }
> +
> + /* All done */

Superfluous comment.

> + dev_set_drvdata(dev, sd);
> + dev_info(dev, "driver probed (cfg-clk=%d)\n", dw_dev->cfg_clk);

This should be at dev_dbg() level.

> + return 0;
> +
> +err_clk:
> + clk_disable_unprepare(dw_dev->clk);
> + return ret;
> +}
> +
> +static int dw_phy_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;

The assignment here could be dropped and &pdev->dev used directly below.

> + struct v4l2_subdev *sd = dev_get_drvdata(dev);

> + struct dw_phy_dev *dw_dev = to_dw_dev(sd);
> +
> + v4l2_async_unregister_subdev(sd);
> + clk_disable_unprepare(dw_dev->clk);

> + dev_info(dev, "driver removed\n");

This should be at dev_dbg() level or dropped entirely.

> + return 0;
> +}
> +
> +static const struct of_device_id dw_hdmi_phy_e405_id[] = {
> + { .compatible = "snps,dw-hdmi-phy-e405" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, dw_hdmi_phy_e405_id);

--
Regards,
Sylwester

Re: [PATCH v1 5/6] [media] ov9650: add multiple variant support (fwd)

[Julia Lawall]

Braces are probably missing aroud lines 1618-1620.

julia

-- Forwarded message --
Date: Sun, 25 Jun 2017 23:06:03 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: Re: [PATCH v1 5/6] [media] ov9650: add multiple variant support

Hi Hugues,

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

url:
https://github.com/0day-ci/linux/commits/Hugues-Fruchet/Add-support-of-OV9655-camera/20170625-201153
base:   git://linuxtv.org/media_tree.git master
:: branch date: 3 hours ago
:: commit date: 3 hours ago

>> drivers/media/i2c/ov9650.c:1618:2-44: code aligned with following code on 
>> line 1619

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a9fe8c23240a7f8df39c6238d98e41f41fedb641
vim +1618 drivers/media/i2c/ov9650.c

a9fe8c23 Hugues Fruchet 2017-06-22  1612ov965x->set_params = 
__ov965x_set_params;
84a15ded Sylwester Nawrocki 2012-12-26  1613
a9fe8c23 Hugues Fruchet 2017-06-22  1614ov965x->frame_size = 
&ov965x->framesizes[0];
a9fe8c23 Hugues Fruchet 2017-06-22  1615
ov965x_get_default_format(ov965x, &ov965x->format);
a9fe8c23 Hugues Fruchet 2017-06-22  1616
a9fe8c23 Hugues Fruchet 2017-06-22  1617if (ov965x->initialize_controls)
a9fe8c23 Hugues Fruchet 2017-06-22 @1618ret = 
ov965x->initialize_controls(ov965x);
84a15ded Sylwester Nawrocki 2012-12-26 @1619if (ret < 0)
84a15ded Sylwester Nawrocki 2012-12-26  1620goto err_ctrls;
84a15ded Sylwester Nawrocki 2012-12-26  1621
84a15ded Sylwester Nawrocki 2012-12-26  1622/* Update exposure time min/max 
to match frame format */

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

Re: DD support improvements (was: Re: [PATCH v3 00/13] stv0367/ddbridge: support CTv6/FlexCT hardware)

[Daniel Scheller]

Am Sat, 24 Jun 2017 13:50:01 -0300
schrieb Mauro Carvalho Chehab :

> Em Thu, 22 Jun 2017 23:35:27 +0200
> Ralph Metzler  escreveu:
> 
> Would it be possible to change things at the dddvb tree to make
> it to use our coding style (for example, replacing CamelCase by the
> kernel_style), in order to minimize the amount of work to sync from
> your tree?

Note that this mostly (if not only) applies to the demodulator drivers. 
ddbridge itself is okay in this regard and has only some minors like indent, 
whitespace and such. There's one bigger thing though I'm not sure of if it 
needs to be changed: Beginning with the 0.9.9-tarball release, functionality 
was split from ddbridge-core.c into ddbridge.c, ddbridge-i2c.c, ddbridge-mod.c 
and ddbridge-ns.c (the two latter being modulator and netstream/octonet related 
code, which we don't need at this time). The issue is that this wasn't done by 
updating the build system to build multiple objects, but rather build from 
ddbridge.c which then does '#include "ddbridge-core.c"', and in that file 
'#include "ddbridge-i2c.c"'. See [1] for how it actually looks like in the 
file. Mauro, do you think this is acceptable?

> > Regarding divergence in the tuner/demod drivers I see some concerns. 
> > The TDA18212 driver as they are presently in kernel and Daniel's  github 
> > tree still seems to be missing features
> > like calibration and spur avoidance. This problem was already discussed 
> > here a few years ago.
> > I would not want to move to these versions if those features are still 
> > missing.  
> 
> I don't see any issue on adding the missing features to the existing
> tda18212 driver. Maybe Daniel can help adding the missing features there.
> 
> The best would be to make those new features opt-in, in order to allow 
> drivers to gradually use them (after tests), avoiding regressions.

I already started something when I searched for possible reasons for the 
stv0367 I2C bus crashes and implemented the tuner calibration (this wasn't the 
reason in the end, but still), see [2]. Guess a config flag like in [3] will 
work. But I'd need advice in what parts are required to be ported over if I 
should do this.

> > - adding SYS_DVBC2 to fe_delivery_system   
> 
> OK, we can do that, when adding a driver needing such feature.

I might volunteer in adding DVB-C2 support to cxd2841er in porting needed bits 
over from the cxd2843 driver, but someone else need to do testing on a DVB-C2 
enabled coax cable.

Best regards,
Daniel Scheller

[1] 
https://github.com/herrnst/dddvb-linux-kernel/blob/17d60ca45dd0294120882af9abbbdf9e5a130cb5/drivers/media/pci/ddbridge/ddbridge.c#L50
[2] 
https://github.com/herrnst/dddvb-linux-kernel/commit/0788bd5e05fffdcd2d00d1fa2732c9712c6c759d
[3] https://patchwork.linuxtv.org/patch/40710/
-- 
https://github.com/herrnst

[PATCH] ov9650: fix semicolon.cocci warnings

[kbuild test robot]

drivers/media/i2c/ov9650.c:2034:2-3: Unneeded semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Fixes: 5ffa34fa8f2e ("ov9650: add support of OV9655 variant")
CC: Hugues Fruchet 
Signed-off-by: Fengguang Wu 
---

 ov9650.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/media/i2c/ov9650.c
+++ b/drivers/media/i2c/ov9650.c
@@ -2031,7 +2031,7 @@ static int ov965x_probe(struct i2c_clien
ov965x->formats = ov9655_formats;
ov965x->nb_of_formats = ARRAY_SIZE(ov9655_formats);
ov965x->set_params = ov965x_set_frame_size;
-   };
+   }
 
ov965x->frame_size = &ov965x->framesizes[0];
ov965x_get_default_format(ov965x, &ov965x->format);

Re: [PATCH v1 6/6] [media] ov9650: add support of OV9655 variant

[kbuild test robot]

Hi Hugues,

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

url:
https://github.com/0day-ci/linux/commits/Hugues-Fruchet/Add-support-of-OV9655-camera/20170625-201153
base:   git://linuxtv.org/media_tree.git master


coccinelle warnings: (new ones prefixed by >>)

>> drivers/media/i2c/ov9650.c:2034:2-3: Unneeded semicolon

Please review and possibly fold the followup patch.

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

[PATCH 1/2] media: dtv-core.rst: add an introduction to FE kAPI

[Mauro Carvalho Chehab]

Instead of just start describing the kAPI functions, add
an introduction giving a general line about a DVB driver's
structure.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/kapi/dtv-core.rst | 33 +
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/Documentation/media/kapi/dtv-core.rst 
b/Documentation/media/kapi/dtv-core.rst
index bec7875a7e2e..1430f0b7e615 100644
--- a/Documentation/media/kapi/dtv-core.rst
+++ b/Documentation/media/kapi/dtv-core.rst
@@ -1,6 +1,31 @@
 Digital TV (DVB) devices
 
 
+Digital TV devices are implemented by several different drivers:
+
+- A bridge driver that is responsible to talk with the bus where the other
+  devices are connected (PCI, USB, SPI), bind to the other drivers and
+  implement the digital demux logic (either in software or in hardware);
+
+- Frontend drivers that are usually implemented as two separate drivers:
+
+  - A tuner driver that implements the logic with commands the part of the
+hardware with is reponsible to tune into a digital TV transponder or
+physical channel. The output of a tuner is usually a baseband or
+Intermediate Frequency (IF) signal;
+
+  - A demodulator driver (a.k.a "demod") that implements the logic with
+commands the digital TV decoding hardware. The output of a demod is
+a digital stream, with multiple audio, video and data channels typically
+multiplexed using MPEG Transport Stream [#f1]_.
+
+On most hardware, the frontend drivers talk with the bridge driver using an
+I2C bus.
+
+.. [#f1] Some standards use TCP/IP for multiplexing data, like DVB-H (an
+   abandoned standard, not used anymore) and ATSC version 3.0 current
+   proposals. Currently, the DVB subsystem doesn't implement those standards.
+
 Digital TV Common functions
 ---
 
@@ -87,7 +112,7 @@ and measuring the quality of service.
 For each statistics measurement, the driver should set the type of scale used,
 or ``FE_SCALE_NOT_AVAILABLE`` if the statistics is not available on a given
 time. Drivers should also provide the number of statistics for each type.
-that's usually 1 for most video standards [#f1]_.
+that's usually 1 for most video standards [#f2]_.
 
 Drivers should initialize each statistic counters with length and
 scale at its init code. For example, if the frontend provides signal
@@ -103,7 +128,7 @@ And, when the statistics got updated, set the scale::
c->strength.stat[0].scale = FE_SCALE_DECIBEL;
c->strength.stat[0].uvalue = strength;
 
-.. [#f1] For ISDB-T, it may provide both a global statistics and a per-layer
+.. [#f2] For ISDB-T, it may provide both a global statistics and a per-layer
set of statistics. On such cases, len should be equal to 4. The first
value corresponds to the global stat; the other ones to each layer, e. g.:
 
@@ -129,13 +154,13 @@ Signal strength (:ref:`DTV-STAT-SIGNAL-STRENGTH`)
 at the maximum value (so, strength is on its minimal).
 
   - As the gain is visible through the set of registers that adjust the gain,
-typically, this statistics is always available [#f2]_.
+typically, this statistics is always available [#f3]_.
 
   - Drivers should try to make it available all the times, as this statistics
 can be used when adjusting an antenna position and to check for troubles
 at the cabling.
 
-  .. [#f2] On a few devices, the gain keeps floating if no carrier.
+  .. [#f3] On a few devices, the gain keeps floating if no carrier.
  On such devices, strength report should check first if carrier is
  detected at the tuner (``FE_HAS_CARRIER``, see :c:type:`fe_status`),
  and otherwise return the lowest possible value.
-- 
2.9.4

[PATCH 2/2] media: dtv-core.rst: complete description of a demod driver

[Mauro Carvalho Chehab]

A section talking about demod statistics implementation was
recently added, but it seemed to start from nowere, without
a previous description about how a demod driver would look
like.

Add such description, for completeness.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/kapi/dtv-core.rst | 139 +-
 1 file changed, 137 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/kapi/dtv-core.rst 
b/Documentation/media/kapi/dtv-core.rst
index 1430f0b7e615..de9a228aca8a 100644
--- a/Documentation/media/kapi/dtv-core.rst
+++ b/Documentation/media/kapi/dtv-core.rst
@@ -80,8 +80,141 @@ Digital TV Frontend
 The Digital TV Frontend kABI defines a driver-internal interface for
 registering low-level, hardware specific driver to a hardware independent
 frontend layer. It is only of interest for Digital TV device driver writers.
-The header file for this API is named dvb_frontend.h and located in
-drivers/media/dvb-core.
+The header file for this API is named ``dvb_frontend.h`` and located in
+``drivers/media/dvb-core``.
+
+Demodulator driver
+^^
+
+The demodulator driver is responsible to talk with the decoding part of the
+hardware. Such driver should implement :c:type:`dvb_frontend_ops`, with
+tells what type of digital TV standards are supported, and points to a
+series of functions that allow the DVB core to command the hardware via
+the code under ``drivers/media/dvb-core/dvb_frontend.c``.
+
+A typical example of such struct in a driver ``foo`` is::
+
+   static struct dvb_frontend_ops foo_ops = {
+   .delsys = { SYS_DVBT, SYS_DVBT2, SYS_DVBC_ANNEX_A },
+   .info = {
+   .name   = "foo DVB-T/T2/C driver",
+   .caps = FE_CAN_FEC_1_2 |
+   FE_CAN_FEC_2_3 |
+   FE_CAN_FEC_3_4 |
+   FE_CAN_FEC_5_6 |
+   FE_CAN_FEC_7_8 |
+   FE_CAN_FEC_AUTO |
+   FE_CAN_QPSK |
+   FE_CAN_QAM_16 |
+   FE_CAN_QAM_32 |
+   FE_CAN_QAM_64 |
+   FE_CAN_QAM_128 |
+   FE_CAN_QAM_256 |
+   FE_CAN_QAM_AUTO |
+   FE_CAN_TRANSMISSION_MODE_AUTO |
+   FE_CAN_GUARD_INTERVAL_AUTO |
+   FE_CAN_HIERARCHY_AUTO |
+   FE_CAN_MUTE_TS |
+   FE_CAN_2G_MODULATION,
+   .frequency_min = 4200, /* Hz */
+   .frequency_max = 100200, /* Hz */
+   .symbol_rate_min = 87,
+   .symbol_rate_max = 1170
+   },
+   .init = foo_init,
+   .sleep = foo_sleep,
+   .release = foo_release,
+   .set_frontend = foo_set_frontend,
+   .get_frontend = foo_get_frontend,
+   .read_status = foo_get_status_and_stats,
+   .tune = foo_tune,
+   .i2c_gate_ctrl = foo_i2c_gate_ctrl,
+   .get_frontend_algo = foo_get_algo,
+   };
+
+A typical example of such struct in a driver ``bar`` meant to be used on
+Satellite TV reception is::
+
+   static const struct dvb_frontend_ops bar_ops = {
+   .delsys = { SYS_DVBS, SYS_DVBS2 },
+   .info = {
+   .name   = "Bar DVB-S/S2 demodulator",
+   .frequency_min  = 50, /* KHz */
+   .frequency_max  = 250, /* KHz */
+   .frequency_stepsize = 0,
+   .symbol_rate_min = 100,
+   .symbol_rate_max = 4500,
+   .symbol_rate_tolerance = 500,
+   .caps = FE_CAN_INVERSION_AUTO |
+   FE_CAN_FEC_AUTO |
+   FE_CAN_QPSK,
+   },
+   .init = bar_init,
+   .sleep = bar_sleep,
+   .release = bar_release,
+   .set_frontend = bar_set_frontend,
+   .get_frontend = bar_get_frontend,
+   .read_status = bar_get_status_and_stats,
+   .i2c_gate_ctrl = bar_i2c_gate_ctrl,
+   .get_frontend_algo = bar_get_algo,
+   .tune = bar_tune,
+
+   /* Satellite-specific */
+   .diseqc_send_master_cmd = bar_send_diseqc_msg,
+   .diseqc_send_burst = bar_send_burst,
+   .set_tone = bar_set_tone,
+   .set_voltage = bar_set_voltage,
+   };
+
+.. note::
+
+   #) For satellite digital TV standards (DVB-S, DVB-S2, ISDB-S), the
+  frequencies are specified in kHz, while, for terrestrial and cable
+  standards, they're specified in 

Re: [PATCH v1 3/5] [media] stm32-dcmi: crop sensor image to match user resolution

[kbuild test robot]

Hi Hugues,

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

url:
https://github.com/0day-ci/linux/commits/Hugues-Fruchet/Camera-support-on-STM32F746G-DISCO-board/20170625-204425
base:   git://linuxtv.org/media_tree.git master


coccinelle warnings: (new ones prefixed by >>)

>> drivers/media/platform/stm32/stm32-dcmi.c:808:2-3: Unneeded semicolon
   drivers/media/platform/stm32/stm32-dcmi.c:562:2-3: Unneeded semicolon
   drivers/media/platform/stm32/stm32-dcmi.c:762:2-3: Unneeded semicolon

Please review and possibly fold the followup patch.

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

[PATCH] stm32-dcmi: fix semicolon.cocci warnings

[kbuild test robot]

drivers/media/platform/stm32/stm32-dcmi.c:808:2-3: Unneeded semicolon
drivers/media/platform/stm32/stm32-dcmi.c:562:2-3: Unneeded semicolon
drivers/media/platform/stm32/stm32-dcmi.c:762:2-3: Unneeded semicolon


 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Fixes: a8a270dedb25 ("stm32-dcmi: crop sensor image to match user resolution")
CC: Hugues Fruchet 
Signed-off-by: Fengguang Wu 
---

 stm32-dcmi.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -559,7 +559,7 @@ static int dcmi_start_streaming(struct v
 
/* Enable crop */
val |= CR_CROP;
-   };
+   }
 
reg_write(dcmi->regs, DCMI_CR, val);
 
@@ -759,7 +759,7 @@ static int dcmi_try_fmt(struct stm32_dcm
/* Restore width/height */
pixfmt->width = width;
pixfmt->height = height;
-   };
+   }
 
pixfmt->field = V4L2_FIELD_NONE;
pixfmt->bytesperline = pixfmt->width * dcmi_fmt->bpp;
@@ -805,7 +805,7 @@ static int dcmi_set_fmt(struct stm32_dcm
mf->width, mf->height,
dcmi->crop.width, dcmi->crop.height,
dcmi->crop.left, dcmi->crop.top);
-   };
+   }
 
dcmi->fmt = *f;
dcmi->current_fmt = current_fmt;

Re: [PATCH v1 5/6] [media] ov9650: add multiple variant support

[kbuild test robot]

Hi Hugues,

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

url:
https://github.com/0day-ci/linux/commits/Hugues-Fruchet/Add-support-of-OV9655-camera/20170625-201153
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-x079-06251032 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/media//i2c/ov9650.c: In function 'ov965x_probe':
>> drivers/media//i2c/ov9650.c:1617:2: warning: this 'if' clause does not 
>> guard... [-Wmisleading-indentation]
 if (ov965x->initialize_controls)
 ^~
   drivers/media//i2c/ov9650.c:1619:3: note: ...this statement, but the latter 
is misleadingly indented as if it is guarded by the 'if'
  if (ret < 0)
  ^~
   Cyclomatic Complexity 1 include/linux/err.h:PTR_ERR
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls
   Cyclomatic Complexity 1 include/linux/device.h:dev_get_drvdata
   Cyclomatic Complexity 1 include/asm-generic/gpio.h:gpio_is_valid
   Cyclomatic Complexity 1 include/linux/i2c.h:i2c_get_clientdata
   Cyclomatic Complexity 1 include/media/media-entity.h:media_entity_cleanup
   Cyclomatic Complexity 1 include/media/v4l2-subdev.h:v4l2_get_subdevdata
   Cyclomatic Complexity 1 drivers/media//i2c/ov9650.c:ctrl_to_sd
   Cyclomatic Complexity 1 drivers/media//i2c/ov9650.c:to_ov965x
   Cyclomatic Complexity 1 drivers/media//i2c/ov9650.c:ov965x_get_default_format
   Cyclomatic Complexity 2 drivers/media//i2c/ov9650.c:ov965x_enum_mbus_code
   Cyclomatic Complexity 4 drivers/media//i2c/ov9650.c:ov965x_enum_frame_sizes
   Cyclomatic Complexity 6 drivers/media//i2c/ov9650.c:__ov965x_try_frame_size
   Cyclomatic Complexity 1 drivers/media//i2c/ov9650.c:ov965x_i2c_driver_init
   Cyclomatic Complexity 1 drivers/media//i2c/ov9650.c:ov965x_remove
   Cyclomatic Complexity 8 
drivers/media//i2c/ov9650.c:__ov965x_set_frame_interval
   Cyclomatic Complexity 5 drivers/media//i2c/ov9650.c:ov965x_read
   Cyclomatic Complexity 11 drivers/media//i2c/ov9650.c:__g_volatile_ctrl
   Cyclomatic Complexity 3 drivers/media//i2c/ov9650.c:ov965x_write
   Cyclomatic Complexity 3 drivers/media//i2c/ov9650.c:ov965x_write_array
   Cyclomatic Complexity 2 drivers/media//i2c/ov9650.c:ov965x_set_frame_size
   Cyclomatic Complexity 3 
drivers/media//i2c/ov9650.c:ov965x_set_default_gamma_curve
   Cyclomatic Complexity 3 drivers/media//i2c/ov9650.c:ov965x_set_color_matrix
   Cyclomatic Complexity 5 drivers/media//i2c/ov9650.c:ov965x_set_white_balance
   Cyclomatic Complexity 3 drivers/media//i2c/ov9650.c:ov965x_set_brightness
   Cyclomatic Complexity 12 drivers/media//i2c/ov9650.c:ov965x_set_gain
   Cyclomatic Complexity 3 drivers/media//i2c/ov9650.c:ov965x_set_flip
   Cyclomatic Complexity 3 drivers/media//i2c/ov9650.c:ov965x_set_saturation
   Cyclomatic Complexity 6 drivers/media//i2c/ov9650.c:ov965x_set_sharpness
   Cyclomatic Complexity 3 drivers/media//i2c/ov9650.c:ov965x_set_test_pattern
   Cyclomatic Complexity 7 drivers/media//i2c/ov9650.c:ov965x_set_banding_filter
   Cyclomatic Complexity 10 drivers/media//i2c/ov9650.c:__ov965x_set_params
   Cyclomatic Complexity 1 include/linux/err.h:IS_ERR
   Cyclomatic Complexity 1 include/media/v4l2-ctrls.h:v4l2_ctrl_lock
   Cyclomatic Complexity 1 include/media/v4l2-ctrls.h:v4l2_ctrl_unlock
   Cyclomatic Complexity 2 drivers/media//i2c/ov9650.c:ov965x_g_volatile_ctrl
   Cyclomatic Complexity 3 drivers/media//i2c/ov9650.c:ov965x_s_frame_interval
   Cyclomatic Complexity 2 drivers/media//i2c/ov9650.c:ov965x_g_frame_interval
   Cyclomatic Complexity 1 include/media/v4l2-ctrls.h:v4l2_ctrl_modify_range
   Cyclomatic Complexity 7 
drivers/media//i2c/ov9650.c:ov965x_update_exposure_ctrl
   Cyclomatic Complexity 3 
drivers/media//i2c/ov9650.c:ov965x_initialize_controls
   Cyclomatic Complexity 10 drivers/media//i2c/ov9650.c:ov965x_set_exposure
   Cyclomatic Complexity 12 drivers/media//i2c/ov9650.c:ov965x_s_ctrl
   Cyclomatic Complexity 1 
include/media/v4l2-subdev.h:v4l2_subdev_get_try_format
   Cyclomatic Complexity 1 drivers/media//i2c/ov9650.c:ov965x_open
   Cyclomatic Complexity 11 drivers/media//i2c/ov9650.c:ov965x_set_fmt
   Cyclomatic Complexity 2 drivers/media//i2c/ov9650.c:ov965x_get_fmt
   Cyclomatic Complexity 13 drivers/media//i2c/ov9650.c:ov965x_s_stream
   Cyclomatic Complexity 2 drivers/media//i2c/ov9650.c:ov965x_gpio_set
   Cyclomatic Complexity 2 drivers/media//i2c/ov9650.c:__ov965x_set_power
   Cyclomatic Complexity 7 drivers/media//i2c/ov9650.c:ov965x_s_power
   Cyclomatic Complexity 4 drivers/media//i2c/ov9650.c:ov965x_detect_sensor
   Cyclomatic Complexity 1 include/linux/device.h:devm_kzalloc
   Cyclomatic Complexity 1 include/asm-gene

[PATCH 18/19] ir-lirc-codec: move the remaining fops over from lirc_dev

[David Härdeman]

ir-lirc-codec is the only user of these functions, so instead of exporting them
from lirc_dev, move all of them over to ir-lirc-codec.

This means that all ir-lirc-codec fops can be found next to each other in
ir-lirc-codec.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c |  181 -
 drivers/media/rc/lirc_dev.c  |  187 --
 include/media/lirc_dev.h |8 --
 3 files changed, 178 insertions(+), 198 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index f914a3d5a468..05f88401f694 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -13,6 +13,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -21,6 +22,7 @@
 #include "rc-core-priv.h"
 
 #define LIRCBUF_SIZE 256
+#define LOGHEAD"lirc_dev (%s[%d]): "
 
 /**
  * ir_lirc_decode() - Send raw IR data to lirc_dev to be relayed to the
@@ -369,6 +371,177 @@ static long ir_lirc_ioctl(struct file *filep, unsigned 
int cmd,
return ret;
 }
 
+static ssize_t ir_lirc_read(struct file *file, char __user *buffer,
+   size_t length, loff_t *ppos)
+{
+   struct lirc_dev *d = file->private_data;
+   unsigned char buf[d->buf->chunk_size];
+   int ret, written = 0;
+   DECLARE_WAITQUEUE(wait, current);
+
+   dev_dbg(&d->dev, LOGHEAD "read called\n", d->name, d->minor);
+
+   ret = mutex_lock_interruptible(&d->mutex);
+   if (ret)
+   return ret;
+
+   if (!d->attached) {
+   ret = -ENODEV;
+   goto out_locked;
+   }
+
+   if (!LIRC_CAN_REC(d->features)) {
+   ret = -EINVAL;
+   goto out_locked;
+   }
+
+   if (length % d->buf->chunk_size) {
+   ret = -EINVAL;
+   goto out_locked;
+   }
+
+   /*
+* we add ourselves to the task queue before buffer check
+* to avoid losing scan code (in case when queue is awaken somewhere
+* between while condition checking and scheduling)
+*/
+   add_wait_queue(&d->buf->wait_poll, &wait);
+
+   /*
+* while we didn't provide 'length' bytes, device is opened in blocking
+* mode and 'copy_to_user' is happy, wait for data.
+*/
+   while (written < length && ret == 0) {
+   if (lirc_buffer_empty(d->buf)) {
+   /* According to the read(2) man page, 'written' can be
+* returned as less than 'length', instead of blocking
+* again, returning -EWOULDBLOCK, or returning
+* -ERESTARTSYS
+*/
+   if (written)
+   break;
+   if (file->f_flags & O_NONBLOCK) {
+   ret = -EWOULDBLOCK;
+   break;
+   }
+   if (signal_pending(current)) {
+   ret = -ERESTARTSYS;
+   break;
+   }
+
+   mutex_unlock(&d->mutex);
+   set_current_state(TASK_INTERRUPTIBLE);
+   schedule();
+   set_current_state(TASK_RUNNING);
+
+   ret = mutex_lock_interruptible(&d->mutex);
+   if (ret) {
+   remove_wait_queue(&d->buf->wait_poll, &wait);
+   goto out_unlocked;
+   }
+
+   if (!d->attached) {
+   ret = -ENODEV;
+   goto out_locked;
+   }
+   } else {
+   lirc_buffer_read(d->buf, buf);
+   ret = copy_to_user((void __user *)buffer+written, buf,
+  d->buf->chunk_size);
+   if (!ret)
+   written += d->buf->chunk_size;
+   else
+   ret = -EFAULT;
+   }
+   }
+
+   remove_wait_queue(&d->buf->wait_poll, &wait);
+
+out_locked:
+   mutex_unlock(&d->mutex);
+
+out_unlocked:
+   return ret ? ret : written;
+}
+
+static unsigned int ir_lirc_poll(struct file *file, poll_table *wait)
+{
+   struct lirc_dev *d = file->private_data;
+   unsigned int ret;
+
+   if (!d->attached)
+   return POLLHUP | POLLERR;
+
+   if (d->buf) {
+   poll_wait(file, &d->buf->wait_poll, wait);
+
+   if (lirc_buffer_empty(d->buf))
+   ret = 0;
+   else
+   ret = POLLIN | POLLRDNORM;
+   } else
+   ret = POLLERR;
+
+   dev_dbg(&d->dev, LOGHEAD "poll result = %d\n", d->name, d->minor, ret);
+
+   return r

[PATCH 19/19] lirc_dev: consistent device registration printk

[David Härdeman]

This patch changes the message that is printed on lirc device registration to
make it more consistent with the input and rc subsystems.

Before:
  rc rc0: rc-core loopback device as /devices/virtual/rc/rc0
  input: rc-core loopback device as /devices/virtual/rc/rc0/input43
  lirc lirc0: lirc_dev: driver ir-lirc-codec (rc-loopback) registered at minor 
= 0

After:
  rc rc0: rc-core loopback device as /devices/virtual/rc/rc0
  input: rc-core loopback device as /devices/virtual/rc/rc0/input23
  lirc lirc0: rc-core loopback device as /devices/virtual/rc/rc0/lirc0

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c |3 +--
 drivers/media/rc/lirc_dev.c  |6 --
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index 05f88401f694..4f33516a95a3 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -595,8 +595,7 @@ static int ir_lirc_register(struct rc_dev *dev)
if (dev->max_timeout)
features |= LIRC_CAN_SET_REC_TIMEOUT;
 
-   snprintf(ldev->name, sizeof(ldev->name), "ir-lirc-codec (%s)",
-dev->driver_name);
+   snprintf(ldev->name, sizeof(ldev->name), "%s", dev->input_name);
ldev->features = features;
ldev->data = &dev->raw->lirc;
ldev->buf = NULL;
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index c1c917932f7e..03430a1fb192 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -105,6 +105,7 @@ int lirc_register_device(struct lirc_dev *d)
 {
int minor;
int err;
+   const char *path;
 
if (!d) {
pr_err("driver pointer must be not NULL!\n");
@@ -171,8 +172,9 @@ int lirc_register_device(struct lirc_dev *d)
return err;
}
 
-   dev_info(&d->dev, "lirc_dev: driver %s registered at minor = %d\n",
-d->name, d->minor);
+   path = kobject_get_path(&d->dev.kobj, GFP_KERNEL);
+   dev_info(&d->dev, "%s as %s\n", d->name, path ?: "N/A");
+   kfree(path);
 
return 0;
 }

[PATCH 17/19] ir-lirc-codec: merge lirc_dev_fop_ioctl into ir_lirc_ioctl

[David Härdeman]

ir_lirc_ioctl() is the only caller of lirc_dev_fop_ioctl() so merging the
latter into the former makes the code more readable. At the same time, this
allows the locking situation in ir_lirc_ioctl() to be fixed by holding
the lirc_dev mutex during the whole ioctl call.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c |  168 --
 drivers/media/rc/lirc_dev.c  |   59 -
 include/media/lirc_dev.h |1 
 3 files changed, 105 insertions(+), 123 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index ba20a4ce9cbc..f914a3d5a468 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -178,12 +178,13 @@ static ssize_t ir_lirc_transmit_ir(struct file *file, 
const char __user *buf,
 }
 
 static long ir_lirc_ioctl(struct file *filep, unsigned int cmd,
-   unsigned long arg)
+ unsigned long arg)
 {
struct lirc_codec *lirc;
struct rc_dev *dev;
+   struct lirc_dev *d;
u32 __user *argp = (u32 __user *)(arg);
-   int ret = 0;
+   int ret;
__u32 val = 0, tmp;
 
lirc = lirc_get_pdata(filep);
@@ -194,10 +195,23 @@ static long ir_lirc_ioctl(struct file *filep, unsigned 
int cmd,
if (!dev)
return -EFAULT;
 
+   d = lirc->ldev;
+   if (!d)
+   return -EFAULT;
+
+   ret = mutex_lock_interruptible(&d->mutex);
+   if (ret)
+   return ret;
+
+   if (!d->attached) {
+   ret = -ENODEV;
+   goto out;
+   }
+
if (_IOC_DIR(cmd) & _IOC_WRITE) {
ret = get_user(val, argp);
if (ret)
-   return ret;
+   goto out;
}
 
switch (cmd) {
@@ -205,125 +219,153 @@ static long ir_lirc_ioctl(struct file *filep, unsigned 
int cmd,
/* legacy support */
case LIRC_GET_SEND_MODE:
if (!dev->tx_ir)
-   return -ENOTTY;
-
-   val = LIRC_MODE_PULSE;
+   ret = -ENOTTY;
+   else
+   val = LIRC_MODE_PULSE;
break;
 
case LIRC_SET_SEND_MODE:
if (!dev->tx_ir)
-   return -ENOTTY;
-
-   if (val != LIRC_MODE_PULSE)
-   return -EINVAL;
-   return 0;
+   ret = -ENOTTY;
+   else if (val != LIRC_MODE_PULSE)
+   ret = -EINVAL;
+   break;
 
/* TX settings */
case LIRC_SET_TRANSMITTER_MASK:
if (!dev->s_tx_mask)
-   return -ENOTTY;
-
-   return dev->s_tx_mask(dev, val);
+   ret = -ENOTTY;
+   else
+   ret = dev->s_tx_mask(dev, val);
+   break;
 
case LIRC_SET_SEND_CARRIER:
if (!dev->s_tx_carrier)
-   return -ENOTTY;
-
-   return dev->s_tx_carrier(dev, val);
+   ret = -ENOTTY;
+   else
+   ret = dev->s_tx_carrier(dev, val);
+   break;
 
case LIRC_SET_SEND_DUTY_CYCLE:
if (!dev->s_tx_duty_cycle)
-   return -ENOTTY;
-
-   if (val <= 0 || val >= 100)
-   return -EINVAL;
-
-   return dev->s_tx_duty_cycle(dev, val);
+   ret = -ENOTTY;
+   else if (val <= 0 || val >= 100)
+   ret = -EINVAL;
+   else
+   ret = dev->s_tx_duty_cycle(dev, val);
+   break;
 
/* RX settings */
case LIRC_SET_REC_CARRIER:
if (!dev->s_rx_carrier_range)
-   return -ENOTTY;
-
-   if (val <= 0)
-   return -EINVAL;
-
-   return dev->s_rx_carrier_range(dev,
-  dev->raw->lirc.carrier_low,
-  val);
+   ret = -ENOTTY;
+   else if (val <= 0)
+   ret = -EINVAL;
+   else
+   ret = dev->s_rx_carrier_range(dev,
+ 
dev->raw->lirc.carrier_low,
+ val);
+   break;
 
case LIRC_SET_REC_CARRIER_RANGE:
if (!dev->s_rx_carrier_range)
-   return -ENOTTY;
-
-   if (val <= 0)
-   return -EINVAL;
-
-   dev->raw->lirc.carrier_low = val;
-   return 0;
+   ret = -ENOTTY;
+   else if (val <= 0)
+   ret = -EINVAL;
+   else
+   dev->raw->lirc.carrier

[PATCH 16/19] lirc_dev: merge struct irctl into struct lirc_dev

[David Härdeman]

The use of two separate structs (lirc_dev aka lirc_driver and irctl) makes
it much harder to follow the proper lifetime of the various structs and
necessitates hacks such as keeping a copy of struct lirc_dev inside
struct irctl.

Merging the two structs means that lirc_dev can properly manage the lifetime
of the resulting struct and simplifies the code at the same time.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c|   15 +-
 drivers/media/rc/lirc_dev.c |  288 +--
 drivers/staging/media/lirc/lirc_zilog.c |   20 +-
 include/media/lirc_dev.h|   26 ++-
 4 files changed, 161 insertions(+), 188 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index f276c4f56fb5..ba20a4ce9cbc 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -35,7 +35,7 @@ static int ir_lirc_decode(struct rc_dev *dev, struct 
ir_raw_event ev)
struct lirc_codec *lirc = &dev->raw->lirc;
int sample;
 
-   if (!dev->raw->lirc.ldev || !dev->raw->lirc.ldev->rbuf)
+   if (!dev->raw->lirc.ldev || !dev->raw->lirc.ldev->buf)
return -EINVAL;
 
/* Packet start */
@@ -84,7 +84,7 @@ static int ir_lirc_decode(struct rc_dev *dev, struct 
ir_raw_event ev)
(u64)LIRC_VALUE_MASK);
 
gap_sample = LIRC_SPACE(lirc->gap_duration);
-   lirc_buffer_write(dev->raw->lirc.ldev->rbuf,
+   lirc_buffer_write(dev->raw->lirc.ldev->buf,
(unsigned char *) &gap_sample);
lirc->gap = false;
}
@@ -95,9 +95,9 @@ static int ir_lirc_decode(struct rc_dev *dev, struct 
ir_raw_event ev)
   TO_US(ev.duration), TO_STR(ev.pulse));
}
 
-   lirc_buffer_write(dev->raw->lirc.ldev->rbuf,
+   lirc_buffer_write(dev->raw->lirc.ldev->buf,
  (unsigned char *) &sample);
-   wake_up(&dev->raw->lirc.ldev->rbuf->wait_poll);
+   wake_up(&dev->raw->lirc.ldev->buf->wait_poll);
 
return 0;
 }
@@ -384,12 +384,12 @@ static int ir_lirc_register(struct rc_dev *dev)
 dev->driver_name);
ldev->features = features;
ldev->data = &dev->raw->lirc;
-   ldev->rbuf = NULL;
+   ldev->buf = NULL;
ldev->code_length = sizeof(struct ir_raw_event) * 8;
ldev->chunk_size = sizeof(int);
ldev->buffer_size = LIRCBUF_SIZE;
ldev->fops = &lirc_fops;
-   ldev->dev = &dev->dev;
+   ldev->dev.parent = &dev->dev;
ldev->rdev = dev;
ldev->owner = THIS_MODULE;
 
@@ -402,7 +402,7 @@ static int ir_lirc_register(struct rc_dev *dev)
return 0;
 
 out:
-   kfree(ldev);
+   lirc_free_device(ldev);
return rc;
 }
 
@@ -411,7 +411,6 @@ static int ir_lirc_unregister(struct rc_dev *dev)
struct lirc_codec *lirc = &dev->raw->lirc;
 
lirc_unregister_device(lirc->ldev);
-   kfree(lirc->ldev);
lirc->ldev = NULL;
 
return 0;
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 80944c2f7e91..4ad08d3d4e2f 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -34,19 +34,6 @@
 
 static dev_t lirc_base_dev;
 
-struct irctl {
-   struct lirc_dev d;
-   bool attached;
-   int open;
-
-   struct mutex mutex;
-   struct lirc_buffer *buf;
-   bool buf_internal;
-
-   struct device dev;
-   struct cdev cdev;
-};
-
 /* Used to keep track of allocated lirc devices */
 #define LIRC_MAX_DEVICES 256
 static DEFINE_IDA(lirc_ida);
@@ -54,69 +41,72 @@ static DEFINE_IDA(lirc_ida);
 /* Only used for sysfs but defined to void otherwise */
 static struct class *lirc_class;
 
-static void lirc_free_buffer(struct irctl *ir)
+static void lirc_release_device(struct device *ld)
 {
-   if (ir->buf_internal) {
-   lirc_buffer_free(ir->buf);
-   kfree(ir->buf);
-   ir->buf = NULL;
+   struct lirc_dev *d = container_of(ld, struct lirc_dev, dev);
+
+   if (d->buf_internal) {
+   lirc_buffer_free(d->buf);
+   kfree(d->buf);
+   d->buf = NULL;
}
+   kfree(d);
+   module_put(THIS_MODULE);
 }
 
-static void lirc_release(struct device *ld)
+static int lirc_allocate_buffer(struct lirc_dev *d)
 {
-   struct irctl *ir = container_of(ld, struct irctl, dev);
-
-   lirc_free_buffer(ir);
-   kfree(ir);
-}
+   int err;
 
-static int lirc_allocate_buffer(struct irctl *ir)
-{
-   int err = 0;
-   struct lirc_dev *d = &ir->d;
-
-   if (d->rbuf) {
-   ir->buf = d->rbuf;
-   ir->buf_internal = false;
-   } else {
-   ir->buf = kmalloc(sizeof(struct lirc_buffer), GFP_KERNEL);
-   if (!ir->buf) {
-

[PATCH 15/19] lirc_zilog: use a dynamically allocated lirc_dev

[David Härdeman]

lirc_zilog currently embeds a struct lirc_dev in its own struct IR, but
subsequent patches will make the lifetime of struct lirc_dev dynamic (i.e.
it will be free():d once lirc_dev is sure there are no users of the struct).

Therefore, change lirc_zilog to use a pointer to a dynamically allocated
struct lirc_dev.

Signed-off-by: David Härdeman 
---
 drivers/staging/media/lirc/lirc_zilog.c |   69 ++-
 1 file changed, 40 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index 51512ba7f5b8..a25ae574 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -102,8 +102,8 @@ struct IR {
struct kref ref;
struct list_head list;
 
-   /* FIXME spinlock access to l.features */
-   struct lirc_dev l;
+   /* FIXME spinlock access to l->features */
+   struct lirc_dev *l;
struct lirc_buffer rbuf;
 
struct mutex ir_lock;
@@ -187,7 +187,10 @@ static void release_ir_device(struct kref *ref)
 * ir->open_count ==  0 - happens on final close()
 * ir_lock, tx_ref_lock, rx_ref_lock, all released
 */
-   lirc_unregister_device(&ir->l);
+   if (ir->l) {
+   lirc_unregister_device(ir->l);
+   lirc_free_device(ir->l);
+   }
 
if (kfifo_initialized(&ir->rbuf.fifo))
lirc_buffer_free(&ir->rbuf);
@@ -244,7 +247,7 @@ static void release_ir_rx(struct kref *ref)
 * and releasing the ir reference can cause a sleep.  That work is
 * performed by put_ir_rx()
 */
-   ir->l.features &= ~LIRC_CAN_REC_LIRCCODE;
+   ir->l->features &= ~LIRC_CAN_REC_LIRCCODE;
/* Don't put_ir_device(rx->ir) here; lock can't be freed yet */
ir->rx = NULL;
/* Don't do the kfree(rx) here; we still need to kill the poll thread */
@@ -289,7 +292,7 @@ static void release_ir_tx(struct kref *ref)
struct IR_tx *tx = container_of(ref, struct IR_tx, ref);
struct IR *ir = tx->ir;
 
-   ir->l.features &= ~LIRC_CAN_SEND_PULSE;
+   ir->l->features &= ~LIRC_CAN_SEND_PULSE;
/* Don't put_ir_device(tx->ir) here, so our lock doesn't get freed */
ir->tx = NULL;
kfree(tx);
@@ -318,7 +321,7 @@ static int add_to_buf(struct IR *ir)
int ret;
int failures = 0;
unsigned char sendbuf[1] = { 0 };
-   struct lirc_buffer *rbuf = ir->l.rbuf;
+   struct lirc_buffer *rbuf = ir->l->rbuf;
struct IR_rx *rx;
struct IR_tx *tx;
 
@@ -464,7 +467,7 @@ static int add_to_buf(struct IR *ir)
 static int lirc_thread(void *arg)
 {
struct IR *ir = arg;
-   struct lirc_buffer *rbuf = ir->l.rbuf;
+   struct lirc_buffer *rbuf = ir->l->rbuf;
 
dev_dbg(ir->dev, "poll thread started\n");
 
@@ -885,7 +888,7 @@ static ssize_t read(struct file *filep, char __user 
*outbuf, size_t n,
 {
struct IR *ir = lirc_get_pdata(filep);
struct IR_rx *rx;
-   struct lirc_buffer *rbuf = ir->l.rbuf;
+   struct lirc_buffer *rbuf = ir->l->rbuf;
int ret = 0, written = 0, retries = 0;
unsigned int m;
DECLARE_WAITQUEUE(wait, current);
@@ -1203,7 +1206,7 @@ static unsigned int poll(struct file *filep, poll_table 
*wait)
 {
struct IR *ir = lirc_get_pdata(filep);
struct IR_rx *rx;
-   struct lirc_buffer *rbuf = ir->l.rbuf;
+   struct lirc_buffer *rbuf = ir->l->rbuf;
unsigned int ret;
 
dev_dbg(ir->dev, "%s called\n", __func__);
@@ -1239,7 +1242,7 @@ static long ioctl(struct file *filep, unsigned int cmd, 
unsigned long arg)
int result;
unsigned long mode, features;
 
-   features = ir->l.features;
+   features = ir->l->features;
 
switch (cmd) {
case LIRC_GET_LENGTH:
@@ -1349,13 +1352,6 @@ static const struct file_operations lirc_fops = {
.release= close
 };
 
-static struct lirc_dev lirc_template = {
-   .name   = "lirc_zilog",
-   .code_length= 13,
-   .fops   = &lirc_fops,
-   .owner  = THIS_MODULE,
-};
-
 static int ir_remove(struct i2c_client *client)
 {
if (strncmp("ir_tx_z8", client->name, 8) == 0) {
@@ -1446,22 +1442,35 @@ static int ir_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
spin_lock_init(&ir->rx_ref_lock);
 
/* set lirc_dev stuff */
-   memcpy(&ir->l, &lirc_template, sizeof(struct lirc_dev));
+   ir->l = lirc_allocate_device();
+   if (!ir->l) {
+   ret = -ENOMEM;
+   goto out_put_ir;
+   }
+
+   snprintf(ir->l->name, sizeof(ir->l->name), "lirc_zilog");
+   ir->l->code_length = 13;
+   ir->l->fops = &lirc_fops;
+   ir->l->owner = THIS_MODULE;
+
/*
 * FIXME this is a 

[PATCH 12/19] lirc_dev: introduce lirc_allocate_device and lirc_free_device

[David Härdeman]

Introduce two new functions so that the API for lirc_dev matches that
of the rc-core and input subsystems.

This means that lirc_dev structs are managed using the usual four functions:
lirc_allocate_device
lirc_free_device
lirc_register_device
lirc_unregister_device

The functions are pretty simplistic at this point, later patches will put
more flesh on the bones of both.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c |2 +-
 drivers/media/rc/lirc_dev.c  |   13 +
 include/media/lirc_dev.h |9 -
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index 2349630ed383..f276c4f56fb5 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -347,7 +347,7 @@ static int ir_lirc_register(struct rc_dev *dev)
int rc = -ENOMEM;
unsigned long features = 0;
 
-   ldev = kzalloc(sizeof(struct lirc_dev), GFP_KERNEL);
+   ldev = lirc_allocate_device();
if (!ldev)
return rc;
 
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 1c2f1a07bdaa..d107ed6b634b 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -101,6 +101,19 @@ static int lirc_allocate_buffer(struct irctl *ir)
return err;
 }
 
+struct lirc_dev *
+lirc_allocate_device(void)
+{
+   return kzalloc(sizeof(struct lirc_dev), GFP_KERNEL);
+}
+EXPORT_SYMBOL(lirc_allocate_device);
+
+void lirc_free_device(struct lirc_dev *d)
+{
+   kfree(d);
+}
+EXPORT_SYMBOL(lirc_free_device);
+
 int lirc_register_device(struct lirc_dev *d)
 {
struct irctl *ir;
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 567959e9524f..3f8edabfef88 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -150,11 +150,10 @@ struct lirc_dev {
struct irctl *irctl;
 };
 
-/* following functions can be called ONLY from user context
- *
- * returns negative value on error or zero
- * contents of the structure pointed by p is copied
- */
+extern struct lirc_dev *lirc_allocate_device(void);
+
+extern void lirc_free_device(struct lirc_dev *d);
+
 extern int lirc_register_device(struct lirc_dev *d);
 
 extern void lirc_unregister_device(struct lirc_dev *d);

[PATCH 13/19] lirc_dev: remove the BUFLEN define

[David Härdeman]

The define is only used in the lirc_zilog driver and once in lirc_dev.

In lirc_dev it rather serves to make the limits on d->code_length less clear,
so move the define to lirc_zilog.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |5 ++---
 drivers/staging/media/lirc/lirc_zilog.c |3 +++
 include/media/lirc_dev.h|2 --
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index d107ed6b634b..80944c2f7e91 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -145,9 +145,8 @@ int lirc_register_device(struct lirc_dev *d)
return -EINVAL;
}
 
-   if (d->code_length < 1 || d->code_length > (BUFLEN * 8)) {
-   dev_err(d->dev, "code length must be less than %d bits\n",
-   BUFLEN * 8);
+   if (d->code_length < 1 || d->code_length > 128) {
+   dev_err(d->dev, "invalid code_length!\n");
return -EBADRQC;
}
 
diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index a8aefd033ad9..f54b66de4a27 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -64,6 +64,9 @@
 /* Max transfer size done by I2C transfer functions */
 #define MAX_XFER_SIZE  64
 
+/* LIRC buffer size */
+#define BUFLEN16
+
 struct IR;
 
 struct IR_rx {
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 3f8edabfef88..21aac9494678 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -9,8 +9,6 @@
 #ifndef _LINUX_LIRC_DEV_H
 #define _LINUX_LIRC_DEV_H
 
-#define BUFLEN16
-
 #include 
 #include 
 #include 

[PATCH 10/19] lirc_dev: use an IDA instead of an array to keep track of registered devices

[David Härdeman]

Using the kernel-provided IDA simplifies the code and makes it possible
to remove the lirc_dev_lock mutex.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |   36 
 include/media/lirc_dev.h|1 -
 2 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 6bd21609a719..996cb5e778dc 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -46,10 +47,9 @@ struct irctl {
struct cdev cdev;
 };
 
-/* This mutex protects the irctls array */
-static DEFINE_MUTEX(lirc_dev_lock);
-
-static struct irctl *irctls[MAX_IRCTL_DEVICES];
+/* Used to keep track of allocated lirc devices */
+#define LIRC_MAX_DEVICES 256
+static DEFINE_IDA(lirc_ida);
 
 /* Only used for sysfs but defined to void otherwise */
 static struct class *lirc_class;
@@ -67,9 +67,6 @@ static void lirc_release(struct device *ld)
 {
struct irctl *ir = container_of(ld, struct irctl, dev);
 
-   mutex_lock(&lirc_dev_lock);
-   irctls[ir->d.minor] = NULL;
-   mutex_unlock(&lirc_dev_lock);
lirc_free_buffer(ir);
kfree(ir);
 }
@@ -107,7 +104,7 @@ static int lirc_allocate_buffer(struct irctl *ir)
 int lirc_register_driver(struct lirc_driver *d)
 {
struct irctl *ir;
-   unsigned minor;
+   int minor;
int err;
 
if (!d) {
@@ -169,27 +166,16 @@ int lirc_register_driver(struct lirc_driver *d)
d->rbuf = ir->buf;
}
 
-   mutex_lock(&lirc_dev_lock);
-
-   /* find first free slot for driver */
-   for (minor = 0; minor < MAX_IRCTL_DEVICES; minor++)
-   if (!irctls[minor])
-   break;
-
-   if (minor == MAX_IRCTL_DEVICES) {
-   dev_err(d->dev, "no free slots for drivers!\n");
-   mutex_unlock(&lirc_dev_lock);
+   minor = ida_simple_get(&lirc_ida, 0, LIRC_MAX_DEVICES, GFP_KERNEL);
+   if (minor < 0) {
lirc_free_buffer(ir);
kfree(ir);
-   return -ENOMEM;
+   return minor;
}
 
-   irctls[minor] = ir;
d->irctl = ir;
d->minor = minor;
 
-   mutex_unlock(&lirc_dev_lock);
-
device_initialize(&ir->dev);
ir->dev.devt = MKDEV(MAJOR(lirc_base_dev), ir->d.minor);
ir->dev.class = lirc_class;
@@ -203,6 +189,7 @@ int lirc_register_driver(struct lirc_driver *d)
 
err = cdev_device_add(&ir->cdev, &ir->dev);
if (err) {
+   ida_simple_remove(&lirc_ida, minor);
put_device(&ir->dev);
return err;
}
@@ -239,6 +226,7 @@ void lirc_unregister_driver(struct lirc_driver *d)
 
mutex_unlock(&ir->mutex);
 
+   ida_simple_remove(&lirc_ida, d->minor);
put_device(&ir->dev);
 }
 EXPORT_SYMBOL(lirc_unregister_driver);
@@ -509,7 +497,7 @@ static int __init lirc_dev_init(void)
return PTR_ERR(lirc_class);
}
 
-   retval = alloc_chrdev_region(&lirc_base_dev, 0, MAX_IRCTL_DEVICES,
+   retval = alloc_chrdev_region(&lirc_base_dev, 0, LIRC_MAX_DEVICES,
 "BaseRemoteCtl");
if (retval) {
class_destroy(lirc_class);
@@ -526,7 +514,7 @@ static int __init lirc_dev_init(void)
 static void __exit lirc_dev_exit(void)
 {
class_destroy(lirc_class);
-   unregister_chrdev_region(lirc_base_dev, MAX_IRCTL_DEVICES);
+   unregister_chrdev_region(lirc_base_dev, LIRC_MAX_DEVICES);
pr_info("module unloaded\n");
 }
 
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index a01fe5433bb7..2629c40e8a39 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -9,7 +9,6 @@
 #ifndef _LINUX_LIRC_DEV_H
 #define _LINUX_LIRC_DEV_H
 
-#define MAX_IRCTL_DEVICES 8
 #define BUFLEN16
 
 #include 

[PATCH 14/19] lirc_zilog: add a pointer to the parent device to struct IR

[David Härdeman]

lirc_zilog stashes a pointer to the parent device in struct lirc_dev and uses
it for logging. It makes more sense to let lirc_zilog keep track of that
pointer in its own struct (this is in preparation for subsequent patches
which will remodel struct lirc_dev).

Signed-off-by: David Härdeman 
---
 drivers/staging/media/lirc/lirc_zilog.c |   98 ---
 1 file changed, 50 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index f54b66de4a27..51512ba7f5b8 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -109,6 +109,7 @@ struct IR {
struct mutex ir_lock;
atomic_t open_count;
 
+   struct device *dev;
struct i2c_adapter *adapter;
 
spinlock_t rx_ref_lock; /* struct IR_rx kref get()/put() */
@@ -322,7 +323,7 @@ static int add_to_buf(struct IR *ir)
struct IR_tx *tx;
 
if (lirc_buffer_full(rbuf)) {
-   dev_dbg(ir->l.dev, "buffer overflow\n");
+   dev_dbg(ir->dev, "buffer overflow\n");
return -EOVERFLOW;
}
 
@@ -368,17 +369,17 @@ static int add_to_buf(struct IR *ir)
 */
ret = i2c_master_send(rx->c, sendbuf, 1);
if (ret != 1) {
-   dev_err(ir->l.dev, "i2c_master_send failed with %d\n",
+   dev_err(ir->dev, "i2c_master_send failed with %d\n",
ret);
if (failures >= 3) {
mutex_unlock(&ir->ir_lock);
-   dev_err(ir->l.dev,
+   dev_err(ir->dev,
"unable to read from the IR chip after 
3 resets, giving up\n");
break;
}
 
/* Looks like the chip crashed, reset it */
-   dev_err(ir->l.dev,
+   dev_err(ir->dev,
"polling the IR receiver chip failed, trying 
reset\n");
 
set_current_state(TASK_UNINTERRUPTIBLE);
@@ -405,14 +406,14 @@ static int add_to_buf(struct IR *ir)
ret = i2c_master_recv(rx->c, keybuf, sizeof(keybuf));
mutex_unlock(&ir->ir_lock);
if (ret != sizeof(keybuf)) {
-   dev_err(ir->l.dev,
+   dev_err(ir->dev,
"i2c_master_recv failed with %d -- keeping last 
read buffer\n",
ret);
} else {
rx->b[0] = keybuf[3];
rx->b[1] = keybuf[4];
rx->b[2] = keybuf[5];
-   dev_dbg(ir->l.dev,
+   dev_dbg(ir->dev,
"key (0x%02x/0x%02x)\n",
rx->b[0], rx->b[1]);
}
@@ -465,7 +466,7 @@ static int lirc_thread(void *arg)
struct IR *ir = arg;
struct lirc_buffer *rbuf = ir->l.rbuf;
 
-   dev_dbg(ir->l.dev, "poll thread started\n");
+   dev_dbg(ir->dev, "poll thread started\n");
 
while (!kthread_should_stop()) {
set_current_state(TASK_INTERRUPTIBLE);
@@ -493,7 +494,7 @@ static int lirc_thread(void *arg)
wake_up_interruptible(&rbuf->wait_poll);
}
 
-   dev_dbg(ir->l.dev, "poll thread ended\n");
+   dev_dbg(ir->dev, "poll thread ended\n");
return 0;
 }
 
@@ -646,10 +647,10 @@ static int send_data_block(struct IR_tx *tx, unsigned 
char *data_block)
buf[0] = (unsigned char)(i + 1);
for (j = 0; j < tosend; ++j)
buf[1 + j] = data_block[i + j];
-   dev_dbg(tx->ir->l.dev, "%*ph", 5, buf);
+   dev_dbg(tx->ir->dev, "%*ph", 5, buf);
ret = i2c_master_send(tx->c, buf, tosend + 1);
if (ret != tosend + 1) {
-   dev_err(tx->ir->l.dev,
+   dev_err(tx->ir->dev,
"i2c_master_send failed with %d\n", ret);
return ret < 0 ? ret : -EFAULT;
}
@@ -674,7 +675,7 @@ static int send_boot_data(struct IR_tx *tx)
buf[1] = 0x20;
ret = i2c_master_send(tx->c, buf, 2);
if (ret != 2) {
-   dev_err(tx->ir->l.dev, "i2c_master_send failed with %d\n", ret);
+   dev_err(tx->ir->dev, "i2c_master_send failed with %d\n", ret);
return ret < 0 ? ret : -EFAULT;
}
 
@@ -691,22 +692,22 @@ static int send_boot_data(struct IR_tx *tx)
}
 
if (ret != 1) {
-   dev_err(tx->ir->l.dev, "i2c_master_send failed with %d\n", ret);
+   dev_err(tx->ir->dev, "i2c_master_send failed with %d\n", ret);
return ret < 0 ? ret : -EFAULT

[PATCH 11/19] lirc_dev: rename struct lirc_driver to struct lirc_dev

[David Härdeman]

This is in preparation for the later patches which do away with
struct irctl entirely.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c|   50 ---
 drivers/media/rc/lirc_dev.c |   12 ---
 drivers/media/rc/rc-core-priv.h |2 +
 drivers/staging/media/lirc/lirc_zilog.c |   12 ---
 include/media/lirc_dev.h|   46 -
 5 files changed, 51 insertions(+), 71 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index 2c1221a61ea1..2349630ed383 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -35,7 +35,7 @@ static int ir_lirc_decode(struct rc_dev *dev, struct 
ir_raw_event ev)
struct lirc_codec *lirc = &dev->raw->lirc;
int sample;
 
-   if (!dev->raw->lirc.drv || !dev->raw->lirc.drv->rbuf)
+   if (!dev->raw->lirc.ldev || !dev->raw->lirc.ldev->rbuf)
return -EINVAL;
 
/* Packet start */
@@ -84,7 +84,7 @@ static int ir_lirc_decode(struct rc_dev *dev, struct 
ir_raw_event ev)
(u64)LIRC_VALUE_MASK);
 
gap_sample = LIRC_SPACE(lirc->gap_duration);
-   lirc_buffer_write(dev->raw->lirc.drv->rbuf,
+   lirc_buffer_write(dev->raw->lirc.ldev->rbuf,
(unsigned char *) &gap_sample);
lirc->gap = false;
}
@@ -95,9 +95,9 @@ static int ir_lirc_decode(struct rc_dev *dev, struct 
ir_raw_event ev)
   TO_US(ev.duration), TO_STR(ev.pulse));
}
 
-   lirc_buffer_write(dev->raw->lirc.drv->rbuf,
+   lirc_buffer_write(dev->raw->lirc.ldev->rbuf,
  (unsigned char *) &sample);
-   wake_up(&dev->raw->lirc.drv->rbuf->wait_poll);
+   wake_up(&dev->raw->lirc.ldev->rbuf->wait_poll);
 
return 0;
 }
@@ -343,12 +343,12 @@ static const struct file_operations lirc_fops = {
 
 static int ir_lirc_register(struct rc_dev *dev)
 {
-   struct lirc_driver *drv;
+   struct lirc_dev *ldev;
int rc = -ENOMEM;
unsigned long features = 0;
 
-   drv = kzalloc(sizeof(struct lirc_driver), GFP_KERNEL);
-   if (!drv)
+   ldev = kzalloc(sizeof(struct lirc_dev), GFP_KERNEL);
+   if (!ldev)
return rc;
 
if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
@@ -380,29 +380,29 @@ static int ir_lirc_register(struct rc_dev *dev)
if (dev->max_timeout)
features |= LIRC_CAN_SET_REC_TIMEOUT;
 
-   snprintf(drv->name, sizeof(drv->name), "ir-lirc-codec (%s)",
+   snprintf(ldev->name, sizeof(ldev->name), "ir-lirc-codec (%s)",
 dev->driver_name);
-   drv->features = features;
-   drv->data = &dev->raw->lirc;
-   drv->rbuf = NULL;
-   drv->code_length = sizeof(struct ir_raw_event) * 8;
-   drv->chunk_size = sizeof(int);
-   drv->buffer_size = LIRCBUF_SIZE;
-   drv->fops = &lirc_fops;
-   drv->dev = &dev->dev;
-   drv->rdev = dev;
-   drv->owner = THIS_MODULE;
-
-   rc = lirc_register_driver(drv);
+   ldev->features = features;
+   ldev->data = &dev->raw->lirc;
+   ldev->rbuf = NULL;
+   ldev->code_length = sizeof(struct ir_raw_event) * 8;
+   ldev->chunk_size = sizeof(int);
+   ldev->buffer_size = LIRCBUF_SIZE;
+   ldev->fops = &lirc_fops;
+   ldev->dev = &dev->dev;
+   ldev->rdev = dev;
+   ldev->owner = THIS_MODULE;
+
+   rc = lirc_register_device(ldev);
if (rc < 0)
goto out;
 
-   dev->raw->lirc.drv = drv;
+   dev->raw->lirc.ldev = ldev;
dev->raw->lirc.dev = dev;
return 0;
 
 out:
-   kfree(drv);
+   kfree(ldev);
return rc;
 }
 
@@ -410,9 +410,9 @@ static int ir_lirc_unregister(struct rc_dev *dev)
 {
struct lirc_codec *lirc = &dev->raw->lirc;
 
-   lirc_unregister_driver(lirc->drv);
-   kfree(lirc->drv);
-   lirc->drv = NULL;
+   lirc_unregister_device(lirc->ldev);
+   kfree(lirc->ldev);
+   lirc->ldev = NULL;
 
return 0;
 }
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 996cb5e778dc..1c2f1a07bdaa 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -35,7 +35,7 @@
 static dev_t lirc_base_dev;
 
 struct irctl {
-   struct lirc_driver d;
+   struct lirc_dev d;
bool attached;
int open;
 
@@ -74,7 +74,7 @@ static void lirc_release(struct device *ld)
 static int lirc_allocate_buffer(struct irctl *ir)
 {
int err = 0;
-   struct lirc_driver *d = &ir->d;
+   struct lirc_dev *d = &ir->d;
 
if (d->rbuf) {
ir->buf = d->rbuf;
@@ -101,7 +101,7 @@ static int lirc_allocate_buffer(struct irctl *ir)
return err;
 }
 
-int lirc_register_driver(struct lirc_

[PATCH 09/19] lirc_dev: sanitize locking

[David Härdeman]

Use the irctl mutex for all device operations and only use lirc_dev_lock to
protect the irctls array. Also, make sure that the device is alive early in
each fops function before doing anything else.

Since this patch touches nearly every line where the irctl mutex is
taken/released, it also renames the mutex at the same time (the name
irctl_lock will be misleading once struct irctl goes away in later
patches).

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |  164 ---
 1 file changed, 91 insertions(+), 73 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index aece6b619796..6bd21609a719 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -38,7 +38,7 @@ struct irctl {
bool attached;
int open;
 
-   struct mutex irctl_lock;
+   struct mutex mutex;
struct lirc_buffer *buf;
bool buf_internal;
 
@@ -46,6 +46,7 @@ struct irctl {
struct cdev cdev;
 };
 
+/* This mutex protects the irctls array */
 static DEFINE_MUTEX(lirc_dev_lock);
 
 static struct irctl *irctls[MAX_IRCTL_DEVICES];
@@ -53,18 +54,23 @@ static struct irctl *irctls[MAX_IRCTL_DEVICES];
 /* Only used for sysfs but defined to void otherwise */
 static struct class *lirc_class;
 
-static void lirc_release(struct device *ld)
+static void lirc_free_buffer(struct irctl *ir)
 {
-   struct irctl *ir = container_of(ld, struct irctl, dev);
-
if (ir->buf_internal) {
lirc_buffer_free(ir->buf);
kfree(ir->buf);
+   ir->buf = NULL;
}
+}
+
+static void lirc_release(struct device *ld)
+{
+   struct irctl *ir = container_of(ld, struct irctl, dev);
 
mutex_lock(&lirc_dev_lock);
irctls[ir->d.minor] = NULL;
mutex_unlock(&lirc_dev_lock);
+   lirc_free_buffer(ir);
kfree(ir);
 }
 
@@ -141,6 +147,28 @@ int lirc_register_driver(struct lirc_driver *d)
return -EBADRQC;
}
 
+   /* some safety check 8-) */
+   d->name[sizeof(d->name)-1] = '\0';
+
+   if (d->features == 0)
+   d->features = LIRC_CAN_REC_LIRCCODE;
+
+   ir = kzalloc(sizeof(struct irctl), GFP_KERNEL);
+   if (!ir)
+   return -ENOMEM;
+
+   mutex_init(&ir->mutex);
+   ir->d = *d;
+
+   if (LIRC_CAN_REC(d->features)) {
+   err = lirc_allocate_buffer(ir);
+   if (err) {
+   kfree(ir);
+   return err;
+   }
+   d->rbuf = ir->buf;
+   }
+
mutex_lock(&lirc_dev_lock);
 
/* find first free slot for driver */
@@ -150,37 +178,17 @@ int lirc_register_driver(struct lirc_driver *d)
 
if (minor == MAX_IRCTL_DEVICES) {
dev_err(d->dev, "no free slots for drivers!\n");
-   err = -ENOMEM;
-   goto out_lock;
-   }
-
-   ir = kzalloc(sizeof(struct irctl), GFP_KERNEL);
-   if (!ir) {
-   err = -ENOMEM;
-   goto out_lock;
+   mutex_unlock(&lirc_dev_lock);
+   lirc_free_buffer(ir);
+   kfree(ir);
+   return -ENOMEM;
}
 
-   mutex_init(&ir->irctl_lock);
irctls[minor] = ir;
d->irctl = ir;
d->minor = minor;
 
-   /* some safety check 8-) */
-   d->name[sizeof(d->name)-1] = '\0';
-
-   if (d->features == 0)
-   d->features = LIRC_CAN_REC_LIRCCODE;
-
-   ir->d = *d;
-
-   if (LIRC_CAN_REC(d->features)) {
-   err = lirc_allocate_buffer(irctls[minor]);
-   if (err) {
-   kfree(ir);
-   goto out_lock;
-   }
-   d->rbuf = ir->buf;
-   }
+   mutex_unlock(&lirc_dev_lock);
 
device_initialize(&ir->dev);
ir->dev.devt = MKDEV(MAJOR(lirc_base_dev), ir->d.minor);
@@ -194,22 +202,15 @@ int lirc_register_driver(struct lirc_driver *d)
ir->attached = true;
 
err = cdev_device_add(&ir->cdev, &ir->dev);
-   if (err)
-   goto out_dev;
-
-   mutex_unlock(&lirc_dev_lock);
+   if (err) {
+   put_device(&ir->dev);
+   return err;
+   }
 
dev_info(ir->d.dev, "lirc_dev: driver %s registered at minor = %d\n",
 ir->d.name, ir->d.minor);
 
return 0;
-
-out_dev:
-   put_device(&ir->dev);
-out_lock:
-   mutex_unlock(&lirc_dev_lock);
-
-   return err;
 }
 EXPORT_SYMBOL(lirc_register_driver);
 
@@ -222,11 +223,13 @@ void lirc_unregister_driver(struct lirc_driver *d)
 
ir = d->irctl;
 
-   mutex_lock(&lirc_dev_lock);
-
dev_dbg(ir->d.dev, "lirc_dev: driver %s unregistered from minor = %d\n",
d->name, d->minor);
 
+   cdev_device_del(&ir->cdev, &ir->dev);
+
+   mutex_lock(&ir->mutex);
+
ir->attached = false;
if (ir->open) {
dev_dbg(ir->

[PATCH 08/19] lirc_dev: change irctl->attached to be a boolean

[David Härdeman]

The "attached" member of struct irctl is a boolean value, so let the code
reflect that.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 92048d945ba7..aece6b619796 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -35,7 +35,7 @@ static dev_t lirc_base_dev;
 
 struct irctl {
struct lirc_driver d;
-   int attached;
+   bool attached;
int open;
 
struct mutex irctl_lock;
@@ -191,7 +191,7 @@ int lirc_register_driver(struct lirc_driver *d)
 
cdev_init(&ir->cdev, d->fops);
ir->cdev.owner = ir->d.owner;
-   ir->attached = 1;
+   ir->attached = true;
 
err = cdev_device_add(&ir->cdev, &ir->dev);
if (err)
@@ -227,7 +227,7 @@ void lirc_unregister_driver(struct lirc_driver *d)
dev_dbg(ir->d.dev, "lirc_dev: driver %s unregistered from minor = %d\n",
d->name, d->minor);
 
-   ir->attached = 0;
+   ir->attached = false;
if (ir->open) {
dev_dbg(ir->d.dev, LOGHEAD "releasing opened driver\n",
d->name, d->minor);

[PATCH 05/19] lirc_dev: make better use of file->private_data

[David Härdeman]

By making better use of file->private_data in lirc_dev we can avoid
digging around in the irctls[] array, thereby simplifying the code.

External drivers need to use lirc_get_pdata() instead of mucking around
in file->private_data.

The newly introduced lirc_init_pdata() function isn't very elegant, but
it's a stopgap measure which can be removed once lirc_zilog is converted
to rc-core.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |   70 +--
 drivers/staging/media/lirc/lirc_zilog.c |   53 ---
 include/media/lirc_dev.h|3 +
 3 files changed, 33 insertions(+), 93 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 61ed90a975ad..2de840dd829d 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -243,36 +243,18 @@ EXPORT_SYMBOL(lirc_unregister_driver);
 
 int lirc_dev_fop_open(struct inode *inode, struct file *file)
 {
-   struct irctl *ir;
+   struct irctl *ir = container_of(inode->i_cdev, struct irctl, cdev);
int retval;
 
-   if (iminor(inode) >= MAX_IRCTL_DEVICES) {
-   pr_err("open result for %d is -ENODEV\n", iminor(inode));
-   return -ENODEV;
-   }
-
-   if (mutex_lock_interruptible(&lirc_dev_lock))
-   return -ERESTARTSYS;
-
-   ir = irctls[iminor(inode)];
-   mutex_unlock(&lirc_dev_lock);
-
-   if (!ir) {
-   retval = -ENODEV;
-   goto error;
-   }
-
dev_dbg(ir->d.dev, LOGHEAD "open called\n", ir->d.name, ir->d.minor);
 
-   if (ir->open) {
-   retval = -EBUSY;
-   goto error;
-   }
+   if (ir->open)
+   return -EBUSY;
 
if (ir->d.rdev) {
retval = rc_open(ir->d.rdev);
if (retval)
-   goto error;
+   return retval;
}
 
if (ir->buf)
@@ -280,25 +262,18 @@ int lirc_dev_fop_open(struct inode *inode, struct file 
*file)
 
ir->open++;
 
+   lirc_init_pdata(inode, file);
nonseekable_open(inode, file);
 
return 0;
-
-error:
-   return retval;
 }
 EXPORT_SYMBOL(lirc_dev_fop_open);
 
 int lirc_dev_fop_close(struct inode *inode, struct file *file)
 {
-   struct irctl *ir = irctls[iminor(inode)];
+   struct irctl *ir = file->private_data;
int ret;
 
-   if (!ir) {
-   pr_err("called with invalid irctl\n");
-   return -EINVAL;
-   }
-
ret = mutex_lock_killable(&lirc_dev_lock);
WARN_ON(ret);
 
@@ -314,14 +289,9 @@ EXPORT_SYMBOL(lirc_dev_fop_close);
 
 unsigned int lirc_dev_fop_poll(struct file *file, poll_table *wait)
 {
-   struct irctl *ir = irctls[iminor(file_inode(file))];
+   struct irctl *ir = file->private_data;
unsigned int ret;
 
-   if (!ir) {
-   pr_err("called with invalid irctl\n");
-   return POLLERR;
-   }
-
if (!ir->attached)
return POLLHUP | POLLERR;
 
@@ -344,14 +314,9 @@ EXPORT_SYMBOL(lirc_dev_fop_poll);
 
 long lirc_dev_fop_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
+   struct irctl *ir = file->private_data;
__u32 mode;
int result = 0;
-   struct irctl *ir = irctls[iminor(file_inode(file))];
-
-   if (!ir) {
-   pr_err("no irctl found!\n");
-   return -ENODEV;
-   }
 
dev_dbg(ir->d.dev, LOGHEAD "ioctl called (0x%x)\n",
ir->d.name, ir->d.minor, cmd);
@@ -410,16 +375,11 @@ ssize_t lirc_dev_fop_read(struct file *file,
  size_t length,
  loff_t *ppos)
 {
-   struct irctl *ir = irctls[iminor(file_inode(file))];
+   struct irctl *ir = file->private_data;
unsigned char *buf;
int ret = 0, written = 0;
DECLARE_WAITQUEUE(wait, current);
 
-   if (!ir) {
-   pr_err("called with invalid irctl\n");
-   return -ENODEV;
-   }
-
if (!LIRC_CAN_REC(ir->d.features))
return -EINVAL;
 
@@ -510,9 +470,19 @@ ssize_t lirc_dev_fop_read(struct file *file,
 }
 EXPORT_SYMBOL(lirc_dev_fop_read);
 
+void lirc_init_pdata(struct inode *inode, struct file *file)
+{
+   struct irctl *ir = container_of(inode->i_cdev, struct irctl, cdev);
+
+   file->private_data = ir;
+}
+EXPORT_SYMBOL(lirc_init_pdata);
+
 void *lirc_get_pdata(struct file *file)
 {
-   return irctls[iminor(file_inode(file))]->d.data;
+   struct irctl *ir = file->private_data;
+
+   return ir->d.data;
 }
 EXPORT_SYMBOL(lirc_get_pdata);
 
diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index 10594aea2a5c..6d4c5a957ab4 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -879,7 +879,7 @@ static int fw_load(struct IR_tx *tx)
 static ssize_t rea

[PATCH 04/19] lirc_dev: use cdev_device_add() helper function

[David Härdeman]

Replace calls to cdev_add() and device_add() with the cdev_device_add()
helper function.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |   17 -
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 591dee9f6ba2..61ed90a975ad 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -191,17 +191,11 @@ int lirc_register_driver(struct lirc_driver *d)
 
cdev_init(&ir->cdev, d->fops);
ir->cdev.owner = ir->d.owner;
-   ir->cdev.kobj.parent = &ir->dev.kobj;
-
-   err = cdev_add(&ir->cdev, ir->dev.devt, 1);
-   if (err)
-   goto out_free_dev;
-
ir->attached = 1;
 
-   err = device_add(&ir->dev);
+   err = cdev_device_add(&ir->cdev, &ir->dev);
if (err)
-   goto out_cdev;
+   goto out_dev;
 
mutex_unlock(&lirc_dev_lock);
 
@@ -210,9 +204,7 @@ int lirc_register_driver(struct lirc_driver *d)
 
return 0;
 
-out_cdev:
-   cdev_del(&ir->cdev);
-out_free_dev:
+out_dev:
put_device(&ir->dev);
 out_lock:
mutex_unlock(&lirc_dev_lock);
@@ -244,8 +236,7 @@ void lirc_unregister_driver(struct lirc_driver *d)
 
mutex_unlock(&lirc_dev_lock);
 
-   device_del(&ir->dev);
-   cdev_del(&ir->cdev);
+   cdev_device_del(&ir->cdev, &ir->dev);
put_device(&ir->dev);
 }
 EXPORT_SYMBOL(lirc_unregister_driver);

[PATCH 06/19] lirc_dev: make chunk_size and buffer_size mandatory

[David Härdeman]

Make setting chunk_size and buffer_size mandatory for drivers which
expect lirc_dev to allocate the lirc_buffer (i.e. ir-lirc-codec) and
don't set them in lirc-zilog (which creates its own buffer).

Also remove an unnecessary copy of chunk_size in struct irctl (the
same information is already available from struct lirc_buffer).

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |   26 +-
 drivers/staging/media/lirc/lirc_zilog.c |5 +
 include/media/lirc_dev.h|9 +
 3 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 2de840dd829d..1773a2934484 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -41,7 +41,6 @@ struct irctl {
struct mutex irctl_lock;
struct lirc_buffer *buf;
bool buf_internal;
-   unsigned int chunk_size;
 
struct device dev;
struct cdev cdev;
@@ -72,16 +71,8 @@ static void lirc_release(struct device *ld)
 static int lirc_allocate_buffer(struct irctl *ir)
 {
int err = 0;
-   int bytes_in_key;
-   unsigned int chunk_size;
-   unsigned int buffer_size;
struct lirc_driver *d = &ir->d;
 
-   bytes_in_key = BITS_TO_LONGS(d->code_length) +
-   (d->code_length % 8 ? 1 : 0);
-   buffer_size = d->buffer_size ? d->buffer_size : BUFLEN / bytes_in_key;
-   chunk_size  = d->chunk_size  ? d->chunk_size  : bytes_in_key;
-
if (d->rbuf) {
ir->buf = d->rbuf;
ir->buf_internal = false;
@@ -92,7 +83,7 @@ static int lirc_allocate_buffer(struct irctl *ir)
goto out;
}
 
-   err = lirc_buffer_init(ir->buf, chunk_size, buffer_size);
+   err = lirc_buffer_init(ir->buf, d->chunk_size, d->buffer_size);
if (err) {
kfree(ir->buf);
ir->buf = NULL;
@@ -102,7 +93,6 @@ static int lirc_allocate_buffer(struct irctl *ir)
ir->buf_internal = true;
d->rbuf = ir->buf;
}
-   ir->chunk_size = ir->buf->chunk_size;
 
 out:
return err;
@@ -129,6 +119,16 @@ int lirc_register_driver(struct lirc_driver *d)
return -EINVAL;
}
 
+   if (!d->rbuf && d->chunk_size < 1) {
+   pr_err("chunk_size must be set!\n");
+   return -EINVAL;
+   }
+
+   if (!d->rbuf && d->buffer_size < 1) {
+   pr_err("buffer_size must be set!\n");
+   return -EINVAL;
+   }
+
if (d->code_length < 1 || d->code_length > (BUFLEN * 8)) {
dev_err(d->dev, "code length must be less than %d bits\n",
BUFLEN * 8);
@@ -385,7 +385,7 @@ ssize_t lirc_dev_fop_read(struct file *file,
 
dev_dbg(ir->d.dev, LOGHEAD "read called\n", ir->d.name, ir->d.minor);
 
-   buf = kzalloc(ir->chunk_size, GFP_KERNEL);
+   buf = kzalloc(ir->buf->chunk_size, GFP_KERNEL);
if (!buf)
return -ENOMEM;
 
@@ -398,7 +398,7 @@ ssize_t lirc_dev_fop_read(struct file *file,
goto out_locked;
}
 
-   if (length % ir->chunk_size) {
+   if (length % ir->buf->chunk_size) {
ret = -EINVAL;
goto out_locked;
}
diff --git a/drivers/staging/media/lirc/lirc_zilog.c 
b/drivers/staging/media/lirc/lirc_zilog.c
index 6d4c5a957ab4..c6a2fe2ad210 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -1348,8 +1348,6 @@ static const struct file_operations lirc_fops = {
 static struct lirc_driver lirc_template = {
.name   = "lirc_zilog",
.code_length= 13,
-   .buffer_size= BUFLEN / 2,
-   .chunk_size = 2,
.fops   = &lirc_fops,
.owner  = THIS_MODULE,
 };
@@ -1456,8 +1454,7 @@ static int ir_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
ir->l.dev  = &adap->dev;
/* This will be returned by lirc_get_pdata() */
ir->l.data = ir;
-   ret = lirc_buffer_init(ir->l.rbuf,
-  ir->l.chunk_size, ir->l.buffer_size);
+   ret = lirc_buffer_init(ir->l.rbuf, 2, BUFLEN / 2);
if (ret)
goto out_put_ir;
}
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 20c5c5d6f101..a01fe5433bb7 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -121,13 +121,14 @@ static inline unsigned int lirc_buffer_write(struct 
lirc_buffer *buf,
  *
  * @code_length:   length of the remote control key code expressed in bits.
  *
- * @buffer_size:   Number of FIFO buffers with @chunk_size size. If zero,
- * creates a buff

[PATCH 07/19] lirc_dev: remove kmalloc in lirc_dev_fop_read()

[David Härdeman]

lirc_zilog uses a chunk_size of 2 and ir-lirc-codec uses sizeof(int).

Therefore, using stack memory should be perfectly fine.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 1773a2934484..92048d945ba7 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -376,7 +376,7 @@ ssize_t lirc_dev_fop_read(struct file *file,
  loff_t *ppos)
 {
struct irctl *ir = file->private_data;
-   unsigned char *buf;
+   unsigned char buf[ir->buf->chunk_size];
int ret = 0, written = 0;
DECLARE_WAITQUEUE(wait, current);
 
@@ -385,10 +385,6 @@ ssize_t lirc_dev_fop_read(struct file *file,
 
dev_dbg(ir->d.dev, LOGHEAD "read called\n", ir->d.name, ir->d.minor);
 
-   buf = kzalloc(ir->buf->chunk_size, GFP_KERNEL);
-   if (!buf)
-   return -ENOMEM;
-
if (mutex_lock_interruptible(&ir->irctl_lock)) {
ret = -ERESTARTSYS;
goto out_unlocked;
@@ -464,8 +460,6 @@ ssize_t lirc_dev_fop_read(struct file *file,
mutex_unlock(&ir->irctl_lock);
 
 out_unlocked:
-   kfree(buf);
-
return ret ? ret : written;
 }
 EXPORT_SYMBOL(lirc_dev_fop_read);

[PATCH 01/19] lirc_dev: clarify error handling

[David Härdeman]

If an error is generated, it is more logical to error out ASAP.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index db1e7b70c998..9c1d55e41e34 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -282,7 +282,7 @@ EXPORT_SYMBOL(lirc_unregister_driver);
 int lirc_dev_fop_open(struct inode *inode, struct file *file)
 {
struct irctl *ir;
-   int retval = 0;
+   int retval;
 
if (iminor(inode) >= MAX_IRCTL_DEVICES) {
pr_err("open result for %d is -ENODEV\n", iminor(inode));
@@ -323,9 +323,11 @@ int lirc_dev_fop_open(struct inode *inode, struct file 
*file)
 
ir->open++;
 
-error:
nonseekable_open(inode, file);
 
+   return 0;
+
+error:
return retval;
 }
 EXPORT_SYMBOL(lirc_dev_fop_open);

[PATCH 03/19] lirc_dev: remove min_timeout and max_timeout

[David Härdeman]

There are no users of this functionality (ir-lirc-codec.c has its own
implementation and lirc_zilog.c doesn't use it) so remove it.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/lirc_dev.c |   18 --
 include/media/lirc_dev.h|8 
 2 files changed, 26 deletions(-)

diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index c9afaf5e64a9..591dee9f6ba2 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -404,24 +404,6 @@ long lirc_dev_fop_ioctl(struct file *file, unsigned int 
cmd, unsigned long arg)
case LIRC_GET_LENGTH:
result = put_user(ir->d.code_length, (__u32 __user *)arg);
break;
-   case LIRC_GET_MIN_TIMEOUT:
-   if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) ||
-   ir->d.min_timeout == 0) {
-   result = -ENOTTY;
-   break;
-   }
-
-   result = put_user(ir->d.min_timeout, (__u32 __user *)arg);
-   break;
-   case LIRC_GET_MAX_TIMEOUT:
-   if (!(ir->d.features & LIRC_CAN_SET_REC_TIMEOUT) ||
-   ir->d.max_timeout == 0) {
-   result = -ENOTTY;
-   break;
-   }
-
-   result = put_user(ir->d.max_timeout, (__u32 __user *)arg);
-   break;
default:
result = -ENOTTY;
}
diff --git a/include/media/lirc_dev.h b/include/media/lirc_dev.h
index 1419d64e2e59..53eef86e07a0 100644
--- a/include/media/lirc_dev.h
+++ b/include/media/lirc_dev.h
@@ -132,12 +132,6 @@ static inline unsigned int lirc_buffer_write(struct 
lirc_buffer *buf,
  * @data:  it may point to any driver data and this pointer will
  * be passed to all callback functions.
  *
- * @min_timeout:   Minimum timeout for record. Valid only if
- * LIRC_CAN_SET_REC_TIMEOUT is defined.
- *
- * @max_timeout:   Maximum timeout for record. Valid only if
- * LIRC_CAN_SET_REC_TIMEOUT is defined.
- *
  * @rbuf:  if not NULL, it will be used as a read buffer, you will
  * have to write to the buffer by other means, like irq's
  * (see also lirc_serial.c).
@@ -168,8 +162,6 @@ struct lirc_driver {
unsigned int chunk_size;
 
void *data;
-   int min_timeout;
-   int max_timeout;
struct lirc_buffer *rbuf;
struct rc_dev *rdev;
const struct file_operations *fops;

[PATCH 02/19] lirc_dev: remove support for manually specifying minor number

[David Härdeman]

All users of lirc_register_driver() uses dynamic minor allocation, therefore
we can remove the ability to explicitly request a given number.

This changes the function prototype of lirc_unregister_driver() to also
take a struct lirc_driver pointer as the sole argument.

Signed-off-by: David Härdeman 
---
 drivers/media/rc/ir-lirc-codec.c|9 +---
 drivers/media/rc/lirc_dev.c |   68 ---
 drivers/staging/media/lirc/lirc_zilog.c |   14 ++
 include/media/lirc_dev.h|   18 
 4 files changed, 33 insertions(+), 76 deletions(-)

diff --git a/drivers/media/rc/ir-lirc-codec.c b/drivers/media/rc/ir-lirc-codec.c
index a30af91710fe..2c1221a61ea1 100644
--- a/drivers/media/rc/ir-lirc-codec.c
+++ b/drivers/media/rc/ir-lirc-codec.c
@@ -382,7 +382,6 @@ static int ir_lirc_register(struct rc_dev *dev)
 
snprintf(drv->name, sizeof(drv->name), "ir-lirc-codec (%s)",
 dev->driver_name);
-   drv->minor = -1;
drv->features = features;
drv->data = &dev->raw->lirc;
drv->rbuf = NULL;
@@ -394,11 +393,9 @@ static int ir_lirc_register(struct rc_dev *dev)
drv->rdev = dev;
drv->owner = THIS_MODULE;
 
-   drv->minor = lirc_register_driver(drv);
-   if (drv->minor < 0) {
-   rc = -ENODEV;
+   rc = lirc_register_driver(drv);
+   if (rc < 0)
goto out;
-   }
 
dev->raw->lirc.drv = drv;
dev->raw->lirc.dev = dev;
@@ -413,7 +410,7 @@ static int ir_lirc_unregister(struct rc_dev *dev)
 {
struct lirc_codec *lirc = &dev->raw->lirc;
 
-   lirc_unregister_driver(lirc->drv->minor);
+   lirc_unregister_driver(lirc->drv);
kfree(lirc->drv);
lirc->drv = NULL;
 
diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
index 9c1d55e41e34..c9afaf5e64a9 100644
--- a/drivers/media/rc/lirc_dev.c
+++ b/drivers/media/rc/lirc_dev.c
@@ -29,7 +29,6 @@
 #include 
 #include 
 
-#define NOPLUG -1
 #define LOGHEAD"lirc_dev (%s[%d]): "
 
 static dev_t lirc_base_dev;
@@ -112,7 +111,7 @@ static int lirc_allocate_buffer(struct irctl *ir)
 int lirc_register_driver(struct lirc_driver *d)
 {
struct irctl *ir;
-   int minor;
+   unsigned minor;
int err;
 
if (!d) {
@@ -130,12 +129,6 @@ int lirc_register_driver(struct lirc_driver *d)
return -EINVAL;
}
 
-   if (d->minor >= MAX_IRCTL_DEVICES) {
-   dev_err(d->dev, "minor must be between 0 and %d!\n",
-   MAX_IRCTL_DEVICES - 1);
-   return -EBADRQC;
-   }
-
if (d->code_length < 1 || d->code_length > (BUFLEN * 8)) {
dev_err(d->dev, "code length must be less than %d bits\n",
BUFLEN * 8);
@@ -150,21 +143,14 @@ int lirc_register_driver(struct lirc_driver *d)
 
mutex_lock(&lirc_dev_lock);
 
-   minor = d->minor;
+   /* find first free slot for driver */
+   for (minor = 0; minor < MAX_IRCTL_DEVICES; minor++)
+   if (!irctls[minor])
+   break;
 
-   if (minor < 0) {
-   /* find first free slot for driver */
-   for (minor = 0; minor < MAX_IRCTL_DEVICES; minor++)
-   if (!irctls[minor])
-   break;
-   if (minor == MAX_IRCTL_DEVICES) {
-   dev_err(d->dev, "no free slots for drivers!\n");
-   err = -ENOMEM;
-   goto out_lock;
-   }
-   } else if (irctls[minor]) {
-   dev_err(d->dev, "minor (%d) just registered!\n", minor);
-   err = -EBUSY;
+   if (minor == MAX_IRCTL_DEVICES) {
+   dev_err(d->dev, "no free slots for drivers!\n");
+   err = -ENOMEM;
goto out_lock;
}
 
@@ -176,6 +162,7 @@ int lirc_register_driver(struct lirc_driver *d)
 
mutex_init(&ir->irctl_lock);
irctls[minor] = ir;
+   d->irctl = ir;
d->minor = minor;
 
/* some safety check 8-) */
@@ -221,7 +208,7 @@ int lirc_register_driver(struct lirc_driver *d)
dev_info(ir->d.dev, "lirc_dev: driver %s registered at minor = %d\n",
 ir->d.name, ir->d.minor);
 
-   return minor;
+   return 0;
 
 out_cdev:
cdev_del(&ir->cdev);
@@ -234,38 +221,24 @@ int lirc_register_driver(struct lirc_driver *d)
 }
 EXPORT_SYMBOL(lirc_register_driver);
 
-int lirc_unregister_driver(int minor)
+void lirc_unregister_driver(struct lirc_driver *d)
 {
struct irctl *ir;
 
-   if (minor < 0 || minor >= MAX_IRCTL_DEVICES) {
-   pr_err("minor (%d) must be between 0 and %d!\n",
-   minor, MAX_IRCTL_DEVICES - 1);
-   return -EBADRQC;
-   }
+   if (!d || !d->irctl)
+   return;
 

[PATCH 00/19] lirc_dev modernisation

[David Härdeman]

This patch series reworks lirc_dev to use a single struct lirc_dev to
keep track of registered lirc devices rather than the current situation
where a combination of a struct lirc_driver and a struct irctl are used.
The fact that two structs are currently used per device makes the current
code harder to read and to analyse (e.g. wrt locking correctness).

The idea started out with this patch:
http://www.mail-archive.com/linux-media@vger.kernel.org/msg112159.html

Which was rejected due to the struct copying. In fixing that issue and
at the same time trying to split up the patch in smaller pieces, I ended
up with quite a bit larger patch series than first expected.

The end result is that struct lirc_dev (which is maintained by lirc_dev)
has proper lifecycle management and that we can avoid the current struct
copying that is performed between struct lirc_driver and struct irctl.

The locking in lirc_dev is also much improved by only having one mutex per
struct lirc_dev which is used to synchronize all operations.

The modifications to lirc_dev and ir-lirc-codec have been tested using
rc-loopback and mceusb. The changes to lirc_zilog are only compile tested.

---

David Härdeman (19):
  lirc_dev: clarify error handling
  lirc_dev: remove support for manually specifying minor number
  lirc_dev: remove min_timeout and max_timeout
  lirc_dev: use cdev_device_add() helper function
  lirc_dev: make better use of file->private_data
  lirc_dev: make chunk_size and buffer_size mandatory
  lirc_dev: remove kmalloc in lirc_dev_fop_read()
  lirc_dev: change irctl->attached to be a boolean
  lirc_dev: sanitize locking
  lirc_dev: use an IDA instead of an array to keep track of registered 
devices
  lirc_dev: rename struct lirc_driver to struct lirc_dev
  lirc_dev: introduce lirc_allocate_device and lirc_free_device
  lirc_dev: remove the BUFLEN define
  lirc_zilog: add a pointer to the parent device to struct IR
  lirc_zilog: use a dynamically allocated lirc_dev
  lirc_dev: merge struct irctl into struct lirc_dev
  ir-lirc-codec: merge lirc_dev_fop_ioctl into ir_lirc_ioctl
  ir-lirc-codec: move the remaining fops over from lirc_dev
  lirc_dev: consistent device registration printk


 drivers/media/rc/ir-lirc-codec.c|  404 -
 drivers/media/rc/lirc_dev.c |  587 ++-
 drivers/media/rc/rc-core-priv.h |2 
 drivers/staging/media/lirc/lirc_zilog.c |  234 +---
 include/media/lirc_dev.h|  111 ++
 5 files changed, 570 insertions(+), 768 deletions(-)

--
David Härdeman

Re: [PATCH v2 08/15] [media] cxd2880: Add top level of the driver

[Mauro Carvalho Chehab]

Em Fri, 23 Jun 2017 10:02:39 -0300
Mauro Carvalho Chehab  escreveu:

> Em Mon, 19 Jun 2017 16:56:13 +0900
> "Takiguchi, Yasunari"  escreveu:
> 
> > >> +static int cxd2880_get_frontend_t(struct dvb_frontend *fe,
> > >> +struct dtv_frontend_properties *c)
> > >> +{
> > >> +enum cxd2880_ret ret = CXD2880_RESULT_OK;
> > >> +int result = 0;
> > >> +struct cxd2880_priv *priv = NULL;
> > >> +enum cxd2880_dvbt_mode mode = CXD2880_DVBT_MODE_2K;
> > >> +enum cxd2880_dvbt_guard guard = CXD2880_DVBT_GUARD_1_32;
> > >> +struct cxd2880_dvbt_tpsinfo tps;
> > >> +enum cxd2880_tnrdmd_spectrum_sense sense;
> > >> +u16 snr = 0;
> > >> +int strength = 0;
> > >> +u32 pre_bit_err = 0, pre_bit_count = 0;
> > >> +u32 post_bit_err = 0, post_bit_count = 0;
> > >> +u32 block_err = 0, block_count = 0;
> > >> +
> > >> +if ((!fe) || (!c)) {
> > >> +pr_err("%s: invalid arg\n", __func__);
> > >> +return -EINVAL;
> > >> +}
> > >> +
> > >> +priv = (struct cxd2880_priv *)fe->demodulator_priv;
> > >> +
> > >> +mutex_lock(priv->spi_mutex);
> > >> +ret = cxd2880_tnrdmd_dvbt_mon_mode_guard(&priv->tnrdmd,
> > >> + &mode, &guard);
> > >> +mutex_unlock(priv->spi_mutex);
> > >> +if (ret == CXD2880_RESULT_OK) {
> > >> +switch (mode) {
> > >> +case CXD2880_DVBT_MODE_2K:
> > >> +c->transmission_mode = TRANSMISSION_MODE_2K;
> > >> +break;
> > >> +case CXD2880_DVBT_MODE_8K:
> > >> +c->transmission_mode = TRANSMISSION_MODE_8K;
> > >> +break;
> > >> +default:
> > >> +c->transmission_mode = TRANSMISSION_MODE_2K;
> > >> +dev_err(&priv->spi->dev, "%s: get invalid mode 
> > >> %d\n",
> > >> +__func__, mode);
> > >> +break;
> > >> +}
> > >> +switch (guard) {
> > >> +case CXD2880_DVBT_GUARD_1_32:
> > >> +c->guard_interval = GUARD_INTERVAL_1_32;
> > >> +break;
> > >> +case CXD2880_DVBT_GUARD_1_16:
> > >> +c->guard_interval = GUARD_INTERVAL_1_16;
> > >> +break;
> > >> +case CXD2880_DVBT_GUARD_1_8:
> > >> +c->guard_interval = GUARD_INTERVAL_1_8;
> > >> +break;
> > >> +case CXD2880_DVBT_GUARD_1_4:
> > >> +c->guard_interval = GUARD_INTERVAL_1_4;
> > >> +break;
> > >> +default:
> > >> +c->guard_interval = GUARD_INTERVAL_1_32;
> > >> +dev_err(&priv->spi->dev, "%s: get invalid guard 
> > >> %d\n",
> > >> +__func__, guard);
> > >> +break;
> > >> +}
> > >> +} else {
> > >> +c->transmission_mode = TRANSMISSION_MODE_2K;
> > >> +c->guard_interval = GUARD_INTERVAL_1_32;
> > >> +dev_dbg(&priv->spi->dev,
> > >> +"%s: ModeGuard err %d\n", __func__, ret);
> > >> +}
> > >> +
> > >> +mutex_lock(priv->spi_mutex);
> > >> +ret = cxd2880_tnrdmd_dvbt_mon_tps_info(&priv->tnrdmd, &tps);
> > >> +mutex_unlock(priv->spi_mutex);
> > >> +if (ret == CXD2880_RESULT_OK) {
> > >> +switch (tps.hierarchy) {
> > >> +case CXD2880_DVBT_HIERARCHY_NON:
> > >> +c->hierarchy = HIERARCHY_NONE;
> > >> +break;
> > >> +case CXD2880_DVBT_HIERARCHY_1:
> > >> +c->hierarchy = HIERARCHY_1;
> > >> +break;
> > >> +case CXD2880_DVBT_HIERARCHY_2:
> > >> +c->hierarchy = HIERARCHY_2;
> > >> +break;
> > >> +case CXD2880_DVBT_HIERARCHY_4:
> > >> +c->hierarchy = HIERARCHY_4;
> > >> +break;
> > >> +default:
> > >> +c->hierarchy = HIERARCHY_NONE;
> > >> +dev_err(&priv->spi->dev,
> > >> +"%s: TPSInfo hierarchy invalid %d\n",
> > >> +__func__, tps.hierarchy);
> > >> +break;
> > >> +}
> > >> +
> > >> +switch (tps.rate_hp) {
> > >> +case CXD2880_DVBT_CODERATE_1_2:
> > >> +c->code_rate_HP = FEC_1_2;
> > >> +break;
> > >> +case CXD2880_DV

[PATCH v3 3/4] [media] dvb-frontends/stv0367: DVB-C signal strength statistics

[Daniel Scheller]

From: Daniel Scheller 

Provide QAM/DVB-C signal strength in decibel scale. Values returned from
stv0367cab_get_rf_lvl() are good but need to be multiplied as they're in
1dBm precision.

Signed-off-by: Daniel Scheller 
---
 drivers/media/dvb-frontends/stv0367.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/media/dvb-frontends/stv0367.c 
b/drivers/media/dvb-frontends/stv0367.c
index 138f859d0f25..6097752a93bc 100644
--- a/drivers/media/dvb-frontends/stv0367.c
+++ b/drivers/media/dvb-frontends/stv0367.c
@@ -3011,6 +3011,25 @@ static int stv0367ddb_set_frontend(struct dvb_frontend 
*fe)
return -EINVAL;
 }
 
+static void stv0367ddb_read_signal_strength(struct dvb_frontend *fe)
+{
+   struct stv0367_state *state = fe->demodulator_priv;
+   struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+   s32 signalstrength;
+
+   switch (state->activedemod) {
+   case demod_cab:
+   signalstrength = stv0367cab_get_rf_lvl(state) * 1000;
+   break;
+   default:
+   p->strength.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+   return;
+   }
+
+   p->strength.stat[0].scale = FE_SCALE_DECIBEL;
+   p->strength.stat[0].uvalue = signalstrength;
+}
+
 static void stv0367ddb_read_snr(struct dvb_frontend *fe)
 {
struct stv0367_state *state = fe->demodulator_priv;
@@ -3086,6 +3105,8 @@ static int stv0367ddb_read_status(struct dvb_frontend *fe,
if (ret)
return ret;
 
+   stv0367ddb_read_signal_strength(fe);
+
/* read carrier/noise when a carrier is detected */
if (*status & FE_HAS_CARRIER)
stv0367ddb_read_snr(fe);
-- 
2.13.0

[PATCH v3 4/4] [media] dvb-frontends/stv0367: update UCB readout condition logic

[Daniel Scheller]

From: Daniel Scheller 

Since the other statistics are read when fe_status conditions are TRUE,
change the ucblocks readout logic to match this aswell.

Signed-off-by: Daniel Scheller 
---
 drivers/media/dvb-frontends/stv0367.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/media/dvb-frontends/stv0367.c 
b/drivers/media/dvb-frontends/stv0367.c
index 6097752a93bc..18ad1488be48 100644
--- a/drivers/media/dvb-frontends/stv0367.c
+++ b/drivers/media/dvb-frontends/stv0367.c
@@ -3113,13 +3113,11 @@ static int stv0367ddb_read_status(struct dvb_frontend 
*fe,
else
p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 
-   /* stop if demod isn't locked */
-   if (!(*status & FE_HAS_LOCK)) {
+   /* read uncorrected blocks on FE_HAS_LOCK */
+   if (*status & FE_HAS_LOCK)
+   stv0367ddb_read_ucblocks(fe);
+   else
p->block_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
-   return ret;
-   }
-
-   stv0367ddb_read_ucblocks(fe);
 
return 0;
 }
-- 
2.13.0

[PATCH v3 2/4] [media] dvb-frontends/stv0367: SNR DVBv5 statistics for DVB-C and T

[Daniel Scheller]

From: Daniel Scheller 

Add signal-to-noise-ratio as provided by the demodulator in decibel scale.
QAM/DVB-C needs some intlog calculation to have usable dB values, OFDM/
DVB-T values from the demod look alright already and are provided as-is.

Signed-off-by: Daniel Scheller 
---
 drivers/media/dvb-frontends/stv0367.c | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/drivers/media/dvb-frontends/stv0367.c 
b/drivers/media/dvb-frontends/stv0367.c
index 9e5432b761b5..138f859d0f25 100644
--- a/drivers/media/dvb-frontends/stv0367.c
+++ b/drivers/media/dvb-frontends/stv0367.c
@@ -25,6 +25,8 @@
 #include 
 #include 
 
+#include "dvb_math.h"
+
 #include "stv0367.h"
 #include "stv0367_defs.h"
 #include "stv0367_regs.h"
@@ -3009,6 +3011,37 @@ static int stv0367ddb_set_frontend(struct dvb_frontend 
*fe)
return -EINVAL;
 }
 
+static void stv0367ddb_read_snr(struct dvb_frontend *fe)
+{
+   struct stv0367_state *state = fe->demodulator_priv;
+   struct dtv_frontend_properties *p = &fe->dtv_property_cache;
+   int cab_pwr;
+   u32 regval, tmpval, snrval = 0;
+
+   switch (state->activedemod) {
+   case demod_ter:
+   snrval = stv0367ter_snr_readreg(fe);
+   break;
+   case demod_cab:
+   cab_pwr = stv0367cab_snr_power(fe);
+   regval = stv0367cab_snr_readreg(fe, 0);
+
+   /* prevent division by zero */
+   if (!regval)
+   snrval = 0;
+
+   tmpval = (cab_pwr * 320) / regval;
+   snrval = ((tmpval != 0) ? (intlog2(tmpval) / 5581) : 0);
+   break;
+   default:
+   p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+   return;
+   }
+
+   p->cnr.stat[0].scale = FE_SCALE_DECIBEL;
+   p->cnr.stat[0].uvalue = snrval;
+}
+
 static void stv0367ddb_read_ucblocks(struct dvb_frontend *fe)
 {
struct stv0367_state *state = fe->demodulator_priv;
@@ -3053,6 +3086,12 @@ static int stv0367ddb_read_status(struct dvb_frontend 
*fe,
if (ret)
return ret;
 
+   /* read carrier/noise when a carrier is detected */
+   if (*status & FE_HAS_CARRIER)
+   stv0367ddb_read_snr(fe);
+   else
+   p->cnr.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
+
/* stop if demod isn't locked */
if (!(*status & FE_HAS_LOCK)) {
p->block_error.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
-- 
2.13.0

[PATCH v3 0/4] STV0367/DDB DVBv5 signal statistics

[Daniel Scheller]

From: Daniel Scheller 

This series adds DVBv5 statistics support to the new DDB codepath of the
stv0367 demodulator driver.

The changes utilise already existing functionality (in form of register
readouts), but wraps the reads in separate functions so the existing
relative scale reporting can be kept as-is, while adding the v5 stats
in dB scale where appropriate.

The {ter,cab}_read_status() additionally have been enhanced to provide a
more detailed status from the values the logic already provides (at least
for DVB-C, DVB-T isn't as detailed, so all flags are set upon lock
instead).

>From my own testing: Reported values look approx. the same as those
reported by the cxd2841er driver for both -C and -T.

Testing for v3: The enhanced status works good for DVB-C - signal almost
instantly goes to FE_HAS_LOCK with good signal strength with the cabling
plugged in. When antenna is pulled, fe_status goes away, cnr goes to
unavailable and signal strength to the lowest the driver can provide.
Statistics come back after cable is plugged back in. Re DVB-T,
unfortunately I cannot do anymore testing with this, since over here in
germany DVB-T was replaced by DVB-T2, which the demod cannot handle.
Since it worked before and the logic didn't really change, I strongly
assume things are still good.

Mauro, re your patch (thanks again!), I slightly changed the commit title
and description but kept you as the author, added t-b and signoff. Also,
thank you for writing up the DVBv5 statistics document/howto, which was
very helpful in fixing this!

Hope everything's good to go now :-)

Changes from v2 to v3:
 - ucblocks reporting and register read out splitting are already merged,
   thus not part of the series anymore
 - enhanced {ter,cab}_read_status(), thank you Mauro for pointing this
   out and providing the (untested) patch!
 - always read signal strength, read cnr on FE_HAS_CARRIER, and ucblocks
   on FE_HAS_LOCK
 - adjust if-status-logic for ucblock to match the rest

Changes from v1 to v2:
 - INTLOG10X100() macro for QAM SNR calculation removed and replaced by
   directly utilising intlog2 plus a div
 - factored statistics collection into *_read_status()
 - prevent a possible division by zero (though requires ridiculously good
   SNR to trigger)
 - _read_status() doesn't return -EINVAL anymore if no demod state is set,
   prevents falsely reported errors from inquiries of userspace tools

Daniel Scheller (3):
  [media] dvb-frontends/stv0367: SNR DVBv5 statistics for DVB-C and T
  [media] dvb-frontends/stv0367: DVB-C signal strength statistics
  [media] dvb-frontends/stv0367: make UCB readout logic more clear

Mauro Carvalho Chehab (1):
  [media] dvb-frontends/stv0367: Improve DVB-C/T frontend status

 drivers/media/dvb-frontends/stv0367.c | 85 ---
 1 file changed, 78 insertions(+), 7 deletions(-)

-- 
2.13.0

[PATCH v3 1/4] [media] dvb-frontends/stv0367: Improve DVB-C/T frontend status

[Daniel Scheller]

From: Mauro Carvalho Chehab 

The stv0367 driver provide a lot of status on its state machine.
Change the logic to provide more information about frontend locking
status. Also, while any detailed status isn't available, provide a more
complete FE_STATUS for DVB-T.

Signed-off-by: Mauro Carvalho Chehab 
Tested-by: Daniel Scheller 
Signed-off-by: Daniel Scheller 
---
 drivers/media/dvb-frontends/stv0367.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/stv0367.c 
b/drivers/media/dvb-frontends/stv0367.c
index f266c18c574c..9e5432b761b5 100644
--- a/drivers/media/dvb-frontends/stv0367.c
+++ b/drivers/media/dvb-frontends/stv0367.c
@@ -1507,7 +1507,8 @@ static int stv0367ter_read_status(struct dvb_frontend *fe,
*status = 0;
 
if (stv0367_readbits(state, F367TER_LK)) {
-   *status |= FE_HAS_LOCK;
+   *status = FE_HAS_SIGNAL | FE_HAS_CARRIER | FE_HAS_VITERBI
+ | FE_HAS_SYNC | FE_HAS_LOCK;
dprintk("%s: stv0367 has locked\n", __func__);
}
 
@@ -2155,6 +2156,18 @@ static int stv0367cab_read_status(struct dvb_frontend 
*fe,
 
*status = 0;
 
+   if (state->cab_state->state > FE_CAB_NOSIGNAL)
+   *status |= FE_HAS_SIGNAL;
+
+   if (state->cab_state->state > FE_CAB_NOCARRIER)
+   *status |= FE_HAS_CARRIER;
+
+   if (state->cab_state->state >= FE_CAB_DEMODOK)
+   *status |= FE_HAS_VITERBI;
+
+   if (state->cab_state->state >= FE_CAB_DATAOK)
+   *status |= FE_HAS_SYNC;
+
if (stv0367_readbits(state, (state->cab_state->qamfec_status_reg ?
state->cab_state->qamfec_status_reg : F367CAB_QAMFEC_LOCK))) {
*status |= FE_HAS_LOCK;
-- 
2.13.0

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

[Daniel Scheller]

From: Daniel Scheller 

When the demod driver puts the demod into sleep or shutdown state and it's
status is then polled e.g. via "dvb-fe-tool -m", i2c errors are printed
to the kernel log. If the last delsys was DVB-T/T2:

  cxd2841er: i2c wr failed=-5 addr=6c reg=00 len=1
  cxd2841er: i2c rd failed=-5 addr=6c reg=26

and if it was DVB-C:

  cxd2841er: i2c wr failed=-5 addr=6c reg=00 len=1
  cxd2841er: i2c rd failed=-5 addr=6c reg=49

This happens when read_status unconditionally calls into the
read_signal_strength() function which triggers the read_agc_gain_*()
functions, where these registered are polled.

This isn't a critical thing since when the demod is active again, no more
such errors are logged, however this might make users suspecting defects.

Fix this by requiring STATE_ACTIVE_* in priv->state. If it isn't in any
active state, additionally set the strength scale to NOT_AVAILABLE.

Signed-off-by: Daniel Scheller 
---
V2/follow-up to https://patchwork.linuxtv.org/patch/42061/, changed as
requested. Tested, working fine (ie. no "false" i2c failures).

 drivers/media/dvb-frontends/cxd2841er.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/cxd2841er.c 
b/drivers/media/dvb-frontends/cxd2841er.c
index 08f67d60a7d9..12bff778c97f 100644
--- a/drivers/media/dvb-frontends/cxd2841er.c
+++ b/drivers/media/dvb-frontends/cxd2841er.c
@@ -3279,7 +3279,10 @@ static int cxd2841er_get_frontend(struct dvb_frontend 
*fe,
else if (priv->state == STATE_ACTIVE_TC)
cxd2841er_read_status_tc(fe, &status);
 
-   cxd2841er_read_signal_strength(fe);
+   if (priv->state == STATE_ACTIVE_TC || priv->state == STATE_ACTIVE_S)
+   cxd2841er_read_signal_strength(fe);
+   else
+   p->strength.stat[0].scale = FE_SCALE_NOT_AVAILABLE;
 
if (status & FE_HAS_LOCK) {
cxd2841er_read_snr(fe);
-- 
2.13.0

[PATCH] [media] ddbridge: dev_* logging fixup

[Daniel Scheller]

From: Daniel Scheller 

Fixup

  commit d52786ddd2d5 ("media: ddbridge: make (ddb)readl in while-loops 
fail-safe")

after/wrt

  commit 11e358bf37e8 ("media: ddbridge: use dev_* macros in favor of printk")

Signed-off-by: Daniel Scheller 
---
 drivers/media/pci/ddbridge/ddbridge-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
b/drivers/media/pci/ddbridge/ddbridge-core.c
index c5745ae2ba5e..32f4d3746c8e 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -122,7 +122,7 @@ static inline u32 safe_ddbreadl(struct ddb *dev, u32 adr)
 
/* (ddb)readl returns (uint)-1 (all bits set) on failure, catch that */
if (val == ~0) {
-   printk(KERN_ERR "ddbreadl failure, adr=%08x\n", adr);
+   dev_err(&dev->pdev->dev, "ddbreadl failure, adr=%08x\n", adr);
return 0;
}
 
-- 
2.13.0

omap3isp camera was Re: [PATCH v1 0/6] Add support of OV9655 camera

[Pavel Machek]

Hi!

> * unfortunately we still get no image :(
> 
> The latter is likely a setup issue of our camera interface (OMAP3 ISP = Image 
> Signal Processor) which
> we were not yet able to solve. Oscilloscoping signals on the interface 
> indicated that signals and
> sync are correct. But we do not know since mplayer only shows a green screen.

What mplayer command line do you use? How did you set up the pipeline
with media-ctl?

On kernel.org, I have tree called camera-fw5-6 , where camera works
for me on n900. On gitlab, there's modifed fcam-dev, which can be used
for testing.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature