cron job: media_tree daily build: ERRORS

[Hans Verkuil]

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

Results of the daily build of media_tree:

date:   Wed Mar  7 05:00:12 CET 2018
media-tree git hash:60d0bbec5965590d72b1a2091ec7a2cc589cb8e0
media_build git hash:   15e592e1abe9d29c9fd3b32302ac50716ef793d5
v4l-utils git hash: c28248deeb2d7fe43fcde948c00b9b8fa2bc1e8f
gcc version:i686-linux-gcc (GCC) 7.3.0
sparse version: v0.5.0-3994-g45eb2282
smatch version: v0.5.0-3994-g45eb2282
host hardware:  x86_64
host os:4.14.0-3-amd64

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

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[linuxtv-media:master 180/263] drivers/media/usb/em28xx/em28xx-dvb.c:1140:26: error: implicit declaration of function 'dvb_module_probe'; did you mean 'module_put'?

[kbuild test robot]

tree:   git://linuxtv.org/media_tree.git master
head:   60d0bbec5965590d72b1a2091ec7a2cc589cb8e0
commit: ad32495b1513fe8cbab717411b9cd8d2d285de30 [180/263] media: em28xx-dvb: 
simplify DVB module probing logic
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout ad32495b1513fe8cbab717411b9cd8d2d285de30
# save the attached .config to linux build tree
make.cross ARCH=ia64 

All error/warnings (new ones prefixed by >>):

   drivers/media/usb/em28xx/em28xx-dvb.c: In function 
'em28174_dvb_init_pctv_460e':
>> drivers/media/usb/em28xx/em28xx-dvb.c:1140:26: error: implicit declaration 
>> of function 'dvb_module_probe'; did you mean 'module_put'? 
>> [-Werror=implicit-function-declaration]
 dvb->i2c_client_demod = dvb_module_probe("tda10071", "tda10071_cx24118",
 ^~~~
 module_put
>> drivers/media/usb/em28xx/em28xx-dvb.c:1140:24: warning: assignment makes 
>> pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_demod = dvb_module_probe("tda10071", "tda10071_cx24118",
   ^
   drivers/media/usb/em28xx/em28xx-dvb.c:1151:22: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_sec = dvb_module_probe("a8293", NULL,
 ^
>> drivers/media/usb/em28xx/em28xx-dvb.c:1155:3: error: implicit declaration of 
>> function 'dvb_module_release'; did you mean 'dvb_dmxdev_release'? 
>> [-Werror=implicit-function-declaration]
  dvb_module_release(dvb->i2c_client_demod);
  ^~
  dvb_dmxdev_release
   drivers/media/usb/em28xx/em28xx-dvb.c: In function 
'em28178_dvb_init_pctv_461e':
   drivers/media/usb/em28xx/em28xx-dvb.c:1178:24: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_demod = dvb_module_probe("m88ds3103", NULL,
   ^
   drivers/media/usb/em28xx/em28xx-dvb.c:1190:24: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_tuner = dvb_module_probe("ts2020", "ts2022",
   ^
   drivers/media/usb/em28xx/em28xx-dvb.c:1204:22: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_sec = dvb_module_probe("a8293", NULL,
 ^
   drivers/media/usb/em28xx/em28xx-dvb.c: In function 
'em28178_dvb_init_pctv_292e':
   drivers/media/usb/em28xx/em28xx-dvb.c:1228:24: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_demod = dvb_module_probe("si2168", NULL,
   ^
   drivers/media/usb/em28xx/em28xx-dvb.c:1240:24: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_tuner = dvb_module_probe("si2157", NULL,
   ^
   drivers/media/usb/em28xx/em28xx-dvb.c: In function 
'em28178_dvb_init_terratec_t2_stick_hd':
   drivers/media/usb/em28xx/em28xx-dvb.c:1264:24: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_demod = dvb_module_probe("si2168", NULL,
   ^
   drivers/media/usb/em28xx/em28xx-dvb.c:1277:24: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_tuner = dvb_module_probe("si2157", "si2146",
   ^
   drivers/media/usb/em28xx/em28xx-dvb.c: In function 
'em28178_dvb_init_plex_px_bcud':
   drivers/media/usb/em28xx/em28xx-dvb.c:1295:24: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_demod = dvb_module_probe("tc90522", "tc90522sat",
   ^
   drivers/media/usb/em28xx/em28xx-dvb.c:1305:24: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_tuner = dvb_module_probe("qm1d1c0042", NULL,
   ^
   drivers/media/usb/em28xx/em28xx-dvb.c: In function 
'em28174_dvb_init_hauppauge_wintv_dualhd_dvb':
   drivers/media/usb/em28xx/em28xx-dvb.c:1333:24: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_demod = dvb_module_probe("si2168", NULL,
   ^
   drivers/media/usb/em28xx/em28xx-dvb.c:1348:24: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]
 dvb->i2c_client_tuner = dvb_module_probe("si2157", NULL,
   ^
   drivers/media/usb/em28xx/em28xx-dvb.c: In function 
'em28174_dvb_init_hauppauge_wintv_dualhd_01595':
   drivers/media/usb/em28xx/em28xx-dvb.c:1373:24: warning: assignment makes 
pointer from integer without a cast [-Wint-conversion]

Re: [PATCHv2 5/7] cec-pin: add error injection support

[Hans Verkuil]

On 05/03/18 14:51, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Implement all the error injection commands.
> 
> The state machine gets new states for the various error situations,
> helper functions are added to detect whether an error injection is
> active and the actual error injections are implemented.
> 
> Signed-off-by: Hans Verkuil 
> ---
>  drivers/media/cec/cec-pin-priv.h |  38 ++-
>  drivers/media/cec/cec-pin.c  | 542 
> +++
>  2 files changed, 521 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/media/cec/cec-pin-priv.h 
> b/drivers/media/cec/cec-pin-priv.h
> index 779384f18689..c9349f68e554 100644
> --- a/drivers/media/cec/cec-pin-priv.h
> +++ b/drivers/media/cec/cec-pin-priv.h



> +static bool tx_error_inj(struct cec_pin *pin, unsigned int mode_offset,
> +  int arg_idx, u8 *arg)
> +{
> +#ifdef CONFIG_CEC_PIN_ERROR_INJ
> + u16 cmd = cec_pin_tx_error_inj(pin);
> + u64 e = pin->error_inj[cmd];
> + unsigned int mode = (e >> mode_offset) & CEC_ERROR_INJ_MODE_MASK;
> +
> + if (arg_idx) {

Oops, this should be arg_idx >= 0. It's -1 if there is no argument associated
with the error injection command.

> + u8 pos = pin->error_inj_args[cmd][arg_idx];
> +
> + if (arg)
> + *arg = pos;
> + else if (pos != pin->tx_bit)
> + return false;
> + }
> +
> + switch (mode) {
> + case CEC_ERROR_INJ_MODE_ONCE:
> + pin->error_inj[cmd] &= ~(CEC_ERROR_INJ_MODE_MASK << 
> mode_offset);
> + return true;
> + case CEC_ERROR_INJ_MODE_ALWAYS:
> + return true;
> + case CEC_ERROR_INJ_MODE_TOGGLE:
> + return pin->tx_toggle;
> + default:
> + return false;
> + }
> +#else
> + return false;
> +#endif
> +}



> + case EOM_BIT:
> + v = pin->tx_bit / 10 ==
> + pin->tx_msg.len + pin->tx_extra_bytes - 1;
> + if (pin->tx_msg.len > 1 && tx_early_eom(pin)) {

tx_early_eom should only be called for the second-to-last byte.

> + /* Error injection: set EOM one byte early */
> + v = pin->tx_bit / 10 ==
> + pin->tx_msg.len + pin->tx_extra_bytes - 
> 2;
> + pin->tx_post_eom = true;
> + }
> + if (tx_no_eom(pin)) {

Ditto for tx_no_eom: only call for the last byte.

Otherwise for mode 'once' this error injection command will be cleared
before the end of the message is reached.

> + /* Error injection: no EOM */
> + v = false;
> + }
>   pin->state = v ? CEC_ST_TX_DATA_BIT_1_LOW :
> - CEC_ST_TX_DATA_BIT_0_LOW;
> +  CEC_ST_TX_DATA_BIT_0_LOW;
>   break;

Regards,

Hans

[PATCH] cec: improve CEC pin event handling

[Hans Verkuil]

It turns out that the struct cec_fh event buffer size of 64 events
(64 for CEC_EVENT_PIN_CEC_LOW and 64 for _HIGH) is too small. It's
about 160 ms worth of events and if the Raspberry Pi is busy, then it
might take too long for the application to be scheduled so that it can
drain the pending events. Increase these buffers to 800 events which
is at least 2 seconds worth of events.

There is also a FIFO in between the interrupt and the cec-pin thread.
The thread passes the events on to the CEC core. It is important that
should this FIFO fill up the cec core will be informed that events
have been lost so this can be communicated to the user by setting
CEC_EVENT_FL_DROPPED_EVENTS.

It is very hard to debug CEC problems if events were lost without
informing the user of that fact.

If events were dropped due to the FIFO filling up, then the debugfs
status file will let you know how many events were dropped.

Signed-off-by: Hans Verkuil 
---
Note: this patch sits on top of my earlier "[PATCHv2 0/7] cec: add error
injection support" patch series
---
 drivers/media/cec/cec-adap.c |  8 +---
 drivers/media/cec/cec-pin-priv.h | 10 +++---
 drivers/media/cec/cec-pin.c  | 30 ++
 drivers/media/platform/vivid/vivid-cec.c |  8 
 include/media/cec.h  |  7 ---
 5 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index cf6dc3f3a17e..002ed4c90371 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -73,8 +73,8 @@ static unsigned int cec_log_addr2dev(const struct cec_adapter 
*adap, u8 log_addr
 void cec_queue_event_fh(struct cec_fh *fh,
const struct cec_event *new_ev, u64 ts)
 {
-   static const u8 max_events[CEC_NUM_EVENTS] = {
-   1, 1, 64, 64, 8, 8,
+   static const u16 max_events[CEC_NUM_EVENTS] = {
+   1, 1, 800, 800, 8, 8,
};
struct cec_event_entry *entry;
unsigned int ev_idx = new_ev->event - 1;
@@ -142,11 +142,13 @@ static void cec_queue_event(struct cec_adapter *adap,
 }

 /* Notify userspace that the CEC pin changed state at the given time. */
-void cec_queue_pin_cec_event(struct cec_adapter *adap, bool is_high, ktime_t 
ts)
+void cec_queue_pin_cec_event(struct cec_adapter *adap, bool is_high,
+bool dropped_events, ktime_t ts)
 {
struct cec_event ev = {
.event = is_high ? CEC_EVENT_PIN_CEC_HIGH :
   CEC_EVENT_PIN_CEC_LOW,
+   .flags = dropped_events ? CEC_EVENT_FL_DROPPED_EVENTS : 0,
};
struct cec_fh *fh;

diff --git a/drivers/media/cec/cec-pin-priv.h b/drivers/media/cec/cec-pin-priv.h
index dae8ba6f1037..f423db8855d9 100644
--- a/drivers/media/cec/cec-pin-priv.h
+++ b/drivers/media/cec/cec-pin-priv.h
@@ -153,7 +153,9 @@ enum cec_pin_state {
 /* The default for the low/high time of the custom pulse */
 #define CEC_TIM_CUSTOM_DEFAULT 1000

-#define CEC_NUM_PIN_EVENTS 128
+#define CEC_NUM_PIN_EVENTS 128
+#define CEC_PIN_EVENT_FL_IS_HIGH   (1 << 0)
+#define CEC_PIN_EVENT_FL_DROPPED   (1 << 1)

 #define CEC_PIN_IRQ_UNCHANGED  0
 #define CEC_PIN_IRQ_DISABLE1
@@ -198,11 +200,13 @@ struct cec_pin {
u8  work_tx_status;
ktime_t work_tx_ts;
atomic_twork_irq_change;
-   atomic_twork_pin_events;
+   atomic_twork_pin_num_events;
unsigned intwork_pin_events_wr;
unsigned intwork_pin_events_rd;
ktime_t work_pin_ts[CEC_NUM_PIN_EVENTS];
-   boolwork_pin_is_high[CEC_NUM_PIN_EVENTS];
+   u8  work_pin_events[CEC_NUM_PIN_EVENTS];
+   boolwork_pin_events_dropped;
+   u32 work_pin_events_dropped_cnt;
ktime_t timer_ts;
u32 timer_cnt;
u32 timer_100ms_overruns;
diff --git a/drivers/media/cec/cec-pin.c b/drivers/media/cec/cec-pin.c
index b509df154ca1..64e17f1b6fd3 100644
--- a/drivers/media/cec/cec-pin.c
+++ b/drivers/media/cec/cec-pin.c
@@ -114,12 +114,21 @@ static void cec_pin_update(struct cec_pin *pin, bool v, 
bool force)
return;

pin->adap->cec_pin_is_high = v;
-   if (atomic_read(>work_pin_events) < CEC_NUM_PIN_EVENTS) {
-   pin->work_pin_is_high[pin->work_pin_events_wr] = v;
+   if (atomic_read(>work_pin_num_events) < CEC_NUM_PIN_EVENTS) {
+   u8 ev = v;
+
+   if (pin->work_pin_events_dropped) {
+ 

Re: [PATCH 3/3] media: vb2-core: vb2_ops: document non-interrupt-cantext calling

[Luca Ceresoli]

Hi,

I noticed a typo in the title:
cantext -> context

I will fix in v2.

On 28/02/2018 23:24, Luca Ceresoli wrote:
> Driver writers can benefit in knowing if/when callbacks are called in
> interrupt context. But it is not completely obvious here, so document it.
> 
> Signed-off-by: Luca Ceresoli 
> Cc: Laurent Pinchart 
> Cc: Pawel Osciak 
> Cc: Marek Szyprowski 
> Cc: Kyungmin Park 
> Cc: Mauro Carvalho Chehab 
> ---
>  include/media/videobuf2-core.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index f2887d3c..f6818f732f34 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -296,6 +296,9 @@ struct vb2_buffer {
>  /**
>   * struct vb2_ops - driver-specific callbacks.
>   *
> + * These operations are not called from interrupt context except where
> + * mentioned specifically.
> + *
>   * @queue_setup: called from VIDIOC_REQBUFS() and VIDIOC_CREATE_BUFS()
>   *   handlers before memory allocation. It can be called
>   *   twice: if the original number of requested buffers
> 


-- 
Luca

[PATCH 4/4] cx23885: Override 888 ImpactVCBe crystal frequency

[Brad Love]

Hauppauge produced a revision of ImpactVCBe using an 888,
with a 25MHz crystal, instead of using the default third
overtone 50Mhz crystal. This overrides that frequency so
that the cx25840 is properly configured. Without the proper
crystal setup the cx25840 cannot load the firmware or
decode video.

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

diff --git a/drivers/media/pci/cx23885/cx23885-core.c 
b/drivers/media/pci/cx23885/cx23885-core.c
index 2c76a02..ff369e9 100644
--- a/drivers/media/pci/cx23885/cx23885-core.c
+++ b/drivers/media/pci/cx23885/cx23885-core.c
@@ -881,6 +881,16 @@ static int cx23885_dev_setup(struct cx23885_dev *dev)
if (cx23885_boards[dev->board].clk_freq > 0)
dev->clk_freq = cx23885_boards[dev->board].clk_freq;
 
+   if (dev->board == CX23885_BOARD_HAUPPAUGE_IMPACTVCBE &&
+   dev->pci->subsystem_device == 0x7137) {
+   /* Hauppauge ImpactVCBe device ID 0x7137 is populated
+* with an 888, and a 25Mhz crystal, instead of the
+* usual third overtone 50Mhz. The default clock rate must
+* be overridden so the cx25840 is properly configured
+*/
+   dev->clk_freq = 2500;
+   }
+
dev->pci_bus  = dev->pci->bus->number;
dev->pci_slot = PCI_SLOT(dev->pci->devfn);
cx23885_irq_add(dev, 0x001f00);
-- 
2.7.4

[PATCH 1/4] cx25840: Use subdev host data for PLL override

[Brad Love]

The cx25840 driver currently configures 885, 887, and 888 using
default divisors for each chip. This check to see if the cx23885
driver has passed the cx25840 a non-default clock rate for a
specific chip. If a cx23885 board has left clk_freq at 0, the
clock default values will be used to configure the PLLs.

This patch only has effect on 888 boards who set clk_freq to 25M.

Signed-off-by: Brad Love 
---
 drivers/media/i2c/cx25840/cx25840-core.c | 28 ++--
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-core.c 
b/drivers/media/i2c/cx25840/cx25840-core.c
index 98be63a..b168bf3 100644
--- a/drivers/media/i2c/cx25840/cx25840-core.c
+++ b/drivers/media/i2c/cx25840/cx25840-core.c
@@ -463,8 +463,13 @@ static void cx23885_initialize(struct i2c_client *client)
 {
DEFINE_WAIT(wait);
struct cx25840_state *state = to_state(i2c_get_clientdata(client));
+   u32 clk_freq = 0;
struct workqueue_struct *q;
 
+   /* cx23885 sets hostdata to clk_freq pointer */
+   if (v4l2_get_subdev_hostdata(>sd))
+   clk_freq = *((u32 *)v4l2_get_subdev_hostdata(>sd));
+
/*
 * Come out of digital power down
 * The CX23888, at least, needs this, otherwise registers aside from
@@ -500,8 +505,13 @@ static void cx23885_initialize(struct i2c_client *client)
 * 50.0 MHz * (0xb + 0xe8ba26/0x200)/4 = 5 * 28.636363 MHz
 * 572.73 MHz before post divide
 */
-   /* HVR1850 or 50MHz xtal */
-   cx25840_write(client, 0x2, 0x71);
+   if (clk_freq == 2500) {
+   /* 888/ImpactVCBe or 25Mhz xtal */
+   ; /* nothing to do */
+   } else {
+   /* HVR1850 or 50MHz xtal */
+   cx25840_write(client, 0x2, 0x71);
+   }
cx25840_write4(client, 0x11c, 0x01d1744c);
cx25840_write4(client, 0x118, 0x0416);
cx25840_write4(client, 0x404, 0x0010253e);
@@ -544,9 +554,15 @@ static void cx23885_initialize(struct i2c_client *client)
/* HVR1850 */
switch (state->id) {
case CX23888_AV:
-   /* 888/HVR1250 specific */
-   cx25840_write4(client, 0x10c, 0x1333);
-   cx25840_write4(client, 0x108, 0x0515);
+   if (clk_freq == 2500) {
+   /* 888/ImpactVCBe or 25MHz xtal */
+   cx25840_write4(client, 0x10c, 0x01b6db7b);
+   cx25840_write4(client, 0x108, 0x0512);
+   } else {
+   /* 888/HVR1250 or 50MHz xtal */
+   cx25840_write4(client, 0x10c, 0x1333);
+   cx25840_write4(client, 0x108, 0x0515);
+   }
break;
default:
cx25840_write4(client, 0x10c, 0x002be2c9);
@@ -576,7 +592,7 @@ static void cx23885_initialize(struct i2c_client *client)
 * 368.64 MHz before post divide
 * 122.88 MHz / 0xa = 12.288 MHz
 */
-   /* HVR1850  or 50MHz xtal */
+   /* HVR1850 or 50MHz xtal or 25MHz xtal */
cx25840_write4(client, 0x114, 0x017dbf48);
cx25840_write4(client, 0x110, 0x000a030e);
break;
-- 
2.7.4

[PATCH 3/4] cx23885: Set subdev host data to clk_freq pointer

[Brad Love]

Currently clk_freq is ignored entirely, because the cx235840 driver
configures the xtal at the chip defaults. This is an issue if a
board is produced with a non-default frequency crystal. If clk_freq
is not zero the cx25840 will attempt to use the setting provided,
or fall back to defaults otherwise.

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

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c 
b/drivers/media/pci/cx23885/cx23885-cards.c
index 41f5669..3a1c551 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -2362,6 +2362,10 @@ void cx23885_card_setup(struct cx23885_dev *dev)
>i2c_bus[2].i2c_adap,
"cx25840", 0x88 >> 1, NULL);
if (dev->sd_cx25840) {
+   /* set host data for clk_freq configuration */
+   v4l2_set_subdev_hostdata(dev->sd_cx25840,
+   >clk_freq);
+
dev->sd_cx25840->grp_id = CX23885_HW_AV_CORE;
v4l2_subdev_call(dev->sd_cx25840, core, load_fw);
}
-- 
2.7.4

[PATCH 0/4] [resend] cx25840: Add non-default crystal frequency support

[Brad Love]

Hauppauge produced a revision of ImpactVCBe using an 888,
with a 25MHz crystal, instead of using the default third
overtone 50Mhz crystal. Custom configuration of the cx25840
is required to load the firmware and properly decode video.

This patch set add an extra clocking option to the cx25840 driver.
If a cx2388x board is produced using non-default crystal, then
clk_freq can be set in the board profile and then used by the
cx25840 to set up the PLL's. To pass frequency, the cx23885 driver
sets the cx25840 v4l subdev host data to a pointer to clk_freq.

The only additional clock rate included is cx23888 with 25MHz crystal.

Brad Love (4):
  cx25840: Use subdev host data for PLL override
  cx23885: change 887/888 default to 888
  cx23885: Set subdev host data to clk_freq pointer
  cx23885: Override 888 ImpactVCBe crystal frequency

 drivers/media/i2c/cx25840/cx25840-core.c  | 28 ++--
 drivers/media/pci/cx23885/cx23885-cards.c |  4 
 drivers/media/pci/cx23885/cx23885-core.c  | 16 +---
 3 files changed, 39 insertions(+), 9 deletions(-)

-- 
2.7.4

[PATCH 2/4] cx23885: change 887/888 default to 888

[Brad Love]

Proper cx2388x chip type is detected in cx25840 probe, the clock
rate is untouched however in probe. The cx25840 only checks for
non default clock values for 888 and provides custom settings for
25MHz 888. This change ensures that cx23888 chips with default 50MHz
crystals will not get configured as if they have 25MHz crystals. A
cx23887 board will continue to be configured for 25Hz crystal as
there is no custom clock support included for it.

Signed-off-by: Brad Love 
---
 drivers/media/pci/cx23885/cx23885-core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-core.c 
b/drivers/media/pci/cx23885/cx23885-core.c
index 8afddd6..2c76a02 100644
--- a/drivers/media/pci/cx23885/cx23885-core.c
+++ b/drivers/media/pci/cx23885/cx23885-core.c
@@ -839,10 +839,10 @@ static int cx23885_dev_setup(struct cx23885_dev *dev)
 
/* Configure the internal memory */
if (dev->pci->device == 0x8880) {
-   /* Could be 887 or 888, assume a default */
-   dev->bridge = CX23885_BRIDGE_887;
+   /* Could be 887 or 888, assume an 888 default */
+   dev->bridge = CX23885_BRIDGE_888;
/* Apply a sensible clock frequency for the PCIe bridge */
-   dev->clk_freq = 2500;
+   dev->clk_freq = 5000;
dev->sram_channels = cx23887_sram_channels;
} else
if (dev->pci->device == 0x8852) {
-- 
2.7.4

[PATCH 8/8] cx23885: Fix gpio on Hauppauge QuadHD PCIe cards

[Brad Love]

Bug fix for gpios

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

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c 
b/drivers/media/pci/cx23885/cx23885-cards.c
index 5a64098..41f5669 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -1828,8 +1828,6 @@ void cx23885_gpio_setup(struct cx23885_dev *dev)
cx23885_gpio_set(dev, GPIO_2);
break;
case CX23885_BOARD_HAUPPAUGE_HVR5525:
-   case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
-   case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
case CX23885_BOARD_HAUPPAUGE_STARBURST2:
/*
 * HVR5525 GPIO Details:
@@ -1861,7 +1859,9 @@ void cx23885_gpio_setup(struct cx23885_dev *dev)
 */
break;
case CX23885_BOARD_HAUPPAUGE_HVR1265_K4:
+   case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB:
case CX23885_BOARD_HAUPPAUGE_QUADHD_DVB_885:
+   case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC:
case CX23885_BOARD_HAUPPAUGE_QUADHD_ATSC_885:
/*
 * GPIO-08 TER1_RESN
-- 
2.7.4

[PATCH 1/8] lgdt3306a: remove symbol count mismatch fix

[Brad Love]

This symbol mismatch is handled by NULL'ing out the release
callback if the driver is loaded as an i2c device.

This patch reverts:
- 94448e21cf08b10f7dc7acdaca387594370396b0
- 835d66173a38538c072a7c393d02360dcfac8582

The symbol count mismatch is handled by:
- 5b3a8e906973540b61dbf402c6b6f8d64d4ae119

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

diff --git a/drivers/media/dvb-frontends/lgdt3306a.c 
b/drivers/media/dvb-frontends/lgdt3306a.c
index 5b19033..7eb4e14 100644
--- a/drivers/media/dvb-frontends/lgdt3306a.c
+++ b/drivers/media/dvb-frontends/lgdt3306a.c
@@ -1814,13 +1814,7 @@ static void lgdt3306a_release(struct dvb_frontend *fe)
struct lgdt3306a_state *state = fe->demodulator_priv;
 
dbg_info("\n");
-
-   /*
-* If state->muxc is not NULL, then we are an i2c device
-* and lgdt3306a_remove will clean up state
-*/
-   if (!state->muxc)
-   kfree(state);
+   kfree(state);
 }
 
 static const struct dvb_frontend_ops lgdt3306a_ops;
@@ -2221,7 +2215,7 @@ static int lgdt3306a_probe(struct i2c_client *client,
sizeof(struct lgdt3306a_config));
 
config->i2c_addr = client->addr;
-   fe = dvb_attach(lgdt3306a_attach, config, client->adapter);
+   fe = lgdt3306a_attach(config, client->adapter);
if (fe == NULL) {
ret = -ENODEV;
goto err_fe;
-- 
2.7.4

[PATCH 3/8] cx231xx: Use frontend i2c adapter with tuner

[Brad Love]

Utilize the i2c mux adapter returned by the frontend.

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

diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c 
b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index 63deca9..c3b2d69 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -1221,7 +1221,7 @@ static int dvb_init(struct cx231xx *dev)
info.platform_data = _config;
request_module("si2157");
 
-   client = i2c_new_device(tuner_i2c, );
+   client = i2c_new_device(adapter, );
if (client == NULL || client->dev.driver == NULL) {
module_put(dvb->i2c_client_demod[0]->dev.driver->owner);
i2c_unregister_device(dvb->i2c_client_demod[0]);
-- 
2.7.4

[PATCH 4/8] cx23885: Add tuner type and analog inputs to 1265

[Brad Love]

Missing composite and s-video inputs

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

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c 
b/drivers/media/pci/cx23885/cx23885-cards.c
index 831b066..5a64098 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -788,8 +788,24 @@ struct cx23885_board cx23885_boards[] = {
},
[CX23885_BOARD_HAUPPAUGE_HVR1265_K4] = {
.name   = "Hauppauge WinTV-HVR-1265(16)",
+   .porta  = CX23885_ANALOG_VIDEO,
.portc  = CX23885_MPEG_DVB,
+   .tuner_type = TUNER_ABSENT,
.force_bff  = 1,
+   .input  = {{
+   .type   = CX23885_VMUX_COMPOSITE1,
+   .vmux   =   CX25840_VIN7_CH3 |
+   CX25840_VIN4_CH2 |
+   CX25840_VIN6_CH1,
+   .amux   = CX25840_AUDIO7,
+   }, {
+   .type   = CX23885_VMUX_SVIDEO,
+   .vmux   =   CX25840_VIN7_CH3 |
+   CX25840_VIN4_CH2 |
+   CX25840_VIN8_CH1 |
+   CX25840_SVIDEO_ON,
+   .amux   = CX25840_AUDIO7,
+   } },
},
[CX23885_BOARD_HAUPPAUGE_STARBURST2] = {
.name   = "Hauppauge WinTV-Starburst2",
-- 
2.7.4

[PATCH 0/8] FIXME: Assorted of missed bits from merge

[Brad Love]

Hello Mauro,

Here are the assorted bits and bobs that wound up missing
due to the patchwork snafu.

One new patch is:
- cx231xx: Set mfe_shared if second frontend found

Due to your suggestion in regards to the shared tuner logic.
I put the check in what seems like a sensical spot.

Brad Love (8):
  lgdt3306a: remove symbol count mismatch fix
  em28xx: Change hex to lower case
  cx231xx: Use frontend i2c adapter with tuner
  cx23885: Add tuner type and analog inputs to 1265
  cx231xx: Set mfe_shared if second frontend found
  cx231xx: Use constant instead of hard code for max
  cx231xx: Add second i2c demod to Hauppauge 975
  cx23885: Fix gpio on Hauppauge QuadHD PCIe cards

 drivers/media/dvb-frontends/lgdt3306a.c   | 10 ++
 drivers/media/pci/cx23885/cx23885-cards.c | 20 +--
 drivers/media/usb/cx231xx/cx231xx-cards.c |  1 +
 drivers/media/usb/cx231xx/cx231xx-dvb.c   | 60 ---
 drivers/media/usb/em28xx/em28xx-core.c|  2 +-
 5 files changed, 77 insertions(+), 16 deletions(-)

-- 
2.7.4

[PATCH 2/8] em28xx: Change hex to lower case

[Brad Love]

Checkpatch fix.

Signed-off-by: Brad Love 
---
 drivers/media/usb/em28xx/em28xx-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/em28xx/em28xx-core.c 
b/drivers/media/usb/em28xx/em28xx-core.c
index bb1b650..36d341f 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -643,7 +643,7 @@ int em28xx_capture_start(struct em28xx *dev, int start)
em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
 EM2874_R5D_TS1_PKT_SIZE :
 EM2874_R5E_TS2_PKT_SIZE,
-0xFF);
+0xff);
} else {
/* ISOC Maximum Transfer Size = 188 * 5 */
em28xx_write_reg(dev, (dev->ts == PRIMARY_TS) ?
-- 
2.7.4

[PATCH 6/8] cx231xx: Use constant instead of hard code for max

[Brad Love]

Nit regarding hard coded value.

Signed-off-by: Brad Love 
---
 drivers/media/usb/cx231xx/cx231xx-dvb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c 
b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index 9d326a0..21e7817 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -53,9 +53,10 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
 #define CX231XX_DVB_NUM_BUFS 5
 #define CX231XX_DVB_MAX_PACKETSIZE 564
 #define CX231XX_DVB_MAX_PACKETS 64
+#define CX231XX_DVB_MAX_FRONTENDS 2
 
 struct cx231xx_dvb {
-   struct dvb_frontend *frontend[2];
+   struct dvb_frontend *frontend[CX231XX_DVB_MAX_FRONTENDS];
 
/* feed count management */
struct mutex lock;
-- 
2.7.4

[PATCH 5/8] cx231xx: Set mfe_shared if second frontend found

[Brad Love]

If frontend[1] exists, then enable the dvb adapter mfe lock system.

Signed-off-by: Brad Love 
---
 drivers/media/usb/cx231xx/cx231xx-dvb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c 
b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index c3b2d69..9d326a0 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -504,6 +504,9 @@ static int register_dvb(struct cx231xx_dvb *dvb,
dev->name, result);
goto fail_frontend1;
}
+
+   /* MFE lock */
+   dvb->adapter.mfe_shared = 1;
}
 
/* register demux stuff */
-- 
2.7.4

[PATCH 7/8] cx231xx: Add second i2c demod to Hauppauge 975

[Brad Love]

Hauppauge HVR-975 is a hybrid, dual frontend, single tuner USB device.
It contains lgdt3306a and si2168 frontends and one si2157 tuner. The
lgdt3306a frontend is currently enabled. This creates the second
demodulator and attaches it to the tuner.

Enables lgdt3306a|si2168 + si2157

Signed-off-by: Brad Love 
---
 drivers/media/usb/cx231xx/cx231xx-cards.c |  1 +
 drivers/media/usb/cx231xx/cx231xx-dvb.c   | 52 +--
 2 files changed, 50 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c 
b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 8582568..00e88a8f 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -979,6 +979,7 @@ struct cx231xx_board cx231xx_boards[] = {
.demod_i2c_master = I2C_1_MUX_3,
.has_dvb = 1,
.demod_addr = 0x59, /* 0xb2 >> 1 */
+   .demod_addr2 = 0x64, /* 0xc8 >> 1 */
.norm = V4L2_STD_ALL,
 
.input = {{
diff --git a/drivers/media/usb/cx231xx/cx231xx-dvb.c 
b/drivers/media/usb/cx231xx/cx231xx-dvb.c
index 21e7817..72f83d0 100644
--- a/drivers/media/usb/cx231xx/cx231xx-dvb.c
+++ b/drivers/media/usb/cx231xx/cx231xx-dvb.c
@@ -1177,14 +1177,17 @@ static int dvb_init(struct cx231xx *dev)
{
struct i2c_client *client;
struct i2c_adapter *adapter;
+   struct i2c_adapter *adapter2;
struct i2c_board_info info = {};
struct si2157_config si2157_config = {};
struct lgdt3306a_config lgdt3306a_config = {};
+   struct si2168_config si2168_config = {};
 
-   /* attach demodulator chip */
+   /* attach first demodulator chip */
lgdt3306a_config = hauppauge_955q_lgdt3306a_config;
lgdt3306a_config.fe = >dvb->frontend[0];
lgdt3306a_config.i2c_adapter = 
+   lgdt3306a_config.deny_i2c_rptr = 0;
 
strlcpy(info.type, "lgdt3306a", sizeof(info.type));
info.addr = dev->board.demod_addr;
@@ -1206,10 +1209,43 @@ static int dvb_init(struct cx231xx *dev)
}
 
dvb->i2c_client_demod[0] = client;
-   dev->dvb->frontend[0]->ops.i2c_gate_ctrl = NULL;
+
+   /* attach second demodulator chip */
+   si2168_config.ts_mode = SI2168_TS_SERIAL;
+   si2168_config.fe = >dvb->frontend[1];
+   si2168_config.i2c_adapter = 
+   si2168_config.ts_clock_inv = true;
+
+   memset(, 0, sizeof(struct i2c_board_info));
+   strlcpy(info.type, "si2168", sizeof(info.type));
+   info.addr = dev->board.demod_addr2;
+   info.platform_data = _config;
+
+   request_module(info.type);
+   client = i2c_new_device(adapter, );
+   if (client == NULL || client->dev.driver == NULL) {
+   dev_err(dev->dev,
+   "Failed to attach %s frontend.\n", info.type);
+   module_put(dvb->i2c_client_demod[0]->dev.driver->owner);
+   i2c_unregister_device(dvb->i2c_client_demod[0]);
+   result = -ENODEV;
+   goto out_free;
+   }
+
+   if (!try_module_get(client->dev.driver->owner)) {
+   i2c_unregister_device(client);
+   module_put(dvb->i2c_client_demod[0]->dev.driver->owner);
+   i2c_unregister_device(dvb->i2c_client_demod[0]);
+   result = -ENODEV;
+   goto out_free;
+   }
+
+   dvb->i2c_client_demod[1] = client;
+   dvb->frontend[1]->id = 1;
 
/* define general-purpose callback pointer */
dvb->frontend[0]->callback = cx231xx_tuner_callback;
+   dvb->frontend[1]->callback = cx231xx_tuner_callback;
 
/* attach tuner */
si2157_config.fe = dev->dvb->frontend[0];
@@ -1227,6 +1263,8 @@ static int dvb_init(struct cx231xx *dev)
 
client = i2c_new_device(adapter, );
if (client == NULL || client->dev.driver == NULL) {
+   module_put(dvb->i2c_client_demod[1]->dev.driver->owner);
+   i2c_unregister_device(dvb->i2c_client_demod[1]);
module_put(dvb->i2c_client_demod[0]->dev.driver->owner);
i2c_unregister_device(dvb->i2c_client_demod[0]);
result = -ENODEV;
@@ -1237,6 +1275,8 @@ static int dvb_init(struct cx231xx *dev)
dev_err(dev->dev,
"Failed to obtain %s tuner.\n", info.type);
i2c_unregister_device(client);
+   

[PATCH] media: rc: meson-ir: add timeout on idle

[Matthias Reichl]

Meson doesn't seem to be able to generate timeout events
in hardware. So install a software timer to generate the
timeout events required by the decoders to prevent
"ghost keypresses".

Signed-off-by: Matthias Reichl 
---
 drivers/media/rc/meson-ir.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/media/rc/meson-ir.c b/drivers/media/rc/meson-ir.c
index f2204eb77e2a..f34c5836412b 100644
--- a/drivers/media/rc/meson-ir.c
+++ b/drivers/media/rc/meson-ir.c
@@ -69,6 +69,7 @@ struct meson_ir {
void __iomem*reg;
struct rc_dev   *rc;
spinlock_t  lock;
+   struct timer_list timeout_timer;
 };
 
 static void meson_ir_set_mask(struct meson_ir *ir, unsigned int reg,
@@ -98,6 +99,10 @@ static irqreturn_t meson_ir_irq(int irqno, void *dev_id)
rawir.pulse = !!(status & STATUS_IR_DEC_IN);
 
ir_raw_event_store(ir->rc, );
+
+   mod_timer(>timeout_timer,
+   jiffies + nsecs_to_jiffies(ir->rc->timeout));
+
ir_raw_event_handle(ir->rc);
 
spin_unlock(>lock);
@@ -105,6 +110,17 @@ static irqreturn_t meson_ir_irq(int irqno, void *dev_id)
return IRQ_HANDLED;
 }
 
+static void meson_ir_timeout_timer(struct timer_list *t)
+{
+   struct meson_ir *ir = from_timer(ir, t, timeout_timer);
+   DEFINE_IR_RAW_EVENT(rawir);
+
+   rawir.timeout = true;
+   rawir.duration = ir->rc->timeout;
+   ir_raw_event_store(ir->rc, );
+   ir_raw_event_handle(ir->rc);
+}
+
 static int meson_ir_probe(struct platform_device *pdev)
 {
struct device *dev = >dev;
@@ -145,7 +161,9 @@ static int meson_ir_probe(struct platform_device *pdev)
ir->rc->map_name = map_name ? map_name : RC_MAP_EMPTY;
ir->rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
ir->rc->rx_resolution = US_TO_NS(MESON_TRATE);
+   ir->rc->min_timeout = 1;
ir->rc->timeout = MS_TO_NS(200);
+   ir->rc->max_timeout = 10 * IR_DEFAULT_TIMEOUT;
ir->rc->driver_name = DRIVER_NAME;
 
spin_lock_init(>lock);
@@ -157,6 +175,8 @@ static int meson_ir_probe(struct platform_device *pdev)
return ret;
}
 
+   timer_setup(>timeout_timer, meson_ir_timeout_timer, 0);
+
ret = devm_request_irq(dev, irq, meson_ir_irq, 0, NULL, ir);
if (ret) {
dev_err(dev, "failed to request irq\n");
@@ -198,6 +218,8 @@ static int meson_ir_remove(struct platform_device *pdev)
meson_ir_set_mask(ir, IR_DEC_REG1, REG1_ENABLE, 0);
spin_unlock_irqrestore(>lock, flags);
 
+   del_timer_sync(>timeout_timer);
+
return 0;
 }
 
-- 
2.11.0

[PATCH 1/2] [media] ngene: move the tsin_exchange() stripcopy block into a function

[Daniel Scheller]

From: Daniel Scheller 

Move the copy logic that will skip previously inserted TS NULL frames when
moving data to the DVB ring buffers into an own function. This is done to
not duplicate code all over the place with the following TS offset shift
fixup patch.

While we're touching this part of the code, get rid of the DEBUG_CI_XFER
debug-ifdeffery. This could be toggleable either by a Kconfig or a module
param, but in the end this will accidentally be enabled and cause lots
of kernel log messages, and such devel debug shouldn't be there anyway.

Signed-off-by: Daniel Scheller 
---
Changed since https://www.spinics.net/lists/linux-media/msg129616.html:
* DEBUG_CI_XFER ifdeffery removed

 drivers/media/pci/ngene/ngene-dvb.c | 39 +++--
 1 file changed, 11 insertions(+), 28 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-dvb.c 
b/drivers/media/pci/ngene/ngene-dvb.c
index 65c79f1b36f7..93a07a370cfd 100644
--- a/drivers/media/pci/ngene/ngene-dvb.c
+++ b/drivers/media/pci/ngene/ngene-dvb.c
@@ -139,12 +139,15 @@ static void swap_buffer(u32 *p, u32 len)
 /* start of filler packet */
 static u8 fill_ts[] = { 0x47, 0x1f, 0xff, 0x10, TS_FILLER };
 
-/* #define DEBUG_CI_XFER */
-#ifdef DEBUG_CI_XFER
-static u32 ok;
-static u32 overflow;
-static u32 stripped;
-#endif
+static inline void tsin_copy_stripped(struct ngene *dev, void *buf)
+{
+   if (memcmp(buf, fill_ts, sizeof(fill_ts)) != 0) {
+   if (dvb_ringbuffer_free(>tsin_rbuf) >= 188) {
+   dvb_ringbuffer_write(>tsin_rbuf, buf, 188);
+   wake_up(>tsin_rbuf.queue);
+   }
+   }
+}
 
 void *tsin_exchange(void *priv, void *buf, u32 len, u32 clock, u32 flags)
 {
@@ -157,28 +160,8 @@ void *tsin_exchange(void *priv, void *buf, u32 len, u32 
clock, u32 flags)
 
if (dev->ci.en && chan->number == 2) {
while (len >= 188) {
-   if (memcmp(buf, fill_ts, sizeof fill_ts) != 0) {
-   if (dvb_ringbuffer_free(>tsin_rbuf) >= 
188) {
-   dvb_ringbuffer_write(>tsin_rbuf, 
buf, 188);
-   wake_up(>tsin_rbuf.queue);
-#ifdef DEBUG_CI_XFER
-   ok++;
-#endif
-   }
-#ifdef DEBUG_CI_XFER
-   else
-   overflow++;
-#endif
-   }
-#ifdef DEBUG_CI_XFER
-   else
-   stripped++;
-
-   if (ok % 100 == 0 && overflow)
-   dev_warn(>pci_dev->dev,
-"%s: ok %u overflow %u dropped %u\n",
-__func__, ok, overflow, stripped);
-#endif
+   tsin_copy_stripped(dev, buf);
+
buf += 188;
len -= 188;
}
-- 
2.16.1

[PATCH 2/2] [media] ngene: compensate for TS buffer offset shifts

[Daniel Scheller]

From: Daniel Scheller 

A possible hardware bug was discovered when using CA addon hardware
attached to the ngene hardware, in that the TS input buffer much likely
will shift and thus become unaligned to 188 byte blocks (a full TS frame)
when things like CA module initialisation (which happens via differing
communication paths) take place. This causes the TS NULL removal in
tsin_exchange() to fail to detect this previously inserted data and thus
causes userspace applications to receive data they didn't sent beforehand
and ultimately cause troubles.

On driver load with an inserted CAM, buffers are fine at first (note that
the driver has to keep the communication running from/to the card by
inserting TS NULL frames, this is done in tsout_exchange() via
FillTSBuffer() - that data is simply sent back by the hardware):

  offset | 01   2   3   4   5  188 189 190 191 192 193  376
  data   | 47  1f  ff  10  6f  6f   47  1f  ff  10  6f  6f   47

After a few seconds, the CA module is recognised and initialised, which is
signalled by

  dvb_ca_en50221: dvb_ca adapter X: DVB CAM detected and initialised 
successfully

This is where the first shift happens (this is always four bytes), buffer
becomes like this:

  offset | 01   2   3   4   5  188 189 190 191 192 193  376
  data   | 6f  6f  6f  6f  47  1f   6f  6f  6f  6f  47  1f   6f

Next, VDR, TVHeadend or any other CI aware application is started, buffers
will shift by even more bytes. It is believed this is due to the hardware
not handling control and data bytes properly distinct, and control data
having an influence on the actual data stream, which we cannot properly
detect at the driver level.

Workaround this hardware quirk by adding a detection for the TS sync byte
0x47 before each TS frame copy, scan for a new SYNC byte and a TS NULL
packet if buffers become unaligned, take note of that offset and apply
that when copying data to the DVB ring buffers. The last <188 bytes from
the hardware buffers are stored in a temp buffer (tsin_buffer), for which
the remainder will be in the beginning of the next hardware buffer (next
iteration of tsin_exchange()). That remainder will be appended to the
temp buffer and finally sent to the DVB ring buffer. The resulting TS
stream is perfectly fine, and the TS NULL packets inserted by the driver
which are sent back are properly removed. The resulting offset is being
clamped to 188 byte segments (one TS packet). Though this can result in
a repeated TS packet if the overall offset grows beyond this (and it
will grow only on CA initialisation), this is still way better than
unaligned TS frames and data sent to userspace that just isn't supposed
to be there.

This compensation can be toggled by the ci_tsfix modparam, which defaults
to 1 (enabled). In the case of problems, this can be turned off by setting
the parameter to 0 to restore the old behaviour.

Signed-off-by: Daniel Scheller 
---
 drivers/media/pci/ngene/ngene-dvb.c | 91 -
 drivers/media/pci/ngene/ngene.h |  3 ++
 2 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/drivers/media/pci/ngene/ngene-dvb.c 
b/drivers/media/pci/ngene/ngene-dvb.c
index 93a07a370cfd..fee89b9ed9c1 100644
--- a/drivers/media/pci/ngene/ngene-dvb.c
+++ b/drivers/media/pci/ngene/ngene-dvb.c
@@ -38,6 +38,9 @@
 
 #include "ngene.h"
 
+static int ci_tsfix = 1;
+module_param(ci_tsfix, int, 0444);
+MODULE_PARM_DESC(ci_tsfix, "Detect and fix TS buffer offset shifs in 
conjunction with CI expansions (default: 1/enabled)");
 
 //
 /* COMMAND API interface /
@@ -139,6 +142,24 @@ static void swap_buffer(u32 *p, u32 len)
 /* start of filler packet */
 static u8 fill_ts[] = { 0x47, 0x1f, 0xff, 0x10, TS_FILLER };
 
+static int tsin_find_offset(void *buf, u32 len)
+{
+   int i, l;
+
+   l = len - sizeof(fill_ts);
+   if (l <= 0)
+   return -1;
+
+   for (i = 0; i < l; i++) {
+   if (((char *)buf)[i] == 0x47) {
+   if (!memcmp(buf + i, fill_ts, sizeof(fill_ts)))
+   return i % 188;
+   }
+   }
+
+   return -1;
+}
+
 static inline void tsin_copy_stripped(struct ngene *dev, void *buf)
 {
if (memcmp(buf, fill_ts, sizeof(fill_ts)) != 0) {
@@ -153,18 +174,86 @@ void *tsin_exchange(void *priv, void *buf, u32 len, u32 
clock, u32 flags)
 {
struct ngene_channel *chan = priv;
struct ngene *dev = chan->dev;
-
+   int tsoff;
 
if (flags & DF_SWAP32)
swap_buffer(buf, len);
 
if (dev->ci.en && chan->number == 2) {
+   /* blindly copy buffers if ci_tsfix is disabled */
+   if (!ci_tsfix) {
+   while (len >= 188) {
+   

[PATCH 0/2] ngene CI TS fixup

[Daniel Scheller]

From: Daniel Scheller 

Remainder of the ngene updates series, taking care of the TS buffer offset
shift which occurs in conjunction with CICAM hardware during CAM inits.

Changed as requested to get rid of the DEBUG_CI_XFER ifdeffery.

This might aswell be considered a v3 of [1], but as 10 of 12 patches were
merged already, these two patches can be treated separate.

[1] https://www.spinics.net/lists/linux-media/msg129606.html

Daniel Scheller (2):
  [media] ngene: move the tsin_exchange() stripcopy block into a
function
  [media] ngene: compensate for TS buffer offset shifts

 drivers/media/pci/ngene/ngene-dvb.c | 126 
 drivers/media/pci/ngene/ngene.h |   3 +
 2 files changed, 102 insertions(+), 27 deletions(-)

-- 
2.16.1

Re: [PATCH] w1: fix w1_ds2438 documentation

[Mauro Carvalho Chehab]

Em Fri,  2 Mar 2018 08:55:24 +0100
Mariusz Bialonczyk  escreveu:

> Signed-off-by: Mariusz Bialonczyk 
> ---
>  Documentation/w1/slaves/w1_ds2438 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/w1/slaves/w1_ds2438 
> b/Documentation/w1/slaves/w1_ds2438
> index b99f3674c5b4..e64f65a09387 100644
> --- a/Documentation/w1/slaves/w1_ds2438
> +++ b/Documentation/w1/slaves/w1_ds2438
> @@ -60,4 +60,4 @@ vad: general purpose A/D input (VAD)
>  vdd: battery input (VDD)
>  
>  After the voltage conversion the value is returned as decimal ASCII.
> -Note: The value is in mV, so to get a volts the value has to be divided by 
> 10.
> +Note: To get a volts the value has to be divided by 100.

Hmm... I've no idea why you sent this to linux-media and to me...

The proper maintainer seems to be Evgeniy:

 ./scripts/get_maintainer.pl -f Documentation/w1/slaves/w1_ds2438
Evgeniy Polyakov  (maintainer:W1 DALLAS'S 1-WIRE BUS)
Jonathan Corbet  (maintainer:DOCUMENTATION)
Greg Kroah-Hartman  (commit_signer:1/1=100%)
Mariusz Bialonczyk  
(commit_signer:1/1=100%,authored:1/1=100%,added_lines:63/63=100%)
linux-...@vger.kernel.org (open list:DOCUMENTATION)
linux-ker...@vger.kernel.org (open list)



Thanks,
Mauro

Re: [v2] [media] Use common error handling code in 20 functions

[SF Markus Elfring]

I got the following notification.

 - http://patchwork.linuxtv.org/patch/47270/
 - for: Linux Media kernel patches
was: New
now: Rejected


Would you like to clarify still any other variant for this change proposal?

Regards,
Markus

[PATCH] media: ov5640: fix get_/set_fmt colorspace related fields

[Hugues Fruchet]

Fix set of missing colorspace related fields in get_/set_fmt.
Detected by v4l2-compliance tool.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 03940f0..676f635 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1874,7 +1874,13 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev 
*sd,
if (ov5640_formats[i].code == fmt->code)
break;
if (i >= ARRAY_SIZE(ov5640_formats))
-   fmt->code = ov5640_formats[0].code;
+   i = 0;
+
+   fmt->code = ov5640_formats[i].code;
+   fmt->colorspace = ov5640_formats[i].colorspace;
+   fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
+   fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+   fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
 
return 0;
 }
@@ -1885,6 +1891,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
 {
struct ov5640_dev *sensor = to_ov5640_dev(sd);
const struct ov5640_mode_info *new_mode;
+   struct v4l2_mbus_framefmt *mbus_fmt = >format;
int ret;
 
if (format->pad != 0)
@@ -1897,7 +1904,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
goto out;
}
 
-   ret = ov5640_try_fmt_internal(sd, >format,
+   ret = ov5640_try_fmt_internal(sd, mbus_fmt,
  sensor->current_fr, _mode);
if (ret)
goto out;
@@ -1906,12 +1913,12 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *fmt =
v4l2_subdev_get_try_format(sd, cfg, 0);
 
-   *fmt = format->format;
+   *fmt = *mbus_fmt;
goto out;
}
 
sensor->current_mode = new_mode;
-   sensor->fmt = format->format;
+   sensor->fmt = *mbus_fmt;
sensor->pending_mode_change = true;
 out:
mutex_unlock(>lock);
@@ -2497,16 +2504,22 @@ static int ov5640_probe(struct i2c_client *client,
struct fwnode_handle *endpoint;
struct ov5640_dev *sensor;
int ret;
+   struct v4l2_mbus_framefmt *fmt;
 
sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
if (!sensor)
return -ENOMEM;
 
sensor->i2c_client = client;
-   sensor->fmt.code = MEDIA_BUS_FMT_UYVY8_2X8;
-   sensor->fmt.width = 640;
-   sensor->fmt.height = 480;
-   sensor->fmt.field = V4L2_FIELD_NONE;
+   fmt = >fmt;
+   fmt->code = ov5640_formats[0].code;
+   fmt->colorspace = ov5640_formats[0].colorspace;
+   fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
+   fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+   fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
+   fmt->width = 640;
+   fmt->height = 480;
+   fmt->field = V4L2_FIELD_NONE;
sensor->frame_interval.numerator = 1;
sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS];
sensor->current_fr = OV5640_30_FPS;
-- 
1.9.1

Re: [PATCH v2 01/11] media: tw9910: Re-order variables declaration

[Mauro Carvalho Chehab]

Em Tue, 6 Mar 2018 17:57:15 +0100
jacopo mondi  escreveu:

> Hi Mauro,
> 
> On Tue, Mar 06, 2018 at 01:51:52PM -0300, Mauro Carvalho Chehab wrote:
> > Em Fri,  2 Mar 2018 15:46:33 +0100
> > Jacopo Mondi  escreveu:
> >  
> > > Re-order variables declaration to respect 'reverse christmas tree'
> > > ordering whenever possible.  
> >
> > To be frank, I don't like the idea of reverse christmas tree ordering
> > myself... Perhaps due to the time I used to program on assembler,
> > where alignment issues could happen, I find a way more logic to order
> > based on complexity and size of the argument...
> >  
> > >
> > > Signed-off-by: Jacopo Mondi 
> > > ---
> > >  drivers/media/i2c/tw9910.c | 23 +++
> > >  1 file changed, 11 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
> > > index cc648de..3a5e307 100644
> > > --- a/drivers/media/i2c/tw9910.c
> > > +++ b/drivers/media/i2c/tw9910.c
> > > @@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client)
> > >
> > >  static int tw9910_power(struct i2c_client *client, int enable)
> > >  {
> > > - int ret;
> > >   u8 acntl1;
> > >   u8 acntl2;
> > > + int ret;  
> >
> > ... So, in this case, the order is already the right one, according
> > with my own criteria :-)
> >
> > There was some discussion about the order sometime ago at LKML:
> >
> > https://patchwork.kernel.org/patch/9411999/
> >
> > As I'm not seeing the proposed patch there at checkpatch, nor any
> > comments about xmas tree at coding style, I think that there were no
> > agreements about the ordering.
> >
> > So, while there's no consensus about that, let's keep it as-is.  
> 
> Thanks for explaining. I was sure it was part of the coding style
> rules! My bad, feel free to ditch this patch (same for ov772x ofc).

Heh, there are so many rules that it is hard to get all of them.

Also, some maintainers might actually be expecting some ordering.

I ditched this patch (and the one for ov772x) and applied the
remaining ones.

Thanks,
Mauro

[PATCH] media: ov5640: fix colorspace compliance

[Hugues Fruchet]

Fix format ioctl colorspace related fields.
Detected by v4l2-compliance tool.

Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 03940f0..676f635 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1874,7 +1874,13 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev 
*sd,
if (ov5640_formats[i].code == fmt->code)
break;
if (i >= ARRAY_SIZE(ov5640_formats))
-   fmt->code = ov5640_formats[0].code;
+   i = 0;
+
+   fmt->code = ov5640_formats[i].code;
+   fmt->colorspace = ov5640_formats[i].colorspace;
+   fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
+   fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+   fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
 
return 0;
 }
@@ -1885,6 +1891,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
 {
struct ov5640_dev *sensor = to_ov5640_dev(sd);
const struct ov5640_mode_info *new_mode;
+   struct v4l2_mbus_framefmt *mbus_fmt = >format;
int ret;
 
if (format->pad != 0)
@@ -1897,7 +1904,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
goto out;
}
 
-   ret = ov5640_try_fmt_internal(sd, >format,
+   ret = ov5640_try_fmt_internal(sd, mbus_fmt,
  sensor->current_fr, _mode);
if (ret)
goto out;
@@ -1906,12 +1913,12 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *fmt =
v4l2_subdev_get_try_format(sd, cfg, 0);
 
-   *fmt = format->format;
+   *fmt = *mbus_fmt;
goto out;
}
 
sensor->current_mode = new_mode;
-   sensor->fmt = format->format;
+   sensor->fmt = *mbus_fmt;
sensor->pending_mode_change = true;
 out:
mutex_unlock(>lock);
@@ -2497,16 +2504,22 @@ static int ov5640_probe(struct i2c_client *client,
struct fwnode_handle *endpoint;
struct ov5640_dev *sensor;
int ret;
+   struct v4l2_mbus_framefmt *fmt;
 
sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
if (!sensor)
return -ENOMEM;
 
sensor->i2c_client = client;
-   sensor->fmt.code = MEDIA_BUS_FMT_UYVY8_2X8;
-   sensor->fmt.width = 640;
-   sensor->fmt.height = 480;
-   sensor->fmt.field = V4L2_FIELD_NONE;
+   fmt = >fmt;
+   fmt->code = ov5640_formats[0].code;
+   fmt->colorspace = ov5640_formats[0].colorspace;
+   fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
+   fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+   fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
+   fmt->width = 640;
+   fmt->height = 480;
+   fmt->field = V4L2_FIELD_NONE;
sensor->frame_interval.numerator = 1;
sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS];
sensor->current_fr = OV5640_30_FPS;
-- 
1.9.1

Re: [PATCH v2 01/11] media: tw9910: Re-order variables declaration

[jacopo mondi]

Hi Mauro,

On Tue, Mar 06, 2018 at 01:51:52PM -0300, Mauro Carvalho Chehab wrote:
> Em Fri,  2 Mar 2018 15:46:33 +0100
> Jacopo Mondi  escreveu:
>
> > Re-order variables declaration to respect 'reverse christmas tree'
> > ordering whenever possible.
>
> To be frank, I don't like the idea of reverse christmas tree ordering
> myself... Perhaps due to the time I used to program on assembler,
> where alignment issues could happen, I find a way more logic to order
> based on complexity and size of the argument...
>
> >
> > Signed-off-by: Jacopo Mondi 
> > ---
> >  drivers/media/i2c/tw9910.c | 23 +++
> >  1 file changed, 11 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
> > index cc648de..3a5e307 100644
> > --- a/drivers/media/i2c/tw9910.c
> > +++ b/drivers/media/i2c/tw9910.c
> > @@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client)
> >
> >  static int tw9910_power(struct i2c_client *client, int enable)
> >  {
> > -   int ret;
> > u8 acntl1;
> > u8 acntl2;
> > +   int ret;
>
> ... So, in this case, the order is already the right one, according
> with my own criteria :-)
>
> There was some discussion about the order sometime ago at LKML:
>
>   https://patchwork.kernel.org/patch/9411999/
>
> As I'm not seeing the proposed patch there at checkpatch, nor any
> comments about xmas tree at coding style, I think that there were no
> agreements about the ordering.
>
> So, while there's no consensus about that, let's keep it as-is.

Thanks for explaining. I was sure it was part of the coding style
rules! My bad, feel free to ditch this patch (same for ov772x ofc).

Thanks
   j

>
> Regards,
> Mauro

[PATCH] media: ov5640: fix colorspace compliance

[Hugues Fruchet]

Fix format ioctl colorspace related fields.
Detected by v4l2-compliance tool.

Change-Id: I645138297033bc409751a3c7fc63e014650b8417
Signed-off-by: Hugues Fruchet 
---
 drivers/media/i2c/ov5640.c | 29 +
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 03940f0..676f635 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1874,7 +1874,13 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev 
*sd,
if (ov5640_formats[i].code == fmt->code)
break;
if (i >= ARRAY_SIZE(ov5640_formats))
-   fmt->code = ov5640_formats[0].code;
+   i = 0;
+
+   fmt->code = ov5640_formats[i].code;
+   fmt->colorspace = ov5640_formats[i].colorspace;
+   fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
+   fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+   fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
 
return 0;
 }
@@ -1885,6 +1891,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
 {
struct ov5640_dev *sensor = to_ov5640_dev(sd);
const struct ov5640_mode_info *new_mode;
+   struct v4l2_mbus_framefmt *mbus_fmt = >format;
int ret;
 
if (format->pad != 0)
@@ -1897,7 +1904,7 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
goto out;
}
 
-   ret = ov5640_try_fmt_internal(sd, >format,
+   ret = ov5640_try_fmt_internal(sd, mbus_fmt,
  sensor->current_fr, _mode);
if (ret)
goto out;
@@ -1906,12 +1913,12 @@ static int ov5640_set_fmt(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *fmt =
v4l2_subdev_get_try_format(sd, cfg, 0);
 
-   *fmt = format->format;
+   *fmt = *mbus_fmt;
goto out;
}
 
sensor->current_mode = new_mode;
-   sensor->fmt = format->format;
+   sensor->fmt = *mbus_fmt;
sensor->pending_mode_change = true;
 out:
mutex_unlock(>lock);
@@ -2497,16 +2504,22 @@ static int ov5640_probe(struct i2c_client *client,
struct fwnode_handle *endpoint;
struct ov5640_dev *sensor;
int ret;
+   struct v4l2_mbus_framefmt *fmt;
 
sensor = devm_kzalloc(dev, sizeof(*sensor), GFP_KERNEL);
if (!sensor)
return -ENOMEM;
 
sensor->i2c_client = client;
-   sensor->fmt.code = MEDIA_BUS_FMT_UYVY8_2X8;
-   sensor->fmt.width = 640;
-   sensor->fmt.height = 480;
-   sensor->fmt.field = V4L2_FIELD_NONE;
+   fmt = >fmt;
+   fmt->code = ov5640_formats[0].code;
+   fmt->colorspace = ov5640_formats[0].colorspace;
+   fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
+   fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
+   fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
+   fmt->width = 640;
+   fmt->height = 480;
+   fmt->field = V4L2_FIELD_NONE;
sensor->frame_interval.numerator = 1;
sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS];
sensor->current_fr = OV5640_30_FPS;
-- 
1.9.1

Re: [PATCH v2 01/11] media: tw9910: Re-order variables declaration

[Mauro Carvalho Chehab]

Em Fri,  2 Mar 2018 15:46:33 +0100
Jacopo Mondi  escreveu:

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

To be frank, I don't like the idea of reverse christmas tree ordering
myself... Perhaps due to the time I used to program on assembler, 
where alignment issues could happen, I find a way more logic to order
based on complexity and size of the argument...

> 
> Signed-off-by: Jacopo Mondi 
> ---
>  drivers/media/i2c/tw9910.c | 23 +++
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
> index cc648de..3a5e307 100644
> --- a/drivers/media/i2c/tw9910.c
> +++ b/drivers/media/i2c/tw9910.c
> @@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client)
>  
>  static int tw9910_power(struct i2c_client *client, int enable)
>  {
> - int ret;
>   u8 acntl1;
>   u8 acntl2;
> + int ret;

... So, in this case, the order is already the right one, according
with my own criteria :-)

There was some discussion about the order sometime ago at LKML:

https://patchwork.kernel.org/patch/9411999/

As I'm not seeing the proposed patch there at checkpatch, nor any
comments about xmas tree at coding style, I think that there were no
agreements about the ordering.

So, while there's no consensus about that, let's keep it as-is.

Regards,
Mauro

[PATCH 2/4] [media] ngene: add I2C_FUNC_I2C to the I2C interface functionality

[Daniel Scheller]

From: Daniel Scheller 

Report I2C_FUNC_I2C in .functionality() aswell. The I2C interface can
handle this fine and even is required for all I2C client drivers that
utilise the regmap API which are used from within the ngene driver.

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

diff --git a/drivers/media/pci/ngene/ngene-i2c.c 
b/drivers/media/pci/ngene/ngene-i2c.c
index 3004947f300b..092d46c2a3a9 100644
--- a/drivers/media/pci/ngene/ngene-i2c.c
+++ b/drivers/media/pci/ngene/ngene-i2c.c
@@ -147,7 +147,7 @@ static int ngene_i2c_master_xfer(struct i2c_adapter 
*adapter,
 
 static u32 ngene_i2c_functionality(struct i2c_adapter *adap)
 {
-   return I2C_FUNC_SMBUS_EMUL;
+   return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
 }
 
 static const struct i2c_algorithm ngene_i2c_algo = {
-- 
2.16.1

[PATCH 3/4] [media] dvb-frontends/cxd2099: remove remainders from old attach way

[Daniel Scheller]

From: Daniel Scheller 

As all drivers using the cxd2099 are converted to handle attach/detach
the generic I2C client way, the static inline cxd2099_attach isn't
required anymore. Thus cleanup cxd2099.h from the remainders, the adr
struct member isn't used anymore aswell.

Signed-off-by: Daniel Scheller 
Signed-off-by: Jasmin Jessich 
---
 drivers/media/dvb-frontends/cxd2099.h | 10 --
 1 file changed, 10 deletions(-)

diff --git a/drivers/media/dvb-frontends/cxd2099.h 
b/drivers/media/dvb-frontends/cxd2099.h
index 679e87512799..8fa45a4c615a 100644
--- a/drivers/media/dvb-frontends/cxd2099.h
+++ b/drivers/media/dvb-frontends/cxd2099.h
@@ -20,7 +20,6 @@
 
 struct cxd2099_cfg {
u32 bitrate;
-   u8  adr;
u8  polarity;
u8  clock_mode;
 
@@ -30,13 +29,4 @@ struct cxd2099_cfg {
struct dvb_ca_en50221 **en;
 };
 
-/* TODO: remove when done */
-static inline struct
-dvb_ca_en50221 *cxd2099_attach(struct cxd2099_cfg *cfg, void *priv,
-  struct i2c_adapter *i2c)
-{
-   dev_warn(>dev, "%s: driver disabled by Kconfig\n", __func__);
-   return NULL;
-}
-
 #endif
-- 
2.16.1

[PATCH 1/4] [media] ddbridge: adapt cxd2099 attach to new i2c_client way

[Daniel Scheller]

From: Daniel Scheller 

Change the way the cxd2099 hardware is being attached to the new I2C
client interface way.

Signed-off-by: Daniel Scheller 
Signed-off-by: Jasmin Jessich 
---
 drivers/media/pci/ddbridge/ddbridge-ci.c | 62 +---
 drivers/media/pci/ddbridge/ddbridge.h|  1 +
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-ci.c 
b/drivers/media/pci/ddbridge/ddbridge-ci.c
index ed19890710d6..6585ef54ac22 100644
--- a/drivers/media/pci/ddbridge/ddbridge-ci.c
+++ b/drivers/media/pci/ddbridge/ddbridge-ci.c
@@ -172,6 +172,7 @@ static void ci_attach(struct ddb_port *port)
memcpy(>en, _templ, sizeof(en_templ));
ci->en.data = ci;
port->en = >en;
+   port->en_freedata = 1;
ci->port = port;
ci->nr = port->nr - 2;
 }
@@ -304,6 +305,7 @@ static void ci_xo2_attach(struct ddb_port *port)
memcpy(>en, _xo2_templ, sizeof(en_xo2_templ));
ci->en.data = ci;
port->en = >en;
+   port->en_freedata = 1;
ci->port = port;
ci->nr = port->nr - 2;
ci->port->creg = 0;
@@ -311,20 +313,58 @@ static void ci_xo2_attach(struct ddb_port *port)
write_creg(ci, 0x08, 0x08);
 }
 
-static struct cxd2099_cfg cxd_cfg = {
+static const struct cxd2099_cfg cxd_cfgtmpl = {
.bitrate =  72000,
-   .adr =  0x40,
.polarity = 1,
.clock_mode = 1,
.max_i2c = 512,
 };
 
+static int ci_cxd2099_attach(struct ddb_port *port, u32 bitrate)
+{
+   struct cxd2099_cfg cxd_cfg = cxd_cfgtmpl;
+   struct i2c_client *client;
+   struct i2c_board_info board_info = {
+   .type = "cxd2099",
+   .addr = 0x40,
+   .platform_data = _cfg,
+   };
+
+   cxd_cfg.bitrate = bitrate;
+   cxd_cfg.en = >en;
+
+   request_module(board_info.type);
+
+   client = i2c_new_device(>i2c->adap, _info);
+   if (!client || !client->dev.driver)
+   goto err_ret;
+
+   if (!try_module_get(client->dev.driver->owner))
+   goto err_i2c;
+
+   if (!port->en)
+   goto err_i2c;
+
+   port->dvb[0].i2c_client[0] = client;
+   port->en_freedata = 0;
+   return 0;
+
+err_i2c:
+   i2c_unregister_device(client);
+err_ret:
+   dev_err(port->dev->dev, "CXD2099AR attach failed\n");
+   return -ENODEV;
+}
+
 int ddb_ci_attach(struct ddb_port *port, u32 bitrate)
 {
+   int ret;
+
switch (port->type) {
case DDB_CI_EXTERNAL_SONY:
-   cxd_cfg.bitrate = bitrate;
-   port->en = cxd2099_attach(_cfg, port, >i2c->adap);
+   ret = ci_cxd2099_attach(port, bitrate);
+   if (ret)
+   return -ENODEV;
break;
case DDB_CI_EXTERNAL_XO2:
case DDB_CI_EXTERNAL_XO2_B:
@@ -345,11 +385,23 @@ int ddb_ci_attach(struct ddb_port *port, u32 bitrate)
 
 void ddb_ci_detach(struct ddb_port *port)
 {
+   struct i2c_client *client;
+
if (port->dvb[0].dev)
dvb_unregister_device(port->dvb[0].dev);
if (port->en) {
dvb_ca_en50221_release(port->en);
-   kfree(port->en->data);
+
+   client = port->dvb[0].i2c_client[0];
+   if (client) {
+   module_put(client->dev.driver->owner);
+   i2c_unregister_device(client);
+   }
+
+   /* free alloc'ed memory if needed */
+   if (port->en_freedata)
+   kfree(port->en->data);
+
port->en = NULL;
}
 }
diff --git a/drivers/media/pci/ddbridge/ddbridge.h 
b/drivers/media/pci/ddbridge/ddbridge.h
index 095457737bc1..f223dc6c9963 100644
--- a/drivers/media/pci/ddbridge/ddbridge.h
+++ b/drivers/media/pci/ddbridge/ddbridge.h
@@ -276,6 +276,7 @@ struct ddb_port {
struct ddb_input  *input[2];
struct ddb_output *output;
struct dvb_ca_en50221 *en;
+   u8 en_freedata;
struct ddb_dvb dvb[2];
u32gap;
u32obr;
-- 
2.16.1

[PATCH 4/4] [media] MAINTAINERS: add entry for cxd2099

[Daniel Scheller]

From: Jasmin Jessich 

The cxd2099 driver is now maintained and being taken care of by

  * Jasmin Jessich 

Signed-off-by: Jasmin Jessich 
---
 MAINTAINERS | 8 
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0eea2f0e9456..298f0b84a4ed 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8612,6 +8612,14 @@ T:   git git://linuxtv.org/media_tree.git
 S: Supported
 F: drivers/media/dvb-frontends/ascot2e*
 
+MEDIA DRIVERS FOR CXD2099AR CI CONTROLLERS
+M: Jasmin Jessich 
+L: linux-media@vger.kernel.org
+W: https://linuxtv.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/dvb-frontends/cxd2099*
+
 MEDIA DRIVERS FOR CXD2841ER
 M: Sergey Kozlov 
 M: Abylay Ospan 
-- 
2.16.1

[PATCH 0/4] cxd2099 series fixup

[Daniel Scheller]

From: Daniel Scheller 

These are the missing bits from the cxd2099 that went MIA during the
merge of the series.

Please pull - ngene driver requires the ngene patch for all I2C clients
to work (including the tda18212 that's now used since the merge of the
other ngene series) and ddbridge can't make use of the cxd2099 driver
at all right now.

Daniel Scheller (3):
  [media] ddbridge: adapt cxd2099 attach to new i2c_client way
  [media] ngene: add I2C_FUNC_I2C to the I2C interface functionality
  [media] dvb-frontends/cxd2099: remove remainders from old attach way

Jasmin Jessich (1):
  [media] MAINTAINERS: add entry for cxd2099

 MAINTAINERS  |  8 +
 drivers/media/dvb-frontends/cxd2099.h| 10 --
 drivers/media/pci/ddbridge/ddbridge-ci.c | 62 +---
 drivers/media/pci/ddbridge/ddbridge.h|  1 +
 drivers/media/pci/ngene/ngene-i2c.c  |  2 +-
 5 files changed, 67 insertions(+), 16 deletions(-)

-- 
2.16.1

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

[Mauro Carvalho Chehab]

Em Tue, 06 Mar 2018 18:35:32 +0200
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> On Tuesday, 6 March 2018 18:25:15 EET Mauro Carvalho Chehab wrote:
> > Em Mon, 26 Feb 2018 19:09:10 +0100 Geert Uytterhoeven escreveu:  
> > > VIDEO_RENESAS_VSP1 depends on ARCH_RENESAS && OF.
> > > As ARCH_RENESAS implies OF, the latter can be dropped.
> > > 
> > > Signed-off-by: Geert Uytterhoeven 
> > > ---
> > > 
> > >  drivers/media/platform/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/media/platform/Kconfig
> > > b/drivers/media/platform/Kconfig index 614fbef08ddcabb0..2b8b1ad0edd9eb31
> > > 100644
> > > --- a/drivers/media/platform/Kconfig
> > > +++ b/drivers/media/platform/Kconfig
> > > @@ -448,7 +448,7 @@ config VIDEO_RENESAS_FCP
> > > 
> > >  config VIDEO_RENESAS_VSP1
> > >  
> > >   tristate "Renesas VSP1 Video Processing Engine"
> > >   depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && HAS_DMA
> > > 
> > > - depends on (ARCH_RENESAS && OF) || COMPILE_TEST
> > > + depends on ARCH_RENESAS || COMPILE_TEST  
> > 
> > That is not correct!
> > 
> > COMPILE_TEST doesn't depend on OF. With this patch, it will likely
> > cause build failures with randconfigs.  
> 
> ARCH_RENESAS implies OF, so replacing (ARCH_RENESAS && OF) with ARCH_RENESAS 
> doesn't change anything. The driver can be compiled with COMPILE_TEST and !OF 
> both before and after this patch.

OK!
> 
> > >   depends on (!ARM64 && !VIDEO_RENESAS_FCP) || VIDEO_RENESAS_FCP
> > >   select VIDEOBUF2_DMA_CONTIG
> > >   select VIDEOBUF2_VMALLOC  
> 



Thanks,
Mauro

Re: [PATCH] media: ov772x: constify ov772x_frame_intervals

[Mauro Carvalho Chehab]

Em Tue, 6 Mar 2018 17:05:26 +0100
jacopo mondi  escreveu:

> Hi Mauro,
> 
> On Tue, Mar 06, 2018 at 10:35:22AM -0500, Mauro Carvalho Chehab wrote:
> > The values on this array never changes. Make it const.
> >
> > Signed-off-by: Mauro Carvalho Chehab   
> 
> Acked-by: Jacopo Mondi 
> 
> Since I'm sure there will be more cleanup/fixes on tw9910 and ov772x,
> could you please take into account my series:
> [PATCH v2 00/11] media: ov772x/tw9910 cleanup
> before any additional change to these 2 drivers?

That is the next on my patch queue :-)

Reviewing them right now.

Regards,
Mauro

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

[Laurent Pinchart]

Hi Mauro,

On Tuesday, 6 March 2018 18:25:15 EET Mauro Carvalho Chehab wrote:
> Em Mon, 26 Feb 2018 19:09:10 +0100 Geert Uytterhoeven escreveu:
> > VIDEO_RENESAS_VSP1 depends on ARCH_RENESAS && OF.
> > As ARCH_RENESAS implies OF, the latter can be dropped.
> > 
> > Signed-off-by: Geert Uytterhoeven 
> > ---
> > 
> >  drivers/media/platform/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/Kconfig
> > b/drivers/media/platform/Kconfig index 614fbef08ddcabb0..2b8b1ad0edd9eb31
> > 100644
> > --- a/drivers/media/platform/Kconfig
> > +++ b/drivers/media/platform/Kconfig
> > @@ -448,7 +448,7 @@ config VIDEO_RENESAS_FCP
> > 
> >  config VIDEO_RENESAS_VSP1
> >  
> > tristate "Renesas VSP1 Video Processing Engine"
> > depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && HAS_DMA
> > 
> > -   depends on (ARCH_RENESAS && OF) || COMPILE_TEST
> > +   depends on ARCH_RENESAS || COMPILE_TEST
> 
> That is not correct!
> 
> COMPILE_TEST doesn't depend on OF. With this patch, it will likely
> cause build failures with randconfigs.

ARCH_RENESAS implies OF, so replacing (ARCH_RENESAS && OF) with ARCH_RENESAS 
doesn't change anything. The driver can be compiled with COMPILE_TEST and !OF 
both before and after this patch.

> > depends on (!ARM64 && !VIDEO_RENESAS_FCP) || VIDEO_RENESAS_FCP
> > select VIDEOBUF2_DMA_CONTIG
> > select VIDEOBUF2_VMALLOC

-- 
Regards,

Laurent Pinchart

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

[Mauro Carvalho Chehab]

Em Mon, 26 Feb 2018 19:09:10 +0100
Geert Uytterhoeven  escreveu:

> VIDEO_RENESAS_VSP1 depends on ARCH_RENESAS && OF.
> As ARCH_RENESAS implies OF, the latter can be dropped.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/media/platform/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index 614fbef08ddcabb0..2b8b1ad0edd9eb31 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -448,7 +448,7 @@ config VIDEO_RENESAS_FCP
>  config VIDEO_RENESAS_VSP1
>   tristate "Renesas VSP1 Video Processing Engine"
>   depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && HAS_DMA
> - depends on (ARCH_RENESAS && OF) || COMPILE_TEST
> + depends on ARCH_RENESAS || COMPILE_TEST

That is not correct!

COMPILE_TEST doesn't depend on OF. With this patch, it will likely
cause build failures with randconfigs.

>   depends on (!ARM64 && !VIDEO_RENESAS_FCP) || VIDEO_RENESAS_FCP
>   select VIDEOBUF2_DMA_CONTIG
>   select VIDEOBUF2_VMALLOC



Thanks,
Mauro

Re: [PATCH v2 11/12] [media] ngene: move the tsin_exchange() stripcopy block into a function

[Mauro Carvalho Chehab]

Em Sun, 25 Feb 2018 13:31:39 +0100
Daniel Scheller  escreveu:

> From: Daniel Scheller 
> 
> Move the copy logic that will skip previously inserted TS NULL frames when
> moving data to the DVB ring buffers into an own function. This is done to
> not duplicate code all over the place with the following TS offset shift
> fixup patch.
> 
> Signed-off-by: Daniel Scheller 
> ---
>  drivers/media/pci/ngene/ngene-dvb.c | 48 
> +
>  1 file changed, 27 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/media/pci/ngene/ngene-dvb.c 
> b/drivers/media/pci/ngene/ngene-dvb.c
> index f71fd41c762c..6d72b9f69418 100644
> --- a/drivers/media/pci/ngene/ngene-dvb.c
> +++ b/drivers/media/pci/ngene/ngene-dvb.c
> @@ -123,6 +123,32 @@ static u32 overflow;
>  static u32 stripped;
>  #endif
>  
> +static inline void tsin_copy_stripped(struct ngene *dev, void *buf)
> +{
> + if (memcmp(buf, fill_ts, sizeof(fill_ts)) != 0) {
> + if (dvb_ringbuffer_free(>tsin_rbuf) >= 188) {
> + dvb_ringbuffer_write(>tsin_rbuf, buf, 188);
> + wake_up(>tsin_rbuf.queue);
> +#ifdef DEBUG_CI_XFER
> + ok++;
> +#endif
> + }
> +#ifdef DEBUG_CI_XFER
> + else
> + overflow++;
> +#endif
> + }
> +#ifdef DEBUG_CI_XFER
> + else
> + stripped++;
> +
> + if (ok % 100 == 0 && overflow)
> + dev_warn(>pci_dev->dev,
> +  "%s: ok %u overflow %u dropped %u\n",
> +  __func__, ok, overflow, stripped);
> +#endif

Those blocks are ugly. Also, there's no Kconfig way to enable those
debug messages.

If they're meant only for your own consumption, just keep it
outside of upstream.

Otherwise, please change the logic in a way that it would
make the code harder to read/enable.

> +}
> +
>  void *tsin_exchange(void *priv, void *buf, u32 len, u32 clock, u32 flags)
>  {
>   struct ngene_channel *chan = priv;
> @@ -134,28 +160,8 @@ void *tsin_exchange(void *priv, void *buf, u32 len, u32 
> clock, u32 flags)
>  
>   if (dev->ci.en && chan->number == 2) {
>   while (len >= 188) {
> - if (memcmp(buf, fill_ts, sizeof fill_ts) != 0) {
> - if (dvb_ringbuffer_free(>tsin_rbuf) >= 
> 188) {
> - dvb_ringbuffer_write(>tsin_rbuf, 
> buf, 188);
> - wake_up(>tsin_rbuf.queue);
> -#ifdef DEBUG_CI_XFER
> - ok++;
> -#endif
> - }
> -#ifdef DEBUG_CI_XFER
> - else
> - overflow++;
> -#endif
> - }
> -#ifdef DEBUG_CI_XFER
> - else
> - stripped++;
> + tsin_copy_stripped(dev, buf);
>  
> - if (ok % 100 == 0 && overflow)
> - dev_warn(>pci_dev->dev,
> -  "%s: ok %u overflow %u dropped %u\n",
> -  __func__, ok, overflow, stripped);
> -#endif
>   buf += 188;
>   len -= 188;
>   }



Thanks,
Mauro

Re: [PATCH v2 08/12] [media] ngene: deduplicate I2C adapter evaluation

[Mauro Carvalho Chehab]

Em Tue, 6 Mar 2018 13:06:15 -0300
Mauro Carvalho Chehab  escreveu:

> Em Sun, 25 Feb 2018 13:31:36 +0100
> Daniel Scheller  escreveu:
> 
> > From: Daniel Scheller 
> > 
> > The I2C adapter evaluation (based on chan->number) is duplicated at
> > several places (tuner_attach_() functions, demod_attach_stv0900() and
> > cineS2_probe()). Clean this up by wrapping that construct in a separate
> > function which all users of that can pass the ngene_channel pointer and
> > get the correct I2C adapter from.
> 
> This patch doesn't apply. 

Forget that. Patch 7 was missing. After applying it, this one
merged OK.

> 
> Please rebase from this point at the top of the media tree.
> 
> PS.: Perhaps I ended by merging an older version of some of your
> patches, as I noticed that some e-mails got lost either by me or
> by patch work along those days.


> 
> > 
> > Signed-off-by: Daniel Scheller 
> > ---
> >  drivers/media/pci/ngene/ngene-cards.c | 41 
> > +--
> >  1 file changed, 15 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/media/pci/ngene/ngene-cards.c 
> > b/drivers/media/pci/ngene/ngene-cards.c
> > index 00b100660784..dff55c7c9f86 100644
> > --- a/drivers/media/pci/ngene/ngene-cards.c
> > +++ b/drivers/media/pci/ngene/ngene-cards.c
> > @@ -118,17 +118,25 @@ static int i2c_read_reg(struct i2c_adapter *adapter, 
> > u8 adr, u8 reg, u8 *val)
> >  /* Demod/tuner attachment 
> > ***/
> >  
> > //
> >  
> > +static struct i2c_adapter *i2c_adapter_from_chan(struct ngene_channel 
> > *chan)
> > +{
> > +   /* tuner 1+2: i2c adapter #0, tuner 3+4: i2c adapter #1 */
> > +   if (chan->number < 2)
> > +   return >dev->channel[0].i2c_adapter;
> > +
> > +   return >dev->channel[1].i2c_adapter;
> > +}
> > +
> >  static int tuner_attach_stv6110(struct ngene_channel *chan)
> >  {
> > struct device *pdev = >dev->pci_dev->dev;
> > -   struct i2c_adapter *i2c;
> > +   struct i2c_adapter *i2c = i2c_adapter_from_chan(chan);
> > struct stv090x_config *feconf = (struct stv090x_config *)
> > chan->dev->card_info->fe_config[chan->number];
> > struct stv6110x_config *tunerconf = (struct stv6110x_config *)
> > chan->dev->card_info->tuner_config[chan->number];
> > const struct stv6110x_devctl *ctl;
> >  
> > -   /* tuner 1+2: i2c adapter #0, tuner 3+4: i2c adapter #1 */
> > if (chan->number < 2)
> > i2c = >dev->channel[0].i2c_adapter;
> > else
> > @@ -158,16 +166,10 @@ static int tuner_attach_stv6110(struct ngene_channel 
> > *chan)
> >  static int tuner_attach_stv6111(struct ngene_channel *chan)
> >  {
> > struct device *pdev = >dev->pci_dev->dev;
> > -   struct i2c_adapter *i2c;
> > +   struct i2c_adapter *i2c = i2c_adapter_from_chan(chan);
> > struct dvb_frontend *fe;
> > u8 adr = 4 + ((chan->number & 1) ? 0x63 : 0x60);
> >  
> > -   /* tuner 1+2: i2c adapter #0, tuner 3+4: i2c adapter #1 */
> > -   if (chan->number < 2)
> > -   i2c = >dev->channel[0].i2c_adapter;
> > -   else
> > -   i2c = >dev->channel[1].i2c_adapter;
> > -
> > fe = dvb_attach(stv6111_attach, chan->fe, i2c, adr);
> > if (!fe) {
> > fe = dvb_attach(stv6111_attach, chan->fe, i2c, adr & ~4);
> > @@ -197,10 +199,9 @@ static int drxk_gate_ctrl(struct dvb_frontend *fe, int 
> > enable)
> >  static int tuner_attach_tda18271(struct ngene_channel *chan)
> >  {
> > struct device *pdev = >dev->pci_dev->dev;
> > -   struct i2c_adapter *i2c;
> > +   struct i2c_adapter *i2c = i2c_adapter_from_chan(chan);
> > struct dvb_frontend *fe;
> >  
> > -   i2c = >dev->channel[0].i2c_adapter;
> > if (chan->fe->ops.i2c_gate_ctrl)
> > chan->fe->ops.i2c_gate_ctrl(chan->fe, 1);
> > fe = dvb_attach(tda18271c2dd_attach, chan->fe, i2c, 0x60);
> > @@ -240,7 +241,7 @@ static int tuner_tda18212_ping(struct ngene_channel 
> > *chan,
> >  static int tuner_attach_tda18212(struct ngene_channel *chan, u32 dmdtype)
> >  {
> > struct device *pdev = >dev->pci_dev->dev;
> > -   struct i2c_adapter *i2c;
> > +   struct i2c_adapter *i2c = i2c_adapter_from_chan(chan);
> > struct i2c_client *client;
> > struct tda18212_config config = {
> > .fe = chan->fe,
> > @@ -262,12 +263,6 @@ static int tuner_attach_tda18212(struct ngene_channel 
> > *chan, u32 dmdtype)
> > else
> > board_info.addr = 0x60;
> >  
> > -   /* tuner 1+2: i2c adapter #0, tuner 3+4: i2c adapter #1 */
> > -   if (chan->number < 2)
> > -   i2c = >dev->channel[0].i2c_adapter;
> > -   else
> > -   i2c = >dev->channel[1].i2c_adapter;
> > -
> > /*
> >  * due to a hardware quirk with the I2C gate on the stv0367+tda18212
> >  * combo, the tda18212 must be probed by reading it's id _twice_ when
> > @@ 

Re: [PATCH v2 08/12] [media] ngene: deduplicate I2C adapter evaluation

[Mauro Carvalho Chehab]

Em Sun, 25 Feb 2018 13:31:36 +0100
Daniel Scheller  escreveu:

> From: Daniel Scheller 
> 
> The I2C adapter evaluation (based on chan->number) is duplicated at
> several places (tuner_attach_() functions, demod_attach_stv0900() and
> cineS2_probe()). Clean this up by wrapping that construct in a separate
> function which all users of that can pass the ngene_channel pointer and
> get the correct I2C adapter from.

This patch doesn't apply. 

Please rebase from this point at the top of the media tree.

PS.: Perhaps I ended by merging an older version of some of your
patches, as I noticed that some e-mails got lost either by me or
by patch work along those days.

> 
> Signed-off-by: Daniel Scheller 
> ---
>  drivers/media/pci/ngene/ngene-cards.c | 41 
> +--
>  1 file changed, 15 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/media/pci/ngene/ngene-cards.c 
> b/drivers/media/pci/ngene/ngene-cards.c
> index 00b100660784..dff55c7c9f86 100644
> --- a/drivers/media/pci/ngene/ngene-cards.c
> +++ b/drivers/media/pci/ngene/ngene-cards.c
> @@ -118,17 +118,25 @@ static int i2c_read_reg(struct i2c_adapter *adapter, u8 
> adr, u8 reg, u8 *val)
>  /* Demod/tuner attachment 
> ***/
>  
> //
>  
> +static struct i2c_adapter *i2c_adapter_from_chan(struct ngene_channel *chan)
> +{
> + /* tuner 1+2: i2c adapter #0, tuner 3+4: i2c adapter #1 */
> + if (chan->number < 2)
> + return >dev->channel[0].i2c_adapter;
> +
> + return >dev->channel[1].i2c_adapter;
> +}
> +
>  static int tuner_attach_stv6110(struct ngene_channel *chan)
>  {
>   struct device *pdev = >dev->pci_dev->dev;
> - struct i2c_adapter *i2c;
> + struct i2c_adapter *i2c = i2c_adapter_from_chan(chan);
>   struct stv090x_config *feconf = (struct stv090x_config *)
>   chan->dev->card_info->fe_config[chan->number];
>   struct stv6110x_config *tunerconf = (struct stv6110x_config *)
>   chan->dev->card_info->tuner_config[chan->number];
>   const struct stv6110x_devctl *ctl;
>  
> - /* tuner 1+2: i2c adapter #0, tuner 3+4: i2c adapter #1 */
>   if (chan->number < 2)
>   i2c = >dev->channel[0].i2c_adapter;
>   else
> @@ -158,16 +166,10 @@ static int tuner_attach_stv6110(struct ngene_channel 
> *chan)
>  static int tuner_attach_stv6111(struct ngene_channel *chan)
>  {
>   struct device *pdev = >dev->pci_dev->dev;
> - struct i2c_adapter *i2c;
> + struct i2c_adapter *i2c = i2c_adapter_from_chan(chan);
>   struct dvb_frontend *fe;
>   u8 adr = 4 + ((chan->number & 1) ? 0x63 : 0x60);
>  
> - /* tuner 1+2: i2c adapter #0, tuner 3+4: i2c adapter #1 */
> - if (chan->number < 2)
> - i2c = >dev->channel[0].i2c_adapter;
> - else
> - i2c = >dev->channel[1].i2c_adapter;
> -
>   fe = dvb_attach(stv6111_attach, chan->fe, i2c, adr);
>   if (!fe) {
>   fe = dvb_attach(stv6111_attach, chan->fe, i2c, adr & ~4);
> @@ -197,10 +199,9 @@ static int drxk_gate_ctrl(struct dvb_frontend *fe, int 
> enable)
>  static int tuner_attach_tda18271(struct ngene_channel *chan)
>  {
>   struct device *pdev = >dev->pci_dev->dev;
> - struct i2c_adapter *i2c;
> + struct i2c_adapter *i2c = i2c_adapter_from_chan(chan);
>   struct dvb_frontend *fe;
>  
> - i2c = >dev->channel[0].i2c_adapter;
>   if (chan->fe->ops.i2c_gate_ctrl)
>   chan->fe->ops.i2c_gate_ctrl(chan->fe, 1);
>   fe = dvb_attach(tda18271c2dd_attach, chan->fe, i2c, 0x60);
> @@ -240,7 +241,7 @@ static int tuner_tda18212_ping(struct ngene_channel *chan,
>  static int tuner_attach_tda18212(struct ngene_channel *chan, u32 dmdtype)
>  {
>   struct device *pdev = >dev->pci_dev->dev;
> - struct i2c_adapter *i2c;
> + struct i2c_adapter *i2c = i2c_adapter_from_chan(chan);
>   struct i2c_client *client;
>   struct tda18212_config config = {
>   .fe = chan->fe,
> @@ -262,12 +263,6 @@ static int tuner_attach_tda18212(struct ngene_channel 
> *chan, u32 dmdtype)
>   else
>   board_info.addr = 0x60;
>  
> - /* tuner 1+2: i2c adapter #0, tuner 3+4: i2c adapter #1 */
> - if (chan->number < 2)
> - i2c = >dev->channel[0].i2c_adapter;
> - else
> - i2c = >dev->channel[1].i2c_adapter;
> -
>   /*
>* due to a hardware quirk with the I2C gate on the stv0367+tda18212
>* combo, the tda18212 must be probed by reading it's id _twice_ when
> @@ -320,7 +315,7 @@ static int tuner_attach_probe(struct ngene_channel *chan)
>  static int demod_attach_stv0900(struct ngene_channel *chan)
>  {
>   struct device *pdev = >dev->pci_dev->dev;
> - struct i2c_adapter *i2c;
> + struct i2c_adapter *i2c = i2c_adapter_from_chan(chan);
>   struct 

Re: [PATCH] media: ov772x: constify ov772x_frame_intervals

[jacopo mondi]

Hi Mauro,

On Tue, Mar 06, 2018 at 10:35:22AM -0500, Mauro Carvalho Chehab wrote:
> The values on this array never changes. Make it const.
>
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Jacopo Mondi 

Since I'm sure there will be more cleanup/fixes on tw9910 and ov772x,
could you please take into account my series:
[PATCH v2 00/11] media: ov772x/tw9910 cleanup
before any additional change to these 2 drivers?

Thanks
   j
> ---
>  drivers/media/i2c/ov772x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
> index 16665af0c712..321105bb3161 100644
> --- a/drivers/media/i2c/ov772x.c
> +++ b/drivers/media/i2c/ov772x.c
> @@ -531,7 +531,7 @@ static const struct ov772x_win_size ov772x_win_sizes[] = {
>  /*
>   * frame rate settings lists
>   */
> -static unsigned int ov772x_frame_intervals[] = { 5, 10, 15, 20, 30, 60 };
> +static const unsigned int ov772x_frame_intervals[] = { 5, 10, 15, 20, 30, 60 
> };
>
>  /*
>   * general function
> --
> 2.14.3
>

[PATCH] media: ov772x: constify ov772x_frame_intervals

[Mauro Carvalho Chehab]

The values on this array never changes. Make it const.

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

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 16665af0c712..321105bb3161 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -531,7 +531,7 @@ static const struct ov772x_win_size ov772x_win_sizes[] = {
 /*
  * frame rate settings lists
  */
-static unsigned int ov772x_frame_intervals[] = { 5, 10, 15, 20, 30, 60 };
+static const unsigned int ov772x_frame_intervals[] = { 5, 10, 15, 20, 30, 60 };
 
 /*
  * general function
-- 
2.14.3

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

[Sakari Ailus]

Hi Yong,

Thanks for the patchset; please see my comments below.

On Tue, Mar 06, 2018 at 10:16:02AM +0800, Yong Deng wrote:
> Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2
> interface and CSI1 is used for parallel interface. This is not
> documented in datasheet but by test and guess.
> 
> This patch implement a v4l2 framework driver for it.
> 
> Currently, the driver only support the parallel interface. MIPI-CSI2,
> ISP's support are not included in this patch.
> 
> Reviewed-by: Maxime Ripard 
> Tested-by: Maxime Ripard 
> Signed-off-by: Yong Deng 
> ---
>  MAINTAINERS|   8 +
>  drivers/media/platform/Kconfig |   1 +
>  drivers/media/platform/Makefile|   2 +
>  drivers/media/platform/sunxi/sun6i-csi/Kconfig |   9 +
>  drivers/media/platform/sunxi/sun6i-csi/Makefile|   3 +
>  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 936 
> +
>  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 145 
>  .../media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h | 196 +
>  .../media/platform/sunxi/sun6i-csi/sun6i_video.c   | 759 +
>  .../media/platform/sunxi/sun6i-csi/sun6i_video.h   |  53 ++
>  10 files changed, 2112 insertions(+)
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Kconfig
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/Makefile
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_reg.h
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
>  create mode 100644 drivers/media/platform/sunxi/sun6i-csi/sun6i_video.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 91ed6adfa4a6..b4a331ad35b5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3793,6 +3793,14 @@ M: Jaya Kumar 
>  S:   Maintained
>  F:   sound/pci/cs5535audio/
>  
> +CSI DRIVERS FOR ALLWINNER V3s
> +M:   Yong Deng 
> +L:   linux-media@vger.kernel.org
> +T:   git git://linuxtv.org/media_tree.git
> +S:   Maintained
> +F:   drivers/media/platform/sunxi/sun6i-csi/
> +F:   Documentation/devicetree/bindings/media/sun6i-csi.txt
> +
>  CW1200 WLAN driver
>  M:   Solomon Peachy 
>  S:   Maintained
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index f9cc0582c8a9..7f1ee46c3258 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -159,6 +159,7 @@ source "drivers/media/platform/am437x/Kconfig"
>  source "drivers/media/platform/xilinx/Kconfig"
>  source "drivers/media/platform/rcar-vin/Kconfig"
>  source "drivers/media/platform/atmel/Kconfig"
> +source "drivers/media/platform/sunxi/sun6i-csi/Kconfig"
>  
>  config VIDEO_TI_CAL
>   tristate "TI CAL (Camera Adaptation Layer) driver"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 85e112122f32..143d8a473b0a 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -96,3 +96,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)  += 
> qcom/camss-8x16/
>  obj-$(CONFIG_VIDEO_QCOM_VENUS)   += qcom/venus/
>  
>  obj-y+= meson/
> +
> +obj-$(CONFIG_VIDEO_SUN6I_CSI)+= sunxi/sun6i-csi/
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/Kconfig 
> b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
> new file mode 100644
> index ..314188aae2c2
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/Kconfig
> @@ -0,0 +1,9 @@
> +config VIDEO_SUN6I_CSI
> + tristate "Allwinner V3s Camera Sensor Interface driver"
> + depends on VIDEO_V4L2 && COMMON_CLK && VIDEO_V4L2_SUBDEV_API && HAS_DMA
> + depends on ARCH_SUNXI || COMPILE_TEST
> + select VIDEOBUF2_DMA_CONTIG
> + select REGMAP_MMIO
> + select V4L2_FWNODE
> + ---help---
> +Support for the Allwinner Camera Sensor Interface Controller on V3s.
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/Makefile 
> b/drivers/media/platform/sunxi/sun6i-csi/Makefile
> new file mode 100644
> index ..213cb6be9e9c
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/Makefile
> @@ -0,0 +1,3 @@
> +sun6i-csi-y += sun6i_video.o sun6i_csi.o
> +
> +obj-$(CONFIG_VIDEO_SUN6I_CSI) += sun6i-csi.o
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c 
> b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> new file mode 100644
> index ..26d57e6053df
> --- /dev/null
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
> @@ -0,0 +1,936 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2011-2018 Magewell Electronics Co., Ltd. (Nanjing)
> + * All rights 

[PATCH] media: rc: oops in ir_timer_keyup after device unplug

[Sean Young]

If there is IR in the raw kfifo when ir_raw_event_unregister() is called,
then kthread_stop() causes ir_raw_event_thread to be scheduled, decode
some scancodes and re-arm timer_keyup. The timer_keyup then fires when
the rc device is long gone.

Cc: sta...@vger.kernel.org
Signed-off-by: Sean Young 
---
 drivers/media/rc/rc-main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 4a952108ba1e..8621761a680f 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1932,12 +1932,12 @@ void rc_unregister_device(struct rc_dev *dev)
if (!dev)
return;
 
-   del_timer_sync(>timer_keyup);
-   del_timer_sync(>timer_repeat);
-
if (dev->driver_type == RC_DRIVER_IR_RAW)
ir_raw_event_unregister(dev);
 
+   del_timer_sync(>timer_keyup);
+   del_timer_sync(>timer_repeat);
+
rc_free_rx_device(dev);
 
mutex_lock(>lock);
-- 
2.14.3

[PATCH v2] media: rc: new driver for early iMon device

[Sean Young]

These devices were supported by the lirc_imon.c driver which was removed
from staging in commit f41003a23a02 ("[media] staging: lirc_imon: port
remaining usb ids to imon and remove").

Signed-off-by: Sean Young 
---
 MAINTAINERS |   7 ++
 drivers/media/rc/Kconfig|  12 +++
 drivers/media/rc/Makefile   |   1 +
 drivers/media/rc/imon_raw.c | 199 
 4 files changed, 219 insertions(+)
 create mode 100644 drivers/media/rc/imon_raw.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 0eea2f0e9456..e1ac64c4830b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6903,6 +6903,13 @@ M:   James Hogan 
 S: Maintained
 F: drivers/media/rc/img-ir/
 
+IMON SOUNDGRAPH USB IR RECEIVER
+M: Sean Young 
+L: linux-media@vger.kernel.org
+S: Maintained
+F: drivers/media/rc/imon_raw.c
+F: drivers/media/rc/imon.c
+
 IMS TWINTURBO FRAMEBUFFER DRIVER
 L: linux-fb...@vger.kernel.org
 S: Orphan
diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index 447f82c1f65a..7ad05a6ef350 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -175,6 +175,18 @@ config IR_IMON
   To compile this driver as a module, choose M here: the
   module will be called imon.
 
+config IR_IMON_RAW
+   tristate "SoundGraph iMON Receiver (early raw IR models)"
+   depends on USB_ARCH_HAS_HCD
+   depends on RC_CORE
+   select USB
+   ---help---
+  Say Y here if you want to use a SoundGraph iMON IR Receiver,
+  early raw models.
+
+  To compile this driver as a module, choose M here: the
+  module will be called imon_raw.
+
 config IR_MCEUSB
tristate "Windows Media Center Ed. eHome Infrared Transceiver"
depends on USB_ARCH_HAS_HCD
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index 0e857816ac2d..e098e127b26a 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_IR_XMP_DECODER) += ir-xmp-decoder.o
 obj-$(CONFIG_RC_ATI_REMOTE) += ati_remote.o
 obj-$(CONFIG_IR_HIX5HD2) += ir-hix5hd2.o
 obj-$(CONFIG_IR_IMON) += imon.o
+obj-$(CONFIG_IR_IMON_RAW) += imon_raw.o
 obj-$(CONFIG_IR_ITE_CIR) += ite-cir.o
 obj-$(CONFIG_IR_MCEUSB) += mceusb.o
 obj-$(CONFIG_IR_FINTEK) += fintek-cir.o
diff --git a/drivers/media/rc/imon_raw.c b/drivers/media/rc/imon_raw.c
new file mode 100644
index ..32709f96de14
--- /dev/null
+++ b/drivers/media/rc/imon_raw.c
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Copyright (C) 2018 Sean Young 
+
+#include 
+#include 
+#include 
+#include 
+
+/* Each bit is 250us */
+#define BIT_DURATION 25
+
+struct imon {
+   struct device *dev;
+   struct urb *ir_urb;
+   struct rc_dev *rcdev;
+   u8 ir_buf[8];
+   char phys[64];
+};
+
+/*
+ * ffs/find_next_bit() searches in the wrong direction, so open-code our own.
+ */
+static inline int is_bit_set(const u8 *buf, int bit)
+{
+   return buf[bit / 8] & (0x80 >> (bit & 7));
+}
+
+static void imon_ir_data(struct imon *imon)
+{
+   DEFINE_IR_RAW_EVENT(rawir);
+   int offset = 0, size = 5 * 8;
+   int bit;
+
+   dev_dbg(imon->dev, "data: %*ph", 8, imon->ir_buf);
+
+   while (offset < size) {
+   bit = offset;
+   while (!is_bit_set(imon->ir_buf, bit) && bit < size)
+   bit++;
+   dev_dbg(imon->dev, "pulse: %d bits", bit - offset);
+   if (bit > offset) {
+   rawir.pulse = true;
+   rawir.duration = (bit - offset) * BIT_DURATION;
+   ir_raw_event_store_with_filter(imon->rcdev, );
+   }
+
+   if (bit >= size)
+   break;
+
+   offset = bit;
+   while (is_bit_set(imon->ir_buf, bit) && bit < size)
+   bit++;
+   dev_dbg(imon->dev, "space: %d bits", bit - offset);
+
+   rawir.pulse = false;
+   rawir.duration = (bit - offset) * BIT_DURATION;
+   ir_raw_event_store_with_filter(imon->rcdev, );
+
+   offset = bit;
+   }
+
+   if (imon->ir_buf[7] == 0x0a) {
+   ir_raw_event_set_idle(imon->rcdev, true);
+   ir_raw_event_handle(imon->rcdev);
+   }
+}
+
+static void imon_ir_rx(struct urb *urb)
+{
+   struct imon *imon = urb->context;
+   int ret;
+
+   switch (urb->status) {
+   case 0:
+   if (imon->ir_buf[7] != 0xff)
+   imon_ir_data(imon);
+   break;
+   case -ECONNRESET:
+   case -ENOENT:
+   case -ESHUTDOWN:
+   usb_unlink_urb(urb);
+   return;
+   case -EPIPE:
+   default:
+   dev_dbg(imon->dev, "error: urb status = %d", urb->status);
+   break;
+   }
+
+   ret = 

Re: [PATCH 3/7] si2157: Add hybrid tuner support

[Brad Love]

Hi Mauro,


On 2018-03-06 06:24, Mauro Carvalho Chehab wrote:
> Hi Brad,
>
> As patches 1 and 2 are independent of this one, and should be backward
> compatible, I'm applying them, but I have issues with this one too :-)
>
> Em Tue, 16 Jan 2018 14:48:35 -0600
> Brad Love  escreveu:
>
>> On 2018-01-15 23:05, Antti Palosaari wrote:
>>> Hello
>>> So the use case is to share single tuner with multiple demodulators?
>>> Why don't just register single tuner and pass that info to multiple
>>> demods?
>>>
>>> Antti  
>> Hello Antti,
>>
>> It was done this way because of lack of knowledge of other ways. The
>> method I used mirrored that done by the three other drivers I found
>> which supported *and* included multiple front ends. We had this _attach
>> function sitting around as part of wip analog support to the si2157, and
>> it seemed like a nice fit here.
> The thing is that dvb_attach() is a very dirty and ugly hack,
> done when we needed to use I2C low level API calls inside DVB,
> while using high level bus-attach based I2C API for V4L.
>
> The hybrid_tuner_instance is yet-another-ugly dirty hack, with even
> causes lots of troubles for static analyzers.
>
> Nowadays, we should, instead, let the I2C bus bind the device
> at the bus and take care of lifetime issues.
>
> Btw, please take a look on this changeset:
>
>   8f569c0b4e6b ("media: dvb-core: add helper functions for I2C binding")
>
> and an example about how to simplify the binding code at:
>
>   ad32495b1513 ("media: em28xx-dvb: simplify DVB module probing logic")
>
> Em28xx is currently the only driver using the newer functions - My
> plan is to cleanup other drivers using the same logic as well,
> eventually improving the implementation of the new functions if needed.

I noticed the cleanup, I'll include it in my revised patch.



>
>> I just perused the tree again and noticed one spot I missed originally,
>> which does not use an _attach function. I didn't realize I could just
>> memcpy tuner_ops from fe[0] to fe[1] and call it a done deal, it does
>> appear to work the same though.
> I didn't write any hybrid tuner support using the new I2C binding
> schema, but, if both demods use the same tuner, I guess that's all
> it should be needed (and set adap->mfe_shared to 1).


This patch can be removed from the set, I have just memcpy'd
the tuner_ops to fe[1] now and everything works. I will resubmit a patch
with the mfe_shared property set though.



>
>> Is this really all that is required? If so, I'll modify patch 7/7 and
>> put this patch to the side for now.
>>
>> Cheers,
>>
>> Brad
>>
>>
>>> On 01/12/2018 06:19 PM, Brad Love wrote:  
 Add ability to share a tuner amongst demodulators. Addtional
 demods are attached using hybrid_tuner_instance_list.

 The changes are equivalent to moving all of probe to _attach.
 Results are backwards compatible with current usage.

 If the tuner is acquired via attach, then .release cleans state.
 if the tuner is an i2c driver, then .release is set to NULL, and
 .remove cleans remaining state.

 The following file contains a static si2157_attach:
 - drivers/media/pci/saa7164/saa7164-dvb.c
 The function name has been appended with _priv to appease
 the compiler.

 Signed-off-by: Brad Love 
 ---
   drivers/media/pci/saa7164/saa7164-dvb.c |  11 +-
   drivers/media/tuners/si2157.c   | 232
 +++-
   drivers/media/tuners/si2157.h   |  14 ++
   drivers/media/tuners/si2157_priv.h  |   5 +
   4 files changed, 192 insertions(+), 70 deletions(-)

 diff --git a/drivers/media/pci/saa7164/saa7164-dvb.c
 b/drivers/media/pci/saa7164/saa7164-dvb.c
 index e76d3ba..9522c6c 100644
 --- a/drivers/media/pci/saa7164/saa7164-dvb.c
 +++ b/drivers/media/pci/saa7164/saa7164-dvb.c
 @@ -110,8 +110,9 @@ static struct si2157_config
 hauppauge_hvr2255_tuner_config = {
   .if_port = 1,
   };
   -static int si2157_attach(struct saa7164_port *port, struct
 i2c_adapter *adapter,
 -    struct dvb_frontend *fe, u8 addr8bit, struct si2157_config *cfg)
 +static int si2157_attach_priv(struct saa7164_port *port,
 +    struct i2c_adapter *adapter, struct dvb_frontend *fe,
 +    u8 addr8bit, struct si2157_config *cfg)
   {
   struct i2c_board_info bi;
   struct i2c_client *tuner;
 @@ -624,11 +625,13 @@ int saa7164_dvb_register(struct saa7164_port
 *port)
   if (port->dvb.frontend != NULL) {
     if (port->nr == 0) {
 -    si2157_attach(port, >i2c_bus[0].i2c_adap,
 +    si2157_attach_priv(port,
 +  >i2c_bus[0].i2c_adap,
     port->dvb.frontend, 0xc0,
     _hvr2255_tuner_config);
   } else {
 -    

Re: [PATCH 1/2] media: ov5645: Fix write_reg return code

[Sakari Ailus]

HI Mauro,

On Tue, Mar 06, 2018 at 10:40:10AM -0300, Mauro Carvalho Chehab wrote:
> Em Thu,  8 Feb 2018 11:41:59 +0200
> Todor Tomov  escreveu:
> 
> > I2C transfer functions return number of successful operations (on success).
> > 
> > Do not return the received positive return code but instead return 0 on
> > success. The users of write_reg function already use this logic.
> > 
> > Signed-off-by: Todor Tomov 
> > ---
> >  drivers/media/i2c/ov5645.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> > index d28845f..9755562 100644
> > --- a/drivers/media/i2c/ov5645.c
> > +++ b/drivers/media/i2c/ov5645.c
> > @@ -600,11 +600,13 @@ static int ov5645_write_reg(struct ov5645 *ov5645, 
> > u16 reg, u8 val)
> > regbuf[2] = val;
> >  
> > ret = i2c_master_send(ov5645->i2c_client, regbuf, 3);
> > -   if (ret < 0)
> > +   if (ret < 0) {
> > dev_err(ov5645->dev, "%s: write reg error %d: reg=%x, val=%x\n",
> > __func__, ret, reg, val);
> > +   return ret;
> > +   }
> 
> Actually, if ret < 3, it should return an error too (like -EREMOTEIO 
> or -EIO).

i2c_master_send() always returns a negative error code or the number of
octets written.

But thank you for reminding me about the patch. :-)

-- 
Regards,

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

Re: [PATCH 2/2] media: ov5645: Improve mode finding function

[Sakari Ailus]

Hi Todor,

On Thu, Feb 08, 2018 at 11:42:00AM +0200, Todor Tomov wrote:
> Find the sensor mode by comparing the size of the requested image size
> and the sensor mode's image size. The distance between image sizes is the
> size in pixels of the non-overlapping regions between the requested size
> and the frame-specified size. This logic is borrowed from et8ek8 sensor
> driver.
> 
> Signed-off-by: Todor Tomov 
> ---
>  drivers/media/i2c/ov5645.c | 24 +++-
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> index 9755562..6d06c50 100644
> --- a/drivers/media/i2c/ov5645.c
> +++ b/drivers/media/i2c/ov5645.c
> @@ -964,18 +964,24 @@ __ov5645_get_pad_crop(struct ov5645 *ov5645, struct 
> v4l2_subdev_pad_config *cfg,
>  static const struct ov5645_mode_info *
>  ov5645_find_nearest_mode(unsigned int width, unsigned int height)
>  {
> - int i;
> + unsigned int max_dist_match = (unsigned int) -1;
> + int i, n = 0;
>  
> - for (i = ARRAY_SIZE(ov5645_mode_info_data) - 1; i >= 0; i--) {
> - if (ov5645_mode_info_data[i].width <= width &&
> - ov5645_mode_info_data[i].height <= height)
> - break;
> + for (i = 0; i < ARRAY_SIZE(ov5645_mode_info_data); i++) {
> + unsigned int dist = min(width, ov5645_mode_info_data[i].width)
> + * min(height, ov5645_mode_info_data[i].height);
> +
> + dist = ov5645_mode_info_data[i].width *
> + ov5645_mode_info_data[i].height
> +  + width * height - 2 * dist;

Could you use v4l2_find_nearest_size()?

The patch is here:



The pull request has been sent on it.

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

Re: [PATCH 1/2] media: ov5645: Fix write_reg return code

[Mauro Carvalho Chehab]

Em Thu,  8 Feb 2018 11:41:59 +0200
Todor Tomov  escreveu:

> I2C transfer functions return number of successful operations (on success).
> 
> Do not return the received positive return code but instead return 0 on
> success. The users of write_reg function already use this logic.
> 
> Signed-off-by: Todor Tomov 
> ---
>  drivers/media/i2c/ov5645.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> index d28845f..9755562 100644
> --- a/drivers/media/i2c/ov5645.c
> +++ b/drivers/media/i2c/ov5645.c
> @@ -600,11 +600,13 @@ static int ov5645_write_reg(struct ov5645 *ov5645, u16 
> reg, u8 val)
>   regbuf[2] = val;
>  
>   ret = i2c_master_send(ov5645->i2c_client, regbuf, 3);
> - if (ret < 0)
> + if (ret < 0) {
>   dev_err(ov5645->dev, "%s: write reg error %d: reg=%x, val=%x\n",
>   __func__, ret, reg, val);
> + return ret;
> + }

Actually, if ret < 3, it should return an error too (like -EREMOTEIO 
or -EIO).

>  
> - return ret;
> + return 0;
>  }
>  
>  static int ov5645_read_reg(struct ov5645 *ov5645, u16 reg, u8 *val)



Thanks,
Mauro

Re: [PATCH v2 1/5] media: dvb_frontend: add FEC modes, S2X modulations and 64K transmission

[Mauro Carvalho Chehab]

Hi Daniel,

Em Mon, 22 Jan 2018 18:13:42 +0100
Daniel Scheller  escreveu:

> From: Daniel Scheller 
> 
> Add 1/4 and 1/3 FEC ratios, 64/128/256-APSK S2X modulations and 64K
> transmission mode. Update relevant doc items aswell.

Please properly document DVB-S2X. You need to add something at
Documentation/media/uapi/dvb/fe_property_parameters.rst,
and Documentation/media/uapi/dvb/frontend-property-satellite-systems.rst
in order to properly describe DVB-S2X.

Please take a look at:

https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/dvb/fe_property_parameters.html
and:
https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/dvb/frontend-property-satellite-systems.html

Basically, for each DVB-S2X parameter, you should say what
values are valid.

Also, I'm not seeing a SYS_DVBS2X delivery system. If the idea is
not to create it, the documentation should state that SYS_DVBS2
should be used, and the Kernel should provide some way to allow
userspace to detect if the DVB-S2X parameters are valid or not.

As we're running out of DVB frontend caps flags, it means that
you need to also add a patch to this series implementing a
new way to query frontend capabilities.

Regards,
Mauro

> 
> Signed-off-by: Daniel Scheller 
> ---
>  Documentation/media/frontend.h.rst.exceptions |  6 ++
>  include/uapi/linux/dvb/frontend.h | 13 +
>  2 files changed, 19 insertions(+)
> 
> diff --git a/Documentation/media/frontend.h.rst.exceptions 
> b/Documentation/media/frontend.h.rst.exceptions
> index f7c4df620a52..ae1148be0a39 100644
> --- a/Documentation/media/frontend.h.rst.exceptions
> +++ b/Documentation/media/frontend.h.rst.exceptions
> @@ -84,6 +84,9 @@ ignore symbol APSK_16
>  ignore symbol APSK_32
>  ignore symbol DQPSK
>  ignore symbol QAM_4_NR
> +ignore symbol APSK_64
> +ignore symbol APSK_128
> +ignore symbol APSK_256
>  
>  ignore symbol SEC_VOLTAGE_13
>  ignore symbol SEC_VOLTAGE_18
> @@ -117,6 +120,8 @@ ignore symbol FEC_AUTO
>  ignore symbol FEC_3_5
>  ignore symbol FEC_9_10
>  ignore symbol FEC_2_5
> +ignore symbol FEC_1_4
> +ignore symbol FEC_1_3
>  
>  ignore symbol TRANSMISSION_MODE_AUTO
>  ignore symbol TRANSMISSION_MODE_1K
> @@ -129,6 +134,7 @@ ignore symbol TRANSMISSION_MODE_C1
>  ignore symbol TRANSMISSION_MODE_C3780
>  ignore symbol TRANSMISSION_MODE_2K
>  ignore symbol TRANSMISSION_MODE_8K
> +ignore symbol TRANSMISSION_MODE_64K
>  
>  ignore symbol GUARD_INTERVAL_AUTO
>  ignore symbol GUARD_INTERVAL_1_128
> diff --git a/include/uapi/linux/dvb/frontend.h 
> b/include/uapi/linux/dvb/frontend.h
> index 4f9b4551c534..227268a657cd 100644
> --- a/include/uapi/linux/dvb/frontend.h
> +++ b/include/uapi/linux/dvb/frontend.h
> @@ -296,6 +296,8 @@ enum fe_spectral_inversion {
>   * @FEC_3_5:  Forward Error Correction Code 3/5
>   * @FEC_9_10: Forward Error Correction Code 9/10
>   * @FEC_2_5:  Forward Error Correction Code 2/5
> + * @FEC_1_4:  Forward Error Correction Code 1/4
> + * @FEC_1_3:  Forward Error Correction Code 1/3
>   *
>   * Please note that not all FEC types are supported by a given standard.
>   */
> @@ -313,6 +315,8 @@ enum fe_code_rate {
>   FEC_3_5,
>   FEC_9_10,
>   FEC_2_5,
> + FEC_1_4,
> + FEC_1_3,
>  };
>  
>  /**
> @@ -331,6 +335,9 @@ enum fe_code_rate {
>   * @APSK_32: 32-APSK modulation
>   * @DQPSK:   DQPSK modulation
>   * @QAM_4_NR:4-QAM-NR modulation
> + * @APSK_64: 64-APSK modulation
> + * @APSK_128:128-APSK modulation
> + * @APSK_256:256-APSK modulation
>   *
>   * Please note that not all modulations are supported by a given standard.
>   *
> @@ -350,6 +357,9 @@ enum fe_modulation {
>   APSK_32,
>   DQPSK,
>   QAM_4_NR,
> + APSK_64,
> + APSK_128,
> + APSK_256,
>  };
>  
>  /**
> @@ -374,6 +384,8 @@ enum fe_modulation {
>   *   Single Carrier (C=1) transmission mode (DTMB only)
>   * @TRANSMISSION_MODE_C3780:
>   *   Multi Carrier (C=3780) transmission mode (DTMB only)
> + * @TRANSMISSION_MODE_64K:
> + *   Transmission mode 64K
>   *
>   * Please note that not all transmission modes are supported by a given
>   * standard.
> @@ -388,6 +400,7 @@ enum fe_transmit_mode {
>   TRANSMISSION_MODE_32K,
>   TRANSMISSION_MODE_C1,
>   TRANSMISSION_MODE_C3780,
> + TRANSMISSION_MODE_64K,
>  };
>  
>  /**



Thanks,
Mauro

Re: [PATCH 3/7] si2157: Add hybrid tuner support

[Mauro Carvalho Chehab]

Hi Brad,

As patches 1 and 2 are independent of this one, and should be backward
compatible, I'm applying them, but I have issues with this one too :-)

Em Tue, 16 Jan 2018 14:48:35 -0600
Brad Love  escreveu:

> On 2018-01-15 23:05, Antti Palosaari wrote:
> > Hello
> > So the use case is to share single tuner with multiple demodulators?
> > Why don't just register single tuner and pass that info to multiple
> > demods?
> >
> > Antti  
> 
> Hello Antti,
> 
> It was done this way because of lack of knowledge of other ways. The
> method I used mirrored that done by the three other drivers I found
> which supported *and* included multiple front ends. We had this _attach
> function sitting around as part of wip analog support to the si2157, and
> it seemed like a nice fit here.

The thing is that dvb_attach() is a very dirty and ugly hack,
done when we needed to use I2C low level API calls inside DVB,
while using high level bus-attach based I2C API for V4L.

The hybrid_tuner_instance is yet-another-ugly dirty hack, with even
causes lots of troubles for static analyzers.

Nowadays, we should, instead, let the I2C bus bind the device
at the bus and take care of lifetime issues.

Btw, please take a look on this changeset:

8f569c0b4e6b ("media: dvb-core: add helper functions for I2C binding")

and an example about how to simplify the binding code at:

ad32495b1513 ("media: em28xx-dvb: simplify DVB module probing logic")

Em28xx is currently the only driver using the newer functions - My
plan is to cleanup other drivers using the same logic as well,
eventually improving the implementation of the new functions if needed.

> I just perused the tree again and noticed one spot I missed originally,
> which does not use an _attach function. I didn't realize I could just
> memcpy tuner_ops from fe[0] to fe[1] and call it a done deal, it does
> appear to work the same though.

I didn't write any hybrid tuner support using the new I2C binding
schema, but, if both demods use the same tuner, I guess that's all
it should be needed (and set adap->mfe_shared to 1).

> Is this really all that is required? If so, I'll modify patch 7/7 and
> put this patch to the side for now.
> 
> Cheers,
> 
> Brad
> 
> 
> >
> > On 01/12/2018 06:19 PM, Brad Love wrote:  
> >> Add ability to share a tuner amongst demodulators. Addtional
> >> demods are attached using hybrid_tuner_instance_list.
> >>
> >> The changes are equivalent to moving all of probe to _attach.
> >> Results are backwards compatible with current usage.
> >>
> >> If the tuner is acquired via attach, then .release cleans state.
> >> if the tuner is an i2c driver, then .release is set to NULL, and
> >> .remove cleans remaining state.
> >>
> >> The following file contains a static si2157_attach:
> >> - drivers/media/pci/saa7164/saa7164-dvb.c
> >> The function name has been appended with _priv to appease
> >> the compiler.
> >>
> >> Signed-off-by: Brad Love 
> >> ---
> >>   drivers/media/pci/saa7164/saa7164-dvb.c |  11 +-
> >>   drivers/media/tuners/si2157.c   | 232
> >> +++-
> >>   drivers/media/tuners/si2157.h   |  14 ++
> >>   drivers/media/tuners/si2157_priv.h  |   5 +
> >>   4 files changed, 192 insertions(+), 70 deletions(-)
> >>
> >> diff --git a/drivers/media/pci/saa7164/saa7164-dvb.c
> >> b/drivers/media/pci/saa7164/saa7164-dvb.c
> >> index e76d3ba..9522c6c 100644
> >> --- a/drivers/media/pci/saa7164/saa7164-dvb.c
> >> +++ b/drivers/media/pci/saa7164/saa7164-dvb.c
> >> @@ -110,8 +110,9 @@ static struct si2157_config
> >> hauppauge_hvr2255_tuner_config = {
> >>   .if_port = 1,
> >>   };
> >>   -static int si2157_attach(struct saa7164_port *port, struct
> >> i2c_adapter *adapter,
> >> -    struct dvb_frontend *fe, u8 addr8bit, struct si2157_config *cfg)
> >> +static int si2157_attach_priv(struct saa7164_port *port,
> >> +    struct i2c_adapter *adapter, struct dvb_frontend *fe,
> >> +    u8 addr8bit, struct si2157_config *cfg)
> >>   {
> >>   struct i2c_board_info bi;
> >>   struct i2c_client *tuner;
> >> @@ -624,11 +625,13 @@ int saa7164_dvb_register(struct saa7164_port
> >> *port)
> >>   if (port->dvb.frontend != NULL) {
> >>     if (port->nr == 0) {
> >> -    si2157_attach(port, >i2c_bus[0].i2c_adap,
> >> +    si2157_attach_priv(port,
> >> +  >i2c_bus[0].i2c_adap,
> >>     port->dvb.frontend, 0xc0,
> >>     _hvr2255_tuner_config);
> >>   } else {
> >> -    si2157_attach(port, >i2c_bus[1].i2c_adap,
> >> +    si2157_attach_priv(port,
> >> +  >i2c_bus[1].i2c_adap,
> >>     port->dvb.frontend, 0xc0,
> >>     _hvr2255_tuner_config);
> >>   }
> >> diff --git a/drivers/media/tuners/si2157.c
> >> b/drivers/media/tuners/si2157.c
> 

Re: [PATCH] dvb: Save port number and provide sysfs attributes to pass values to udev

[Mauro Carvalho Chehab]

Hi David,

Em Wed, 10 Jan 2018 14:50:35 +
David Howells  escreveu:

> Some devices, such as the DVBSky S952 and T982 cards, are dual port cards
> that provide two cx23885 devices on the same PCI device, which means the
> attributes available for writing udev rules are exactly the same, apart
> from the adapter number.  Unfortunately, the adapter numbers are dependent
> on the order in which things are initialised, so this can change over
> different releases of the kernel.
> 
> The struct cx23885_tsport has a port number available, which is printed
> during boot:
> 
>   [   10.951517] DVBSky T982 port 1 MAC address: 00:17:42:54:09:87
>   ...
>   [   10.984875] DVBSky T982 port 2 MAC address: 00:17:42:54:09:88
> 
> To make it possible to distinguish these in udev, do the following steps:
> 
>  (1) Save the port number into struct dvb_adapter.
> 
>  (2) Provide sysfs attributes to export port number and also MAC address,
>  adapter number and type.  There are other fields that could perhaps be
>  exported also.
> 
> The new sysfs attributes can be seen from userspace as:
> 
>   [root@deneb ~]# ls /sys/class/dvb/dvb0.frontend0/
>   dev  device  dvb_adapter  dvb_mac  dvb_port  dvb_type
>   power  subsystem  uevent
>   [root@deneb ~]# cat /sys/class/dvb/dvb0.frontend0/dvb_*
>   0
>   00:17:42:54:09:87
>   0
>   frontend
> 
> They can be used in udev rules:
> 
>   SUBSYSTEM=="dvb", ATTRS{vendor}=="0x14f1", ATTRS{device}=="0x8852", 
> ATTRS{subsystem_device}=="0x0982", ATTR{dvb_mac}=="00:17:42:54:09:87", 
> PROGRAM="/bin/sh -c 'K=%k; K=$${K#dvb}; printf dvb/adapter9820/%%s 
> $${K#*.}'", SYMLINK+="%c"
>   SUBSYSTEM=="dvb", ATTRS{vendor}=="0x14f1", ATTRS{device}=="0x8852", 
> ATTRS{subsystem_device}=="0x0982", ATTR{dvb_mac}=="00:17:42:54:09:88", 
> PROGRAM="/bin/sh -c 'K=%k; K=$${K#dvb}; printf dvb/adapter9821/%%s 
> $${K#*.}'", SYMLINK+="%c"
> 
> where the match is made with ATTR{dvb_mac} or similar.  The rules above
> make symlinks from /dev/dvb/adapter982/* to /dev/dvb/adapterXX/*.
> 
> Note that binding the dvb-net device to a network interface and changing it
> there does not reflect back into the the dvb_adapter struct and doesn't
> change the MAC address here.  This means that a system with two identical
> cards in it may need to distinguish them by some other means than MAC
> address.

Sorry for not looking on it earlier... Has been very busy those days,
and the dvb sub-maintainer has not been responsive lately, due to some
personal issues.

> 
> Signed-off-by: David Howells 
> ---
> 
>  drivers/media/dvb-core/dvbdev.c |   46 
> +++
>  drivers/media/dvb-core/dvbdev.h |2 +
>  drivers/media/pci/cx23885/cx23885-dvb.c |2 +
>  3 files changed, 50 insertions(+)
> 
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 060c60ddfcc3..b3aa5ae3d57f 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -941,6 +941,51 @@ int dvb_usercopy(struct file *file,
>   return err;
>  }
>  
> +static ssize_t dvb_adapter_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct dvb_device *dvbdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%d\n", dvbdev->adapter->num);
> +}
> +static DEVICE_ATTR_RO(dvb_adapter);
> +
> +static ssize_t dvb_mac_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct dvb_device *dvbdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%pM\n", dvbdev->adapter->proposed_mac);
> +}
> +static DEVICE_ATTR_RO(dvb_mac);
> +
> +static ssize_t dvb_port_show(struct device *dev,
> +  struct device_attribute *attr, char *buf)
> +{
> + struct dvb_device *dvbdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%d\n", dvbdev->adapter->port_num);
> +}
> +static DEVICE_ATTR_RO(dvb_port);
> +
> +static ssize_t dvb_type_show(struct device *dev,
> +  struct device_attribute *attr, char *buf)
> +{
> + struct dvb_device *dvbdev = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%s\n", dnames[dvbdev->type]);
> +}
> +static DEVICE_ATTR_RO(dvb_type);
> +
> +static struct attribute *dvb_class_attrs[] = {
> + _attr_dvb_adapter.attr,
> + _attr_dvb_mac.attr,
> + _attr_dvb_port.attr,
> + _attr_dvb_type.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(dvb_class);
> +
>  static int dvb_uevent(struct device *dev, struct kobj_uevent_env *env)
>  {
>   struct dvb_device *dvbdev = dev_get_drvdata(dev);
> @@ -981,6 +1026,7 @@ static int __init init_dvbdev(void)
>   retval = PTR_ERR(dvb_class);
>   goto error;
>   }
> + dvb_class->dev_groups = dvb_class_groups,
>   dvb_class->dev_uevent = dvb_uevent;
>   dvb_class->devnode = dvb_devnode;
>   

[PATCH v2] media: s5h14*.h: fix typos for CONTINUOUS

[Mauro Carvalho Chehab]

There is a typo at the several s5h14*.h headers: continuous were
spelled incorrectly.

Fix it with this script:

for i in $(git grep -l S5H1409_MPEGTIMING_CONTINOUS_NONINVERTING_CLOCK); do
sed 
s,S5H1409_MPEGTIMING_CONTINOUS_NONINVERTING_CLOCK,S5H1409_MPEGTIMING_CONTINUOUS_NONINVERTING_CLOCK,g
 -i $i
done
for i in $(git grep -l -i continous drivers/media); do sed 
s,CONTINOUS,CONTINUOUS,g -i $i; done

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/dvb-frontends/s5h1409.c   |  8 
 drivers/media/dvb-frontends/s5h1409.h   |  8 
 drivers/media/dvb-frontends/s5h1411.c   |  8 
 drivers/media/dvb-frontends/s5h1411.h   |  8 
 drivers/media/dvb-frontends/s5h1432.h   |  8 
 drivers/media/pci/cx18/cx18-dvb.c   |  4 ++--
 drivers/media/pci/cx23885/cx23885-dvb.c | 16 
 drivers/media/pci/cx88/cx88-dvb.c   |  8 
 drivers/media/pci/saa7134/saa7134-dvb.c |  2 +-
 drivers/media/pci/saa7164/saa7164-dvb.c |  2 +-
 drivers/media/usb/cx231xx/cx231xx-dvb.c |  6 +++---
 drivers/media/usb/dvb-usb/dib0700_devices.c |  2 +-
 drivers/media/usb/em28xx/em28xx-dvb.c   |  2 +-
 13 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/drivers/media/dvb-frontends/s5h1409.c 
b/drivers/media/dvb-frontends/s5h1409.c
index aced6a956ec5..a23ba8727218 100644
--- a/drivers/media/dvb-frontends/s5h1409.c
+++ b/drivers/media/dvb-frontends/s5h1409.c
@@ -682,17 +682,17 @@ static int s5h1409_set_mpeg_timing(struct dvb_frontend 
*fe, int mode)
 
val = s5h1409_readreg(state, 0xac) & 0xcfff;
switch (mode) {
-   case S5H1409_MPEGTIMING_CONTINOUS_INVERTING_CLOCK:
+   case S5H1409_MPEGTIMING_CONTINUOUS_INVERTING_CLOCK:
val |= 0x;
break;
-   case S5H1409_MPEGTIMING_CONTINOUS_NONINVERTING_CLOCK:
+   case S5H1409_MPEGTIMING_CONTINUOUS_NONINVERTING_CLOCK:
dprintk("%s(%d) Mode1 or Defaulting\n", __func__, mode);
val |= 0x1000;
break;
-   case S5H1409_MPEGTIMING_NONCONTINOUS_INVERTING_CLOCK:
+   case S5H1409_MPEGTIMING_NONCONTINUOUS_INVERTING_CLOCK:
val |= 0x2000;
break;
-   case S5H1409_MPEGTIMING_NONCONTINOUS_NONINVERTING_CLOCK:
+   case S5H1409_MPEGTIMING_NONCONTINUOUS_NONINVERTING_CLOCK:
val |= 0x3000;
break;
default:
diff --git a/drivers/media/dvb-frontends/s5h1409.h 
b/drivers/media/dvb-frontends/s5h1409.h
index b38557c451b9..87de58ffc822 100644
--- a/drivers/media/dvb-frontends/s5h1409.h
+++ b/drivers/media/dvb-frontends/s5h1409.h
@@ -52,10 +52,10 @@ struct s5h1409_config {
u8 status_mode;
 
/* MPEG signal timing */
-#define S5H1409_MPEGTIMING_CONTINOUS_INVERTING_CLOCK   0
-#define S5H1409_MPEGTIMING_CONTINOUS_NONINVERTING_CLOCK1
-#define S5H1409_MPEGTIMING_NONCONTINOUS_INVERTING_CLOCK2
-#define S5H1409_MPEGTIMING_NONCONTINOUS_NONINVERTING_CLOCK 3
+#define S5H1409_MPEGTIMING_CONTINUOUS_INVERTING_CLOCK   0
+#define S5H1409_MPEGTIMING_CONTINUOUS_NONINVERTING_CLOCK1
+#define S5H1409_MPEGTIMING_NONCONTINUOUS_INVERTING_CLOCK2
+#define S5H1409_MPEGTIMING_NONCONTINUOUS_NONINVERTING_CLOCK 3
u16 mpeg_timing;
 
/* HVR-1600 optimizations (to better work with MXL5005s)
diff --git a/drivers/media/dvb-frontends/s5h1411.c 
b/drivers/media/dvb-frontends/s5h1411.c
index c4b1e9725f3e..af5962807f2c 100644
--- a/drivers/media/dvb-frontends/s5h1411.c
+++ b/drivers/media/dvb-frontends/s5h1411.c
@@ -433,17 +433,17 @@ static int s5h1411_set_mpeg_timing(struct dvb_frontend 
*fe, int mode)
 
val = s5h1411_readreg(state, S5H1411_I2C_TOP_ADDR, 0xbe) & 0xcfff;
switch (mode) {
-   case S5H1411_MPEGTIMING_CONTINOUS_INVERTING_CLOCK:
+   case S5H1411_MPEGTIMING_CONTINUOUS_INVERTING_CLOCK:
val |= 0x;
break;
-   case S5H1411_MPEGTIMING_CONTINOUS_NONINVERTING_CLOCK:
+   case S5H1411_MPEGTIMING_CONTINUOUS_NONINVERTING_CLOCK:
dprintk("%s(%d) Mode1 or Defaulting\n", __func__, mode);
val |= 0x1000;
break;
-   case S5H1411_MPEGTIMING_NONCONTINOUS_INVERTING_CLOCK:
+   case S5H1411_MPEGTIMING_NONCONTINUOUS_INVERTING_CLOCK:
val |= 0x2000;
break;
-   case S5H1411_MPEGTIMING_NONCONTINOUS_NONINVERTING_CLOCK:
+   case S5H1411_MPEGTIMING_NONCONTINUOUS_NONINVERTING_CLOCK:
val |= 0x3000;
break;
default:
diff --git a/drivers/media/dvb-frontends/s5h1411.h 
b/drivers/media/dvb-frontends/s5h1411.h
index 791bab0e16e9..850ee713d64c 100644
--- a/drivers/media/dvb-frontends/s5h1411.h
+++ b/drivers/media/dvb-frontends/s5h1411.h
@@ -40,10 +40,10 @@ struct s5h1411_config {
u8 gpio;
 
/* MPEG signal timing */
-#define S5H1411_MPEGTIMING_CONTINOUS_INVERTING_CLOCK   0

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

[Sakari Ailus]

On Tue, Mar 06, 2018 at 06:52:16PM +0900, Tomasz Figa wrote:
> On Tue, Mar 6, 2018 at 6:46 PM, Sakari Ailus
>  wrote:
> > On Tue, Mar 06, 2018 at 06:28:43PM +0900, Tomasz Figa wrote:
> >> On Tue, Mar 6, 2018 at 6:18 PM, Sakari Ailus
> >>  wrote:
> >> > On Tue, Mar 06, 2018 at 05:51:36PM +0900, Tomasz Figa wrote:
> >> >> On Tue, Mar 6, 2018 at 5:40 PM, Sakari Ailus
> >> >>  wrote:
> >> >> > Hi Tomasz and Andy,
> >> >> >
> >> >> > On Sat, Mar 03, 2018 at 12:43:59AM +0900, Tomasz Figa wrote:
> >> >> > ...
> >> >> >> > +static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
> >> >> >> > +{
> >> >> >> > +   struct imx258 *imx258 =
> >> >> >> > +   container_of(ctrl->handler, struct imx258, 
> >> >> >> > ctrl_handler);
> >> >> >> > +   struct i2c_client *client = 
> >> >> >> > v4l2_get_subdevdata(>sd);
> >> >> >> > +   int ret = 0;
> >> >> >> > +   /*
> >> >> >> > +* Applying V4L2 control value only happens
> >> >> >> > +* when power is up for streaming
> >> >> >> > +*/
> >> >> >> > +   if (pm_runtime_get_if_in_use(>dev) <= 0)
> >> >> >> > +   return 0;
> >> >> >>
> >> >> >> I thought we decided to fix this to handle disabled runtime PM 
> >> >> >> properly.
> >> >> >
> >> >> > Good point. I bet this is a problem in a few other drivers, too. How 
> >> >> > would
> >> >> > you fix that? Check for zero only?
> >> >> >
> >> >>
> >> >> bool need_runtime_put;
> >> >>
> >> >> ret = pm_runtime_get_if_in_use(>dev);
> >> >> if (ret <= 0 && ret != -EINVAL)
> >> >> return ret;
> >> >> need_runtime_put = ret > 0;
> >> >>
> >> >> // Do stuff ...
> >> >>
> >> >> if (need_runtime_put)
> >> >>pm_runtime_put(>dev);
> >> >>
> >> >> I don't like how ugly it is, but it appears to be the only way to
> >> >> handle this correctly.
> >> >
> >> > The driver enables runtime PM so if runtime PM is enabled in kernel
> >> > configuration, it is enabled here. In that case 
> >> > pm_runtime_get_if_in_use()
> >> > will return either 0 or 1. So as far as I can see, changing the lines to:
> >> >
> >> > if (!pm_runtime_get_if_in_use(>dev))
> >> > return 0;
> >> >
> >> > is enough.
> >>
> >> Right, my bad. Somehow I was convinced that enable status can change at
> >> runtime.
> >
> > Good point. I guess in principle this could happen although I can't see a
> > reason to do so, other than to break things --- quoting
> > Documentation/power/runtime_pm.txt:
> >
> > The user space can effectively disallow the driver of the device to
> > power manage it at run time by changing the value of its
> > /sys/devices/.../power/control attribute to "on", which causes
> > pm_runtime_forbid() to be called. In principle, this mechanism may
> > also be used by the driver to effectively turn off the runtime
> > power management of the device until the user space turns it on.
> > Namely, during the initialization the driver can make sure that the
> > runtime PM status of the device is 'active' and call
> > pm_runtime_forbid(). It should be noted, however, that if the user
> > space has already intentionally changed the value of
> > /sys/devices/.../power/control to "auto" to allow the driver to
> > power manage the device at run time, the driver may confuse it by
> > using pm_runtime_forbid() this way.
> >
> > So that comes with a warning that things might not work well after doing
> > so.
> >
> > What comes to the driver code, I still wouldn't complicate it by attempting
> > to make a driver work in such a case.
> 
> I think pm_runtime_forbid() and pm_runtime_enable() operate on
> complete different data. That was exactly the source of my confusion
> earlier. Looking at the code [1], even if runtime PM is "forbidden",
> it is still "enabled" and just the usage count is incremented.
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/base/power/runtime.c#L1314

Ah, right. Thanks for the correction. Then indeed this is very clear.

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

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

[Tomasz Figa]

On Tue, Mar 6, 2018 at 6:46 PM, Sakari Ailus
 wrote:
> On Tue, Mar 06, 2018 at 06:28:43PM +0900, Tomasz Figa wrote:
>> On Tue, Mar 6, 2018 at 6:18 PM, Sakari Ailus
>>  wrote:
>> > On Tue, Mar 06, 2018 at 05:51:36PM +0900, Tomasz Figa wrote:
>> >> On Tue, Mar 6, 2018 at 5:40 PM, Sakari Ailus
>> >>  wrote:
>> >> > Hi Tomasz and Andy,
>> >> >
>> >> > On Sat, Mar 03, 2018 at 12:43:59AM +0900, Tomasz Figa wrote:
>> >> > ...
>> >> >> > +static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
>> >> >> > +{
>> >> >> > +   struct imx258 *imx258 =
>> >> >> > +   container_of(ctrl->handler, struct imx258, 
>> >> >> > ctrl_handler);
>> >> >> > +   struct i2c_client *client = v4l2_get_subdevdata(>sd);
>> >> >> > +   int ret = 0;
>> >> >> > +   /*
>> >> >> > +* Applying V4L2 control value only happens
>> >> >> > +* when power is up for streaming
>> >> >> > +*/
>> >> >> > +   if (pm_runtime_get_if_in_use(>dev) <= 0)
>> >> >> > +   return 0;
>> >> >>
>> >> >> I thought we decided to fix this to handle disabled runtime PM 
>> >> >> properly.
>> >> >
>> >> > Good point. I bet this is a problem in a few other drivers, too. How 
>> >> > would
>> >> > you fix that? Check for zero only?
>> >> >
>> >>
>> >> bool need_runtime_put;
>> >>
>> >> ret = pm_runtime_get_if_in_use(>dev);
>> >> if (ret <= 0 && ret != -EINVAL)
>> >> return ret;
>> >> need_runtime_put = ret > 0;
>> >>
>> >> // Do stuff ...
>> >>
>> >> if (need_runtime_put)
>> >>pm_runtime_put(>dev);
>> >>
>> >> I don't like how ugly it is, but it appears to be the only way to
>> >> handle this correctly.
>> >
>> > The driver enables runtime PM so if runtime PM is enabled in kernel
>> > configuration, it is enabled here. In that case pm_runtime_get_if_in_use()
>> > will return either 0 or 1. So as far as I can see, changing the lines to:
>> >
>> > if (!pm_runtime_get_if_in_use(>dev))
>> > return 0;
>> >
>> > is enough.
>>
>> Right, my bad. Somehow I was convinced that enable status can change at
>> runtime.
>
> Good point. I guess in principle this could happen although I can't see a
> reason to do so, other than to break things --- quoting
> Documentation/power/runtime_pm.txt:
>
> The user space can effectively disallow the driver of the device to
> power manage it at run time by changing the value of its
> /sys/devices/.../power/control attribute to "on", which causes
> pm_runtime_forbid() to be called. In principle, this mechanism may
> also be used by the driver to effectively turn off the runtime
> power management of the device until the user space turns it on.
> Namely, during the initialization the driver can make sure that the
> runtime PM status of the device is 'active' and call
> pm_runtime_forbid(). It should be noted, however, that if the user
> space has already intentionally changed the value of
> /sys/devices/.../power/control to "auto" to allow the driver to
> power manage the device at run time, the driver may confuse it by
> using pm_runtime_forbid() this way.
>
> So that comes with a warning that things might not work well after doing
> so.
>
> What comes to the driver code, I still wouldn't complicate it by attempting
> to make a driver work in such a case.

I think pm_runtime_forbid() and pm_runtime_enable() operate on
complete different data. That was exactly the source of my confusion
earlier. Looking at the code [1], even if runtime PM is "forbidden",
it is still "enabled" and just the usage count is incremented.

https://elixir.bootlin.com/linux/latest/source/drivers/base/power/runtime.c#L1314

Best regards,
Tomasz

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

[Sakari Ailus]

On Tue, Mar 06, 2018 at 06:28:43PM +0900, Tomasz Figa wrote:
> On Tue, Mar 6, 2018 at 6:18 PM, Sakari Ailus
>  wrote:
> > On Tue, Mar 06, 2018 at 05:51:36PM +0900, Tomasz Figa wrote:
> >> On Tue, Mar 6, 2018 at 5:40 PM, Sakari Ailus
> >>  wrote:
> >> > Hi Tomasz and Andy,
> >> >
> >> > On Sat, Mar 03, 2018 at 12:43:59AM +0900, Tomasz Figa wrote:
> >> > ...
> >> >> > +static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
> >> >> > +{
> >> >> > +   struct imx258 *imx258 =
> >> >> > +   container_of(ctrl->handler, struct imx258, 
> >> >> > ctrl_handler);
> >> >> > +   struct i2c_client *client = v4l2_get_subdevdata(>sd);
> >> >> > +   int ret = 0;
> >> >> > +   /*
> >> >> > +* Applying V4L2 control value only happens
> >> >> > +* when power is up for streaming
> >> >> > +*/
> >> >> > +   if (pm_runtime_get_if_in_use(>dev) <= 0)
> >> >> > +   return 0;
> >> >>
> >> >> I thought we decided to fix this to handle disabled runtime PM properly.
> >> >
> >> > Good point. I bet this is a problem in a few other drivers, too. How 
> >> > would
> >> > you fix that? Check for zero only?
> >> >
> >>
> >> bool need_runtime_put;
> >>
> >> ret = pm_runtime_get_if_in_use(>dev);
> >> if (ret <= 0 && ret != -EINVAL)
> >> return ret;
> >> need_runtime_put = ret > 0;
> >>
> >> // Do stuff ...
> >>
> >> if (need_runtime_put)
> >>pm_runtime_put(>dev);
> >>
> >> I don't like how ugly it is, but it appears to be the only way to
> >> handle this correctly.
> >
> > The driver enables runtime PM so if runtime PM is enabled in kernel
> > configuration, it is enabled here. In that case pm_runtime_get_if_in_use()
> > will return either 0 or 1. So as far as I can see, changing the lines to:
> >
> > if (!pm_runtime_get_if_in_use(>dev))
> > return 0;
> >
> > is enough.
> 
> Right, my bad. Somehow I was convinced that enable status can change at
> runtime.

Good point. I guess in principle this could happen although I can't see a
reason to do so, other than to break things --- quoting
Documentation/power/runtime_pm.txt:

The user space can effectively disallow the driver of the device to
power manage it at run time by changing the value of its
/sys/devices/.../power/control attribute to "on", which causes
pm_runtime_forbid() to be called. In principle, this mechanism may
also be used by the driver to effectively turn off the runtime
power management of the device until the user space turns it on.
Namely, during the initialization the driver can make sure that the
runtime PM status of the device is 'active' and call
pm_runtime_forbid(). It should be noted, however, that if the user
space has already intentionally changed the value of
/sys/devices/.../power/control to "auto" to allow the driver to
power manage the device at run time, the driver may confuse it by
using pm_runtime_forbid() this way.

So that comes with a warning that things might not work well after doing
so.

What comes to the driver code, I still wouldn't complicate it by attempting
to make a driver work in such a case.

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

Re: [PATCH 0/7] media: sun6i: Various fixes and improvements

[Maxime Ripard]

On Mon, Mar 05, 2018 at 06:35:35PM +0800, Yong wrote:
> > Hi Yong,
> > 
> > Here are a bunch of patches I came up with after testing your last
> > (v8) version of the CSI patches.
> > 
> > There's some improvements (patches 1 and 7) and fixes for
> > regressions found in the v8 compared to the v7 (patches 2, 3, 4 and
> > 5), and one fix that we discussed for the signals polarity for the
> > parallel interface (patch 6).
> > 
> > Feel free to squash them in your serie for the v9.
> 
> OK. Thank you!
> 
> I notice that your responses have not been listed in google group
> since February.

Yeah, I know, apparently I can't change my email address in google
groups, or unsubscribe, so I'm not subscribed with my new mail, which
means I can't post either.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature

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

[Tomasz Figa]

On Tue, Mar 6, 2018 at 6:18 PM, Sakari Ailus
 wrote:
> On Tue, Mar 06, 2018 at 05:51:36PM +0900, Tomasz Figa wrote:
>> On Tue, Mar 6, 2018 at 5:40 PM, Sakari Ailus
>>  wrote:
>> > Hi Tomasz and Andy,
>> >
>> > On Sat, Mar 03, 2018 at 12:43:59AM +0900, Tomasz Figa wrote:
>> > ...
>> >> > +static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
>> >> > +{
>> >> > +   struct imx258 *imx258 =
>> >> > +   container_of(ctrl->handler, struct imx258, 
>> >> > ctrl_handler);
>> >> > +   struct i2c_client *client = v4l2_get_subdevdata(>sd);
>> >> > +   int ret = 0;
>> >> > +   /*
>> >> > +* Applying V4L2 control value only happens
>> >> > +* when power is up for streaming
>> >> > +*/
>> >> > +   if (pm_runtime_get_if_in_use(>dev) <= 0)
>> >> > +   return 0;
>> >>
>> >> I thought we decided to fix this to handle disabled runtime PM properly.
>> >
>> > Good point. I bet this is a problem in a few other drivers, too. How would
>> > you fix that? Check for zero only?
>> >
>>
>> bool need_runtime_put;
>>
>> ret = pm_runtime_get_if_in_use(>dev);
>> if (ret <= 0 && ret != -EINVAL)
>> return ret;
>> need_runtime_put = ret > 0;
>>
>> // Do stuff ...
>>
>> if (need_runtime_put)
>>pm_runtime_put(>dev);
>>
>> I don't like how ugly it is, but it appears to be the only way to
>> handle this correctly.
>
> The driver enables runtime PM so if runtime PM is enabled in kernel
> configuration, it is enabled here. In that case pm_runtime_get_if_in_use()
> will return either 0 or 1. So as far as I can see, changing the lines to:
>
> if (!pm_runtime_get_if_in_use(>dev))
> return 0;
>
> is enough.

Right, my bad. Somehow I was convinced that enable status can change at runtime.

>
>> >> > +   ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
>> >> > +   if (ret)
>> >>
>> >> Missing error message.
>
> Same for this actually, printing an error message here isn't useful. It'd
> be just waiting for someone to clean it up. :-)

Fair enough.

>
>> >>
>> >> > +   return ret;
>> >> > +
>> >> > +   mutex_init(>mutex);
>> >> > +   ctrl_hdlr->lock = >mutex;
>> >> > +   imx258->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
>> >> > +   _ctrl_ops,
>> >> > +   V4L2_CID_LINK_FREQ,
>> >> > +   ARRAY_SIZE(link_freq_menu_items) - 1,
>> >> > +   0,
>> >> > +   link_freq_menu_items);
>> >> > +
>> >> > +   if (!imx258->link_freq) {
>> >> > +   ret = -EINVAL;
>> >>
>> >> Missing error message.
>> >
>> > I wouldn't add an error message here. Typically you'd need that information
>> > at development time only, never after that. v4l2_ctrl_new_int_menu(), as
>> > other control framework functions creating new controls, can fail due to
>> > memory allocation failure (which is already vocally reported) or due to bad
>> > parameters (that are constants).
>> >
>> > I'd rather do:
>> >
>> > if (!imx258->link_freq)
>> > ... |= ...;
>> >
>> > It simplifies error handling and removes the need for goto.
>>
>> Hmm, I'm not sure I understand your suggestion. Do you perhaps mean
>>
>> if (imx258->link_freq) {
>> // Do stuff that dereferences imx258->link_freq
>> }
>>
>> // ...
>>
>> if (ctrl_hdlr->error) {
>> // Check for error only here, at the end of control initialization.
>> }
>>
>> then it would be better indeed.
>
> Yes, indeed.

SGTM.

Best regards,
Tomasz

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

[Sakari Ailus]

On Tue, Mar 06, 2018 at 05:51:36PM +0900, Tomasz Figa wrote:
> On Tue, Mar 6, 2018 at 5:40 PM, Sakari Ailus
>  wrote:
> > Hi Tomasz and Andy,
> >
> > On Sat, Mar 03, 2018 at 12:43:59AM +0900, Tomasz Figa wrote:
> > ...
> >> > +static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
> >> > +{
> >> > +   struct imx258 *imx258 =
> >> > +   container_of(ctrl->handler, struct imx258, ctrl_handler);
> >> > +   struct i2c_client *client = v4l2_get_subdevdata(>sd);
> >> > +   int ret = 0;
> >> > +   /*
> >> > +* Applying V4L2 control value only happens
> >> > +* when power is up for streaming
> >> > +*/
> >> > +   if (pm_runtime_get_if_in_use(>dev) <= 0)
> >> > +   return 0;
> >>
> >> I thought we decided to fix this to handle disabled runtime PM properly.
> >
> > Good point. I bet this is a problem in a few other drivers, too. How would
> > you fix that? Check for zero only?
> >
> 
> bool need_runtime_put;
> 
> ret = pm_runtime_get_if_in_use(>dev);
> if (ret <= 0 && ret != -EINVAL)
> return ret;
> need_runtime_put = ret > 0;
> 
> // Do stuff ...
> 
> if (need_runtime_put)
>pm_runtime_put(>dev);
> 
> I don't like how ugly it is, but it appears to be the only way to
> handle this correctly.

The driver enables runtime PM so if runtime PM is enabled in kernel
configuration, it is enabled here. In that case pm_runtime_get_if_in_use()
will return either 0 or 1. So as far as I can see, changing the lines to:

if (!pm_runtime_get_if_in_use(>dev))
return 0;

is enough.

> >> > +   ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
> >> > +   if (ret)
> >>
> >> Missing error message.

Same for this actually, printing an error message here isn't useful. It'd
be just waiting for someone to clean it up. :-)

> >>
> >> > +   return ret;
> >> > +
> >> > +   mutex_init(>mutex);
> >> > +   ctrl_hdlr->lock = >mutex;
> >> > +   imx258->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
> >> > +   _ctrl_ops,
> >> > +   V4L2_CID_LINK_FREQ,
> >> > +   ARRAY_SIZE(link_freq_menu_items) - 1,
> >> > +   0,
> >> > +   link_freq_menu_items);
> >> > +
> >> > +   if (!imx258->link_freq) {
> >> > +   ret = -EINVAL;
> >>
> >> Missing error message.
> >
> > I wouldn't add an error message here. Typically you'd need that information
> > at development time only, never after that. v4l2_ctrl_new_int_menu(), as
> > other control framework functions creating new controls, can fail due to
> > memory allocation failure (which is already vocally reported) or due to bad
> > parameters (that are constants).
> >
> > I'd rather do:
> >
> > if (!imx258->link_freq)
> > ... |= ...;
> >
> > It simplifies error handling and removes the need for goto.
> 
> Hmm, I'm not sure I understand your suggestion. Do you perhaps mean
> 
> if (imx258->link_freq) {
> // Do stuff that dereferences imx258->link_freq
> }
> 
> // ...
> 
> if (ctrl_hdlr->error) {
> // Check for error only here, at the end of control initialization.
> }
> 
> then it would be better indeed.

Yes, indeed.

-- 
Regards,

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

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

[Tomasz Figa]

On Tue, Mar 6, 2018 at 5:40 PM, Sakari Ailus
 wrote:
> Hi Tomasz and Andy,
>
> On Sat, Mar 03, 2018 at 12:43:59AM +0900, Tomasz Figa wrote:
> ...
>> > +static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
>> > +{
>> > +   struct imx258 *imx258 =
>> > +   container_of(ctrl->handler, struct imx258, ctrl_handler);
>> > +   struct i2c_client *client = v4l2_get_subdevdata(>sd);
>> > +   int ret = 0;
>> > +   /*
>> > +* Applying V4L2 control value only happens
>> > +* when power is up for streaming
>> > +*/
>> > +   if (pm_runtime_get_if_in_use(>dev) <= 0)
>> > +   return 0;
>>
>> I thought we decided to fix this to handle disabled runtime PM properly.
>
> Good point. I bet this is a problem in a few other drivers, too. How would
> you fix that? Check for zero only?
>

bool need_runtime_put;

ret = pm_runtime_get_if_in_use(>dev);
if (ret <= 0 && ret != -EINVAL)
return ret;
need_runtime_put = ret > 0;

// Do stuff ...

if (need_runtime_put)
   pm_runtime_put(>dev);

I don't like how ugly it is, but it appears to be the only way to
handle this correctly.

> ...
>
>> [snip]
>> > +/* Initialize control handlers */
>> > +static int imx258_init_controls(struct imx258 *imx258)
>> > +{
>> > +   struct i2c_client *client = v4l2_get_subdevdata(>sd);
>> > +   struct v4l2_ctrl_handler *ctrl_hdlr;
>> > +   s64 exposure_max;
>> > +   s64 vblank_def;
>> > +   s64 vblank_min;
>> > +   s64 pixel_rate_min;
>> > +   s64 pixel_rate_max;
>> > +   int ret;
>> > +
>> > +   ctrl_hdlr = >ctrl_handler;
>> > +   ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
>> > +   if (ret)
>>
>> Missing error message.
>>
>> > +   return ret;
>> > +
>> > +   mutex_init(>mutex);
>> > +   ctrl_hdlr->lock = >mutex;
>> > +   imx258->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
>> > +   _ctrl_ops,
>> > +   V4L2_CID_LINK_FREQ,
>> > +   ARRAY_SIZE(link_freq_menu_items) - 1,
>> > +   0,
>> > +   link_freq_menu_items);
>> > +
>> > +   if (!imx258->link_freq) {
>> > +   ret = -EINVAL;
>>
>> Missing error message.
>
> I wouldn't add an error message here. Typically you'd need that information
> at development time only, never after that. v4l2_ctrl_new_int_menu(), as
> other control framework functions creating new controls, can fail due to
> memory allocation failure (which is already vocally reported) or due to bad
> parameters (that are constants).
>
> I'd rather do:
>
> if (!imx258->link_freq)
> ... |= ...;
>
> It simplifies error handling and removes the need for goto.

Hmm, I'm not sure I understand your suggestion. Do you perhaps mean

if (imx258->link_freq) {
// Do stuff that dereferences imx258->link_freq
}

// ...

if (ctrl_hdlr->error) {
// Check for error only here, at the end of control initialization.
}

then it would be better indeed.

>
>>
>> > +   goto error;
>> > +   }
>> > +
>> > +   imx258->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>> > +
>> > +   pixel_rate_max = link_freq_to_pixel_rate(link_freq_menu_items[0]);
>> > +   pixel_rate_min = link_freq_to_pixel_rate(link_freq_menu_items[1]);
>> > +   /* By default, PIXEL_RATE is read only */
>> > +   imx258->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, _ctrl_ops,
>> > +   V4L2_CID_PIXEL_RATE,
>> > +   pixel_rate_min, pixel_rate_max,
>> > +   1, pixel_rate_max);
>> > +
>> > +
>> > +   vblank_def = imx258->cur_mode->vts_def - imx258->cur_mode->height;
>> > +   vblank_min = imx258->cur_mode->vts_min - imx258->cur_mode->height;
>> > +   imx258->vblank = v4l2_ctrl_new_std(
>> > +   ctrl_hdlr, _ctrl_ops, 
>> > V4L2_CID_VBLANK,
>> > +   vblank_min,
>> > +   IMX258_VTS_MAX - imx258->cur_mode->height, 
>> > 1,
>> > +   vblank_def);
>> > +
>> > +   imx258->hblank = v4l2_ctrl_new_std(
>> > +   ctrl_hdlr, _ctrl_ops, 
>> > V4L2_CID_HBLANK,
>> > +   IMX258_PPL_650MHZ - 
>> > imx258->cur_mode->width,
>> > +   IMX258_PPL_650MHZ - 
>> > imx258->cur_mode->width,
>> > +   1,
>> > +   IMX258_PPL_650MHZ - 
>> > imx258->cur_mode->width);
>> > +
>> > +   if (!imx258->hblank) {
>> > +   ret = -EINVAL;
>> > +   goto error;
>> > +   }
>>
>> Why checking hblank, but not other controls? I think in this case just
>> the general check for ctrl_hdlr->error should be enough.
>
> There's no need to access most other controls (except blank and 

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

[Sakari Ailus]

Hi Tomasz and Andy,

On Sat, Mar 03, 2018 at 12:43:59AM +0900, Tomasz Figa wrote:
...
> > +static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +   struct imx258 *imx258 =
> > +   container_of(ctrl->handler, struct imx258, ctrl_handler);
> > +   struct i2c_client *client = v4l2_get_subdevdata(>sd);
> > +   int ret = 0;
> > +   /*
> > +* Applying V4L2 control value only happens
> > +* when power is up for streaming
> > +*/
> > +   if (pm_runtime_get_if_in_use(>dev) <= 0)
> > +   return 0;
> 
> I thought we decided to fix this to handle disabled runtime PM properly.

Good point. I bet this is a problem in a few other drivers, too. How would
you fix that? Check for zero only?

...

> [snip]
> > +/* Initialize control handlers */
> > +static int imx258_init_controls(struct imx258 *imx258)
> > +{
> > +   struct i2c_client *client = v4l2_get_subdevdata(>sd);
> > +   struct v4l2_ctrl_handler *ctrl_hdlr;
> > +   s64 exposure_max;
> > +   s64 vblank_def;
> > +   s64 vblank_min;
> > +   s64 pixel_rate_min;
> > +   s64 pixel_rate_max;
> > +   int ret;
> > +
> > +   ctrl_hdlr = >ctrl_handler;
> > +   ret = v4l2_ctrl_handler_init(ctrl_hdlr, 8);
> > +   if (ret)
> 
> Missing error message.
> 
> > +   return ret;
> > +
> > +   mutex_init(>mutex);
> > +   ctrl_hdlr->lock = >mutex;
> > +   imx258->link_freq = v4l2_ctrl_new_int_menu(ctrl_hdlr,
> > +   _ctrl_ops,
> > +   V4L2_CID_LINK_FREQ,
> > +   ARRAY_SIZE(link_freq_menu_items) - 1,
> > +   0,
> > +   link_freq_menu_items);
> > +
> > +   if (!imx258->link_freq) {
> > +   ret = -EINVAL;
> 
> Missing error message.

I wouldn't add an error message here. Typically you'd need that information
at development time only, never after that. v4l2_ctrl_new_int_menu(), as
other control framework functions creating new controls, can fail due to
memory allocation failure (which is already vocally reported) or due to bad
parameters (that are constants).

I'd rather do:

if (!imx258->link_freq)
... |= ...;

It simplifies error handling and removes the need for goto.

> 
> > +   goto error;
> > +   }
> > +
> > +   imx258->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > +   pixel_rate_max = link_freq_to_pixel_rate(link_freq_menu_items[0]);
> > +   pixel_rate_min = link_freq_to_pixel_rate(link_freq_menu_items[1]);
> > +   /* By default, PIXEL_RATE is read only */
> > +   imx258->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, _ctrl_ops,
> > +   V4L2_CID_PIXEL_RATE,
> > +   pixel_rate_min, pixel_rate_max,
> > +   1, pixel_rate_max);
> > +
> > +
> > +   vblank_def = imx258->cur_mode->vts_def - imx258->cur_mode->height;
> > +   vblank_min = imx258->cur_mode->vts_min - imx258->cur_mode->height;
> > +   imx258->vblank = v4l2_ctrl_new_std(
> > +   ctrl_hdlr, _ctrl_ops, 
> > V4L2_CID_VBLANK,
> > +   vblank_min,
> > +   IMX258_VTS_MAX - imx258->cur_mode->height, 
> > 1,
> > +   vblank_def);
> > +
> > +   imx258->hblank = v4l2_ctrl_new_std(
> > +   ctrl_hdlr, _ctrl_ops, 
> > V4L2_CID_HBLANK,
> > +   IMX258_PPL_650MHZ - imx258->cur_mode->width,
> > +   IMX258_PPL_650MHZ - imx258->cur_mode->width,
> > +   1,
> > +   IMX258_PPL_650MHZ - 
> > imx258->cur_mode->width);
> > +
> > +   if (!imx258->hblank) {
> > +   ret = -EINVAL;
> > +   goto error;
> > +   }
> 
> Why checking hblank, but not other controls? I think in this case just
> the general check for ctrl_hdlr->error should be enough.

There's no need to access most other controls (except blank and link_freq).
The flags field is set for hblank below, therefore the check.

> 
> > +
> > +   imx258->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > +   exposure_max = imx258->cur_mode->vts_def - 8;
> > +   imx258->exposure = v4l2_ctrl_new_std(
> > +   ctrl_hdlr, _ctrl_ops,
> > +   V4L2_CID_EXPOSURE, IMX258_EXPOSURE_MIN,
> > +   IMX258_EXPOSURE_MAX, IMX258_EXPOSURE_STEP,
> > +   IMX258_EXPOSURE_DEFAULT);
> > +
> > +   v4l2_ctrl_new_std(ctrl_hdlr, _ctrl_ops, 
> > V4L2_CID_ANALOGUE_GAIN,
> > +   IMX258_ANA_GAIN_MIN, IMX258_ANA_GAIN_MAX,
> > +   IMX258_ANA_GAIN_STEP, 
> > IMX258_ANA_GAIN_DEFAULT);
> > +
> > +