cron job: media_tree daily build: WARNINGS

[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:   Fri Sep 22 05:00:16 CEST 2017
media-tree git hash:1efdf1776e2253b77413c997bed862410e4b6aaf
media_build git hash:   19087750b61fc0c5528e798c47ff845f9234
v4l-utils git hash: 9ee29df352dad950784f0f6f4a1bb96c0aefacc4
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.12.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: OK
linux-git-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: WARNINGS
linux-2.6.37.6-i686: WARNINGS
linux-2.6.38.8-i686: WARNINGS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: WARNINGS
linux-3.2.37-i686: WARNINGS
linux-3.3.8-i686: WARNINGS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: WARNINGS
linux-3.12.67-i686: WARNINGS
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-4.13-i686: OK
linux-2.6.36.4-x86_64: WARNINGS
linux-2.6.37.6-x86_64: WARNINGS
linux-2.6.38.8-x86_64: WARNINGS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: WARNINGS
linux-3.2.37-x86_64: WARNINGS
linux-3.3.8-x86_64: WARNINGS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: OK
apps: OK
spec-git: OK

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

[PATCH] media: bt8xx: Fix err 'bt878_probe()'

[Christophe JAILLET]

This is odd to call 'pci_disable_device()' in an error path before a
coresponding successful 'pci_enable_device()'.

Return directly instead.

Fixes: 77e0be12100a ("V4L/DVB (4176): Bug-fix: Fix memory overflow")
Signed-off-by: Christophe JAILLET 
---
 drivers/media/pci/bt8xx/bt878.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/pci/bt8xx/bt878.c b/drivers/media/pci/bt8xx/bt878.c
index a5f52137d306..d4bc78b4fcb5 100644
--- a/drivers/media/pci/bt8xx/bt878.c
+++ b/drivers/media/pci/bt8xx/bt878.c
@@ -422,8 +422,7 @@ static int bt878_probe(struct pci_dev *dev, const struct 
pci_device_id *pci_id)
   bt878_num);
if (bt878_num >= BT878_MAX) {
printk(KERN_ERR "bt878: Too many devices inserted\n");
-   result = -ENOMEM;
-   goto fail0;
+   return -ENOMEM;
}
if (pci_enable_device(dev))
return -EIO;
-- 
2.11.0

Re: [PATCH 00/11] drm/sun4i: add CEC support

[Hans Verkuil]

On 21/09/17 22:37, Maxime Ripard wrote:
> Hi Hans,
> 
> On Fri, Sep 08, 2017 at 10:59:44AM +, Hans Verkuil wrote:
>> Hi Maxime,
>>
>> On 07/18/17 18:29, Maxime Ripard wrote:
>>> Hi,
>>>
>>> On Tue, Jul 11, 2017 at 11:06:52PM +0200, Hans Verkuil wrote:
 On 11/07/17 22:39, Maxime Ripard wrote:
> On Tue, Jul 11, 2017 at 08:30:33AM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> This patch series adds CEC support for the sun4i HDMI controller.
>>
>> The CEC hardware support for the A10 is very low-level as it just
>> controls the CEC pin. Since I also wanted to support GPIO-based CEC
>> hardware most of this patch series is in the CEC framework to
>> add a generic low-level CEC pin framework. It is only the final patch
>> that adds the sun4i support.
>>
>> This patch series first makes some small changes in the CEC framework
>> (patches 1-4) to prepare for this CEC pin support.
>>
>> Patch 5-7 adds the new API elements and documents it. Patch 6 reworks
>> the CEC core event handling.
>>
>> Patch 8 adds pin monitoring support (allows userspace to see all
>> CEC pin transitions as they happen).
>>
>> Patch 9 adds the core cec-pin implementation that translates low-level
>> pin transitions into valid CEC messages. Basically this does what any
>> SoC with a proper CEC hardware implementation does.
>>
>> Patch 10 documents the cec-pin kAPI (and also the cec-notifier kAPI
>> which was missing).
>>
>> Finally patch 11 adds the actual sun4i_hdmi CEC implementation.
>>
>> I tested this on my cubieboard. There were no errors at all
>> after 126264 calls of 'cec-ctl --give-device-vendor-id' while at the
>> same time running a 'make -j4' of the v4l-utils git repository and
>> doing a continuous scp to create network traffic.
>>
>> This patch series is based on top of the mainline kernel as of
>> yesterday (so with all the sun4i and cec patches for 4.13 merged).
>
> For the whole serie:
> Reviewed-by: Maxime Ripard 
>
>> Maxime, patches 1-10 will go through the media subsystem. How do you
>> want to handle the final patch? It can either go through the media
>> subsystem as well, or you can sit on it and handle this yourself during
>> the 4.14 merge window. Another option is to separate the Kconfig change
>> into its own patch. That way you can merge the code changes and only
>> have to handle the Kconfig patch as a final change during the merge
>> window.
>
> We'll probably have a number of reworks for 4.14, so it would be
> better if I merged it.
>
> However, I guess if we just switch to a depends on CEC_PIN instead of
> a select, everything would just work even if we merge your patches in
> a separate tree, right?

 This small patch will do it:

 diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
 index e884d265c0b3..ebad80aefc87 100644
 --- a/drivers/gpu/drm/sun4i/Kconfig
 +++ b/drivers/gpu/drm/sun4i/Kconfig
 @@ -25,7 +25,7 @@ config DRM_SUN4I_HDMI_CEC
 bool "Allwinner A10 HDMI CEC Support"
 depends on DRM_SUN4I_HDMI
 select CEC_CORE
 -   select CEC_PIN
 +   depends on CEC_PIN
 help
  Choose this option if you have an Allwinner SoC with an HDMI
  controller and want to use CEC.
>>
>> Just a reminder: now that both this driver and the CEC_PIN code has been
>> merged in 4.14, this 'depends on' can become a 'select' again.
> 
> Thanks for the reminder.
> 
> Would that commit work for you:
> http://code.bulix.org/19o9y6-201254

Acked-by: Hans Verkuil 

This obviously should be merged for 4.14.

Regards,

Hans

> 
> Maxime
> 

Re: [PATCH 00/11] drm/sun4i: add CEC support

[Maxime Ripard]

Hi Hans,

On Fri, Sep 08, 2017 at 10:59:44AM +, Hans Verkuil wrote:
> Hi Maxime,
> 
> On 07/18/17 18:29, Maxime Ripard wrote:
> > Hi,
> > 
> > On Tue, Jul 11, 2017 at 11:06:52PM +0200, Hans Verkuil wrote:
> >> On 11/07/17 22:39, Maxime Ripard wrote:
> >>> On Tue, Jul 11, 2017 at 08:30:33AM +0200, Hans Verkuil wrote:
>  From: Hans Verkuil 
> 
>  This patch series adds CEC support for the sun4i HDMI controller.
> 
>  The CEC hardware support for the A10 is very low-level as it just
>  controls the CEC pin. Since I also wanted to support GPIO-based CEC
>  hardware most of this patch series is in the CEC framework to
>  add a generic low-level CEC pin framework. It is only the final patch
>  that adds the sun4i support.
> 
>  This patch series first makes some small changes in the CEC framework
>  (patches 1-4) to prepare for this CEC pin support.
> 
>  Patch 5-7 adds the new API elements and documents it. Patch 6 reworks
>  the CEC core event handling.
> 
>  Patch 8 adds pin monitoring support (allows userspace to see all
>  CEC pin transitions as they happen).
> 
>  Patch 9 adds the core cec-pin implementation that translates low-level
>  pin transitions into valid CEC messages. Basically this does what any
>  SoC with a proper CEC hardware implementation does.
> 
>  Patch 10 documents the cec-pin kAPI (and also the cec-notifier kAPI
>  which was missing).
> 
>  Finally patch 11 adds the actual sun4i_hdmi CEC implementation.
> 
>  I tested this on my cubieboard. There were no errors at all
>  after 126264 calls of 'cec-ctl --give-device-vendor-id' while at the
>  same time running a 'make -j4' of the v4l-utils git repository and
>  doing a continuous scp to create network traffic.
> 
>  This patch series is based on top of the mainline kernel as of
>  yesterday (so with all the sun4i and cec patches for 4.13 merged).
> >>>
> >>> For the whole serie:
> >>> Reviewed-by: Maxime Ripard 
> >>>
>  Maxime, patches 1-10 will go through the media subsystem. How do you
>  want to handle the final patch? It can either go through the media
>  subsystem as well, or you can sit on it and handle this yourself during
>  the 4.14 merge window. Another option is to separate the Kconfig change
>  into its own patch. That way you can merge the code changes and only
>  have to handle the Kconfig patch as a final change during the merge
>  window.
> >>>
> >>> We'll probably have a number of reworks for 4.14, so it would be
> >>> better if I merged it.
> >>>
> >>> However, I guess if we just switch to a depends on CEC_PIN instead of
> >>> a select, everything would just work even if we merge your patches in
> >>> a separate tree, right?
> >>
> >> This small patch will do it:
> >>
> >> diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
> >> index e884d265c0b3..ebad80aefc87 100644
> >> --- a/drivers/gpu/drm/sun4i/Kconfig
> >> +++ b/drivers/gpu/drm/sun4i/Kconfig
> >> @@ -25,7 +25,7 @@ config DRM_SUN4I_HDMI_CEC
> >> bool "Allwinner A10 HDMI CEC Support"
> >> depends on DRM_SUN4I_HDMI
> >> select CEC_CORE
> >> -   select CEC_PIN
> >> +   depends on CEC_PIN
> >> help
> >>  Choose this option if you have an Allwinner SoC with an HDMI
> >>  controller and want to use CEC.
> 
> Just a reminder: now that both this driver and the CEC_PIN code has been
> merged in 4.14, this 'depends on' can become a 'select' again.

Thanks for the reminder.

Would that commit work for you:
http://code.bulix.org/19o9y6-201254

Maxime

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


signature.asc
Description: PGP signature

[PATCH 3/3] [media] uvcvideo: Add some spaces for better code readability

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 21:12:29 +0200

Use space characters at some source code places according to
the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/uvc/uvc_v4l2.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 184edf8a0885..cebaba5c4e86 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -123,13 +123,13 @@ static __u32 uvc_try_frame_interval(struct uvc_frame 
*frame, __u32 interval)
best = dist;
}
 
-   interval = frame->dwFrameInterval[i-1];
+   interval = frame->dwFrameInterval[i - 1];
} else {
const __u32 min = frame->dwFrameInterval[0];
const __u32 max = frame->dwFrameInterval[1];
const __u32 step = frame->dwFrameInterval[2];
 
-   interval = min + (interval - min + step/2) / step * step;
+   interval = min + (interval - min + step / 2) / step * step;
if (interval > max)
interval = max;
}
@@ -201,7 +201,7 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
__u16 h = format->frame[i].wHeight;
 
d = min(w, rw) * min(h, rh);
-   d = w*h + rw*rh - 2*d;
+   d = w * h + rw * rh - 2 * d;
if (d < maxd) {
maxd = d;
frame = >frame[i];
@@ -219,9 +219,10 @@ static int uvc_v4l2_try_format(struct uvc_streaming 
*stream,
 
/* Use the default frame interval. */
interval = frame->dwDefaultFrameInterval;
-   uvc_trace(UVC_TRACE_FORMAT, "Using default frame interval %u.%u us "
-   "(%u.%u fps).\n", interval/10, interval%10, 1000/interval,
-   (1/interval)%10);
+   uvc_trace(UVC_TRACE_FORMAT,
+ "Using default frame interval %u.%u us (%u.%u fps).\n",
+ interval / 10, interval % 10,
+ 1000 / interval, (1 / interval) % 10);
 
/* Set the format index, frame index and frame interval. */
memset(probe, 0, sizeof *probe);
-- 
2.14.1

[PATCH 2/3] [media] uvcvideo: Adjust 14 checks for null pointers

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 21:00:21 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written …

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/uvc/uvc_v4l2.c | 27 +--
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 6ec2b255c44a..184edf8a0885 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -41,7 +41,7 @@ static int uvc_ioctl_ctrl_map(struct uvc_video_chain *chain,
int ret;
 
map = kzalloc(sizeof *map, GFP_KERNEL);
-   if (map == NULL)
+   if (!map)
return -ENOMEM;
 
map->id = xmap->id;
@@ -211,7 +211,7 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
break;
}
 
-   if (frame == NULL) {
+   if (!frame) {
uvc_trace(UVC_TRACE_FORMAT, "Unsupported size %ux%u.\n",
fmt->fmt.pix.width, fmt->fmt.pix.height);
return -EINVAL;
@@ -260,9 +260,9 @@ static int uvc_v4l2_try_format(struct uvc_streaming *stream,
fmt->fmt.pix.colorspace = format->colorspace;
fmt->fmt.pix.priv = 0;
 
-   if (uvc_format != NULL)
+   if (uvc_format)
*uvc_format = format;
-   if (uvc_frame != NULL)
+   if (uvc_frame)
*uvc_frame = frame;
 
 done:
@@ -282,8 +282,7 @@ static int uvc_v4l2_get_format(struct uvc_streaming *stream,
mutex_lock(>mutex);
format = stream->cur_format;
frame = stream->cur_frame;
-
-   if (format == NULL || frame == NULL) {
+   if (!format || !frame) {
ret = -EINVAL;
goto done;
}
@@ -499,7 +498,7 @@ static int uvc_v4l2_open(struct file *file)
 
/* Create the device handle. */
handle = kzalloc(sizeof *handle, GFP_KERNEL);
-   if (handle == NULL) {
+   if (!handle) {
usb_autopm_put_interface(stream->dev->intf);
return -ENOMEM;
}
@@ -811,7 +810,7 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
u32 index = input->index;
int pin = 0;
 
-   if (selector == NULL ||
+   if (!selector ||
(chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
if (index != 0)
return -EINVAL;
@@ -830,7 +829,7 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
}
}
 
-   if (iterm == NULL || iterm->id != pin)
+   if (!iterm || iterm->id != pin)
return -EINVAL;
 
memset(input, 0, sizeof(*input));
@@ -849,7 +848,7 @@ static int uvc_ioctl_g_input(struct file *file, void *fh, 
unsigned int *input)
int ret;
u8 i;
 
-   if (chain->selector == NULL ||
+   if (!chain->selector ||
(chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
*input = 0;
return 0;
@@ -876,7 +875,7 @@ static int uvc_ioctl_s_input(struct file *file, void *fh, 
unsigned int input)
if (ret < 0)
return ret;
 
-   if (chain->selector == NULL ||
+   if (!chain->selector ||
(chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
if (input)
return -EINVAL;
@@ -1160,7 +1159,7 @@ static int uvc_ioctl_enum_framesizes(struct file *file, 
void *fh,
break;
}
}
-   if (format == NULL)
+   if (!format)
return -EINVAL;
 
if (fsize->index >= format->nframes)
@@ -1189,7 +1188,7 @@ static int uvc_ioctl_enum_frameintervals(struct file 
*file, void *fh,
break;
}
}
-   if (format == NULL)
+   if (!format)
return -EINVAL;
 
for (i = 0; i < format->nframes; i++) {
@@ -1199,7 +1198,7 @@ static int uvc_ioctl_enum_frameintervals(struct file 
*file, void *fh,
break;
}
}
-   if (frame == NULL)
+   if (!frame)
return -EINVAL;
 
if (frame->bFrameIntervalType) {
-- 
2.14.1

[PATCH 1/3] [media] uvcvideo: Use common error handling code in uvc_ioctl_g_ext_ctrls()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 20:47:02 +0200

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/uvc/uvc_v4l2.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 3e7e283a44a8..6ec2b255c44a 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -998,10 +998,8 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, void 
*fh,
struct v4l2_queryctrl qc = { .id = ctrl->id };
 
ret = uvc_query_v4l2_ctrl(chain, );
-   if (ret < 0) {
-   ctrls->error_idx = i;
-   return ret;
-   }
+   if (ret < 0)
+   goto set_index;
 
ctrl->value = qc.default_value;
}
@@ -1017,14 +1015,17 @@ static int uvc_ioctl_g_ext_ctrls(struct file *file, 
void *fh,
ret = uvc_ctrl_get(chain, ctrl);
if (ret < 0) {
uvc_ctrl_rollback(handle);
-   ctrls->error_idx = i;
-   return ret;
+   goto set_index;
}
}
 
ctrls->error_idx = 0;
 
return uvc_ctrl_rollback(handle);
+
+set_index:
+   ctrls->error_idx = i;
+   return ret;
 }
 
 static int uvc_ioctl_s_try_ext_ctrls(struct uvc_fh *handle,
-- 
2.14.1

[PATCH 0/3] [media] uvcvideo: Fine-tuning for some function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 21:20:12 +0200

Three update suggestions were taken into account
from static source code analysis.

Markus Elfring (3):
  Use common error handling code in uvc_ioctl_g_ext_ctrls()
  Adjust 14 checks for null pointers
  Add some spaces for better code readability

 drivers/media/usb/uvc/uvc_v4l2.c | 53 
 1 file changed, 27 insertions(+), 26 deletions(-)

-- 
2.14.1

Re: [PATCH] ov9655: fix potential integer overflow

[kbuild test robot]

Hi Gustavo,

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

url:
https://github.com/0day-ci/linux/commits/Gustavo-A-R-Silva/ov9655-fix-potential-integer-overflow/20170921-174735
base:   git://linuxtv.org/media_tree.git master
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> ERROR: "__aeabi_uldivmod" [drivers/media/i2c/ov9650.ko] undefined!

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


.config.gz
Description: application/gzip

Re: [PATCH v3 2/2] media: rc: Add driver for tango HW IR decoder

[Måns Rullgård]

Marc Gonzalez  writes:

> On 21/09/2017 17:46, Måns Rullgård wrote:
>
>> Marc Gonzalez writes:
>> 
>>> From: Mans Rullgard 
>>>
>>> The tango HW IR decoder supports NEC, RC-5, RC-6 protocols.
>>>
>>> Signed-off-by: Marc Gonzalez 
>> 
>> Have you been able to test all the protocols?  Universal remotes usually
>> support something or other with each of them.
>
> I found the Great Pile of Remotes locked away in a drawer.
> Played "What kind of batteries do you eat?" for about an hour.
> And found several NEC remotes, one RC-5, and one RC-6.
> Repeats seem to be handled differently than for NEC.
> I'll take a closer look.

That's not surprising, seeing as the way repeats are transmitted differs
between protocols.

If you're new to IR remote controls, this site has some good
information:
http://www.sbprojects.com/knowledge/ir/index.php

-- 
Måns Rullgård

Re: [PATCH v3 2/2] media: rc: Add driver for tango HW IR decoder

[Måns Rullgård]

Marc Gonzalez  writes:

> On 21/09/2017 17:57, Sean Young wrote:
>
>> On Thu, Sep 21, 2017 at 04:49:53PM +0200, Marc Gonzalez wrote:
>> 
>>> The tango HW IR decoder supports NEC, RC-5, RC-6 protocols.
>>>
>>> Signed-off-by: Marc Gonzalez 
>> 
>> Missing signed-off-by.
>
> I am aware of that. Hopefully, at some point, Mans will add his.

I will once I'm happy with it.  Shouldn't be far off.

-- 
Måns Rullgård

Re: [PATCH v3 2/2] media: rc: Add driver for tango HW IR decoder

[Sean Young]

On Thu, Sep 21, 2017 at 06:20:28PM +0200, Marc Gonzalez wrote:
> On 21/09/2017 17:57, Sean Young wrote:
> 
> > On Thu, Sep 21, 2017 at 04:49:53PM +0200, Marc Gonzalez wrote:
> > 
> >> The tango HW IR decoder supports NEC, RC-5, RC-6 protocols.
> >>
> >> Signed-off-by: Marc Gonzalez 
> > 
> > Missing signed-off-by.
> 
> I am aware of that. Hopefully, at some point, Mans will add his.
> I have no control over this, unless I rewrite the driver from
> scratch.
> 
> > Your patch still gives numerous checkpatch warnings, please run it
> > preferaby with --strict.
> 
> Some checkpatch warnings are silly, such as unconditionally mandating
> 4 lines for a Kconfig help message. Do you consider it mandatory to
> address all warnings, whatever they are?

Yes, they're mandatory.

The Kconfig could state what the module name is, which is helpful.

See:

https://www.kernel.org/doc/html/v4.12/process/submitting-patches.html

https://www.kernel.org/doc/html/v4.12/process/coding-style.html


Sean

Re: [PATCH v3 2/2] media: rc: Add driver for tango HW IR decoder

[Marc Gonzalez]

On 21/09/2017 17:57, Sean Young wrote:

> On Thu, Sep 21, 2017 at 04:49:53PM +0200, Marc Gonzalez wrote:
> 
>> The tango HW IR decoder supports NEC, RC-5, RC-6 protocols.
>>
>> Signed-off-by: Marc Gonzalez 
> 
> Missing signed-off-by.

I am aware of that. Hopefully, at some point, Mans will add his.
I have no control over this, unless I rewrite the driver from
scratch.

> Your patch still gives numerous checkpatch warnings, please run it
> preferaby with --strict.

Some checkpatch warnings are silly, such as unconditionally mandating
4 lines for a Kconfig help message. Do you consider it mandatory to
address all warnings, whatever they are?

Regards.

Re: [PATCH v3 2/2] media: rc: Add driver for tango HW IR decoder

[Marc Gonzalez]

On 21/09/2017 17:46, Måns Rullgård wrote:

> Marc Gonzalez writes:
> 
>> From: Mans Rullgard 
>>
>> The tango HW IR decoder supports NEC, RC-5, RC-6 protocols.
>>
>> Signed-off-by: Marc Gonzalez 
> 
> Have you been able to test all the protocols?  Universal remotes usually
> support something or other with each of them.

I found the Great Pile of Remotes locked away in a drawer.
Played "What kind of batteries do you eat?" for about an hour.
And found several NEC remotes, one RC-5, and one RC-6.
Repeats seem to be handled differently than for NEC.
I'll take a closer look.

>> +err = devm_request_irq(dev, irq, tango_ir_irq, IRQF_SHARED, 
>> dev_name(dev), ir);
>> +if (err)
>> +return err;
> 
> You shouldn't enable the irq until after you've configured the device.
> Otherwise you have no idea what state it's in, and it might start firing
> unexpectedly.
> 
> My original code did this properly.  Why did you move it?

I got caught up in the great devm rewrite.
Will take another swipe at it on Monday.

>> +writel_relaxed(0x110, ir->rc5_base + IR_CTRL);
> 
> Since you've defined DISABLE_NEC above, I think you should use it here too.

OK.

Regards.

Re: [PATCH 17/25] media: dvb_frontend: dtv_property_process_set() cleanups

[Mauro Carvalho Chehab]

Em Thu, 21 Sep 2017 08:32:25 -0600
Shuah Khan  escreveu:

> On 09/20/2017 01:11 PM, Mauro Carvalho Chehab wrote:
> > From: Satendra Singh Thakur 
> > 
> > Since all properties in the func dtv_property_process_set() use
> > at most 4 bytes arguments, change the code to pass
> > u32 cmd and u32 data as function arguments, instead of passing a
> > pointer to the entire struct dtv_property *tvp.
> > 
> > Instead of having a generic dtv_property_dump(), added its own
> > properties debug logic at dtv_property_process_set().  
> 
> Nit: in the dtv_property_process_set()
> 
> > 
> > Signed-off-by: Satendra Singh Thakur 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  drivers/media/dvb-core/dvb_frontend.c | 125 
> > --
> >  1 file changed, 72 insertions(+), 53 deletions(-)
> > 
> > diff --git a/drivers/media/dvb-core/dvb_frontend.c 
> > b/drivers/media/dvb-core/dvb_frontend.c
> > index bd60a490ce0f..b7094c7a405f 100644
> > --- a/drivers/media/dvb-core/dvb_frontend.c
> > +++ b/drivers/media/dvb-core/dvb_frontend.c
> > @@ -1107,22 +1107,19 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 
> > 1] = {
> > _DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
> >  };
> >  
> > -static void dtv_property_dump(struct dvb_frontend *fe,
> > - bool is_set,
> > +static void dtv_get_property_dump(struct dvb_frontend *fe,
> >   struct dtv_property *tvp)
> >  {
> > int i;
> >  
> > if (tvp->cmd <= 0 || tvp->cmd > DTV_MAX_COMMAND) {
> > -   dev_warn(fe->dvb->device, "%s: %s tvp.cmd = 0x%08x undefined\n",
> > -   __func__,
> > -   is_set ? "SET" : "GET",
> > +   dev_warn(fe->dvb->device, "%s: GET tvp.cmd = 0x%08x undefined\n"
> > +   , __func__,
> > tvp->cmd);
> > return;
> > }
> >  
> > -   dev_dbg(fe->dvb->device, "%s: %s tvp.cmd= 0x%08x (%s)\n", __func__,
> > -   is_set ? "SET" : "GET",
> > +   dev_dbg(fe->dvb->device, "%s: GET tvp.cmd= 0x%08x (%s)\n", __func__,
> > tvp->cmd,
> > dtv_cmds[tvp->cmd].name);
> >  
> > @@ -1532,7 +1529,7 @@ static int dtv_property_process_get(struct 
> > dvb_frontend *fe,
> > return -EINVAL;
> > }
> >  
> > -   dtv_property_dump(fe, false, tvp);
> > +   dtv_get_property_dump(fe, tvp);
> >  
> > return 0;
> >  }
> > @@ -1755,16 +1752,36 @@ static int dvbv3_set_delivery_system(struct 
> > dvb_frontend *fe)
> > return emulate_delivery_system(fe, delsys);
> >  }
> >  
> > +/**
> > + * dtv_property_process_set -  Sets a single DTV property
> > + * @fe:Pointer to  dvb_frontend  
> 
> Nit: extra tab looks like:
> 
> > + * @file:  Pointer to  file
> > + * @cmd:   Digital TV command
> > + * @data:  An unsigned 32-bits number
> > + *
> > + * This routine assigns the property
> > + * value to the corresponding member of
> > + *  dtv_frontend_properties
> > + *
> > + * Returns:
> > + * Zero on success, negative errno on failure.
> > + */
> >  static int dtv_property_process_set(struct dvb_frontend *fe,
> > -   struct dtv_property *tvp,
> > -   struct file *file)
> > +   struct file *file,
> > +   u32 cmd, u32 data)
> >  {
> > int r = 0;
> > struct dtv_frontend_properties *c = >dtv_property_cache;
> >  
> > -   dtv_property_dump(fe, true, tvp);  
> 
> Why not keep dtv_property_dump() the way it is? I am not seeing the
> value of this change.
> 

See below.

> > -
> > -   switch(tvp->cmd) {
> > +   /** Dump DTV command name and value*/
> > +   if (!cmd || cmd > DTV_MAX_COMMAND)
> > +   dev_warn(fe->dvb->device, "%s: SET cmd 0x%08x undefined\n",
> > +__func__, cmd);
> > +   else
> > +   dev_dbg(fe->dvb->device,
> > +   "%s: SET cmd 0x%08x (%s) to 0x%08x\n",
> > +   __func__, cmd, dtv_cmds[cmd].name, data);  
> 
> The code looked simpler before this change? Why add almost identical duplicate
> code here?

Looking on each separate patch (17 and 18) makes harder to see the value :-)

Basically, after the changes we have, at dtv_property_process_set():

int r = 0;
struct dtv_frontend_properties *c = >dtv_property_cache;

/** Dump DTV command name and value*/
if (!cmd || cmd > DTV_MAX_COMMAND)
dev_warn(fe->dvb->device, "%s: SET cmd 0x%08x undefined\n",
 __func__, cmd);
else
dev_dbg(fe->dvb->device,
"%s: SET cmd 0x%08x (%s) to 0x%08x\n",
__func__, cmd, dtv_cmds[cmd].name, data);

As here the prints happen before setting the property, the code
needs 

Re: [PATCH 1/2] dt: bindings: media: Document port and endpoint numbering

[Rob Herring]

On Thu, Sep 21, 2017 at 4:25 AM, Sakari Ailus
 wrote:
> Hi Rob,
>
> Thanks for the reply.
>
> On Wed, Sep 20, 2017 at 03:53:13PM -0500, Rob Herring wrote:
>> On Mon, Sep 18, 2017 at 11:25:04AM +0300, Sakari Ailus wrote:
>> > A lot of devices do not need and do not document port or endpoint
>> > numbering at all, e.g. in case where there's just a single port and a
>> > single endpoint. Whereas this is just common sense, document it to make it
>> > explicit.
>> >
>> > Signed-off-by: Sakari Ailus 
>> > ---
>> >  Documentation/devicetree/bindings/media/video-interfaces.txt | 12 
>> > 
>> >  1 file changed, 12 insertions(+)
>>
>> This is fine, but bindings should still be explicit. Otherwise, I'm
>> wondering if it's a single port/endpoint or they just forgot to document
>> it. And I shouldn't have to look at the example to infer that.
>>
>> Acked-by: Rob Herring 
>
> The purpose of the patch was to actually document port and endpoint
> numbering for devices for which it is not documented, not to suggest that
> this would be omitted in in binding documentation. The fact is that I
> couldn't find documentation for endpoint numbering for a single device
> under Documentation/devicetree/bindings/media/ . Yet I haven't come across
> DT source where other than zero would have been used. And the drivers
> (mostly?) have ignored endpoint numbers so far.

That's surprising. I know there are some for display controllers and
it's a common review comment I give.

>
> Some bindings have been omitted on the grounds that they're documented in
> video-interfaces.txt.
>
> What would you think of the following? I'm not sure it'd really belong
> there, but it'd be a small piece of documentation one can easily refer to.

Looks good.

>
>
> From e735979005244eb10597fe5333130b93e41d5a38 Mon Sep 17 00:00:00 2001
> From: Sakari Ailus 
> Date: Mon, 18 Sep 2017 11:15:53 +0300
> Subject: [PATCH 1/1] dt: bindings: media: Document practices for DT bindings,
>  ports, endpoints
>
> Port and endpoint numbering has been omitted in DT binding documentation
> for a large number of devices. Also common properties the device uses have
> been missed in binding documentation. Make it explicit that these things
> need to be documented.
>
> Signed-off-by: Sakari Ailus 
> ---
>  .../devicetree/bindings/media/video-interfaces.txt| 15 
> +++
>  1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
> b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index 852041a..3c5382f 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -55,6 +55,21 @@ divided into two separate ITU-R BT.656 8-bit busses.  In 
> such case bus-width
>  and data-shift properties can be used to assign physical data lines to each
>  endpoint node (logical bus).
>
> +Documenting bindings for devices
> +
> +
> +All required and optional bindings the device supports shall be explicitly
> +documented in device DT binding documentation. This also includes port and
> +endpoint numbering for the device.
> +
> +Port and endpoint numbering
> +---
> +
> +Old binding documentation may have omitted explicitly specifying port and
> +endpoint numbers. This often applies to devices that have a single port and a
> +single endpoint in that port. In this case, the only valid port number for 
> such
> +a device is zero. The same applies for devices for which bindings do not
> +document endpoint numbering: only zero is a valid endpoint.
>
>  Required properties
>  ---
> --
> 2.7.4
>
> --
> Sakari Ailus
> sakari.ai...@linux.intel.com

Re: [PATCH v3 2/2] media: rc: Add driver for tango HW IR decoder

[Sean Young]

On Thu, Sep 21, 2017 at 04:49:53PM +0200, Marc Gonzalez wrote:
> From: Mans Rullgard 
> 
> The tango HW IR decoder supports NEC, RC-5, RC-6 protocols.
> 
> Signed-off-by: Marc Gonzalez 

Missing signed-off-by.

Your patch still gives numerous checkpatch warnings, please run it
preferaby with --strict.

> ---
>  drivers/media/rc/Kconfig|   9 ++
>  drivers/media/rc/Makefile   |   1 +
>  drivers/media/rc/tango-ir.c | 265 
> 
>  3 files changed, 275 insertions(+)
>  create mode 100644 drivers/media/rc/tango-ir.c
> 
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index d9ce8ff55d0c..497c838786c8 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -469,6 +469,15 @@ config IR_SIR
>  To compile this driver as a module, choose M here: the module will
>  be called sir-ir.
>  
> +config IR_TANGO
> + tristate "Sigma Designs SMP86xx IR decoder"
> + depends on RC_CORE
> + depends on ARCH_TANGO || COMPILE_TEST
> + ---help---
> +Adds support for the HW IR decoder embedded on Sigma Designs
> +Tango-based systems (SMP86xx, SMP87xx).
> +The HW decoder supports NEC, RC-5, RC-6 IR protocols.
> +
>  config IR_ZX
>   tristate "ZTE ZX IR remote control"
>   depends on RC_CORE
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index 9bc6a3980ed0..643797dc971b 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -44,3 +44,4 @@ obj-$(CONFIG_IR_SERIAL) += serial_ir.o
>  obj-$(CONFIG_IR_SIR) += sir_ir.o
>  obj-$(CONFIG_IR_MTK) += mtk-cir.o
>  obj-$(CONFIG_IR_ZX) += zx-irdec.o
> +obj-$(CONFIG_IR_TANGO) += tango-ir.o
> diff --git a/drivers/media/rc/tango-ir.c b/drivers/media/rc/tango-ir.c
> new file mode 100644
> index ..fe19fd726aba
> --- /dev/null
> +++ b/drivers/media/rc/tango-ir.c
> @@ -0,0 +1,265 @@
> +/*
> + * Copyright (C) 2015 Mans Rullgard 
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define IR_NEC_CTRL  0x00
> +#define IR_NEC_DATA  0x04
> +#define IR_CTRL  0x08
> +#define IR_RC5_CLK_DIV   0x0c
> +#define IR_RC5_DATA  0x10
> +#define IR_INT   0x14
> +
> +#define NEC_TIME_BASE560
> +#define RC5_TIME_BASE1778
> +
> +#define RC6_CTRL 0x00
> +#define RC6_CLKDIV   0x04
> +#define RC6_DATA00x08
> +#define RC6_DATA10x0c
> +#define RC6_DATA20x10
> +#define RC6_DATA30x14
> +#define RC6_DATA40x18
> +
> +#define RC6_CARRIER  36000
> +#define RC6_TIME_BASE16
> +
> +struct tango_ir {
> + void __iomem *rc5_base;
> + void __iomem *rc6_base;
> + struct rc_dev *rc;
> + struct clk *clk;
> +};
> +
> +static void tango_ir_handle_nec(struct tango_ir *ir)
> +{
> + u32 v, code;
> + enum rc_proto proto;
> +
> + v = readl_relaxed(ir->rc5_base + IR_NEC_DATA);
> + if (!v) {
> + rc_repeat(ir->rc);
> + return;
> + }
> +
> + code = ir_nec_bytes_to_scancode(v, v >> 8, v >> 16, v >> 24, );
> + rc_keydown(ir->rc, proto, code, 0);
> +}
> +
> +static void tango_ir_handle_rc5(struct tango_ir *ir)
> +{
> + u32 data, field, toggle, addr, cmd, code;
> +
> + data = readl_relaxed(ir->rc5_base + IR_RC5_DATA);
> + if (data & BIT(31))
> + return;
> +
> + field = data >> 12 & 1;
> + toggle = data >> 11 & 1;
> + addr = data >> 6 & 0x1f;
> + cmd = (data & 0x3f) | (field ^ 1) << 6;
> +
> + code = RC_SCANCODE_RC5(addr, cmd);
> + rc_keydown(ir->rc, RC_PROTO_RC5, code, toggle);
> +}
> +
> +static void tango_ir_handle_rc6(struct tango_ir *ir)
> +{
> + u32 data0, data1, toggle, mode, addr, cmd, code;
> +
> + data0 = readl_relaxed(ir->rc6_base + RC6_DATA0);
> + data1 = readl_relaxed(ir->rc6_base + RC6_DATA1);
> +
> + mode = data0 >> 1 & 7;
> + if (mode != 0)
> + return;
> +
> + toggle = data0 & 1;
> + addr = data0 >> 16;
> + cmd = data1;
> +
> + code = RC_SCANCODE_RC6_0(addr, cmd);
> + rc_keydown(ir->rc, RC_PROTO_RC6_0, code, toggle);
> +}
> +
> +static irqreturn_t tango_ir_irq(int irq, void *dev_id)
> +{
> + struct tango_ir *ir = dev_id;
> + unsigned int rc5_stat;
> + unsigned int rc6_stat;
> +
> + rc5_stat = readl_relaxed(ir->rc5_base + IR_INT);
> + writel_relaxed(rc5_stat, ir->rc5_base + IR_INT);
> +
> + rc6_stat = readl_relaxed(ir->rc6_base + RC6_CTRL);
> + writel_relaxed(rc6_stat, ir->rc6_base + RC6_CTRL);
> +
> + if (!(rc5_stat & 3) && !(rc6_stat & BIT(31)))
> + 

Re: [PATCH v3 2/2] media: rc: Add driver for tango HW IR decoder

[Måns Rullgård]

Marc Gonzalez  writes:

> From: Mans Rullgard 
>
> The tango HW IR decoder supports NEC, RC-5, RC-6 protocols.
>
> Signed-off-by: Marc Gonzalez 

Have you been able to test all the protocols?  Universal remotes usually
support something or other with each of them.

> ---
>  drivers/media/rc/Kconfig|   9 ++
>  drivers/media/rc/Makefile   |   1 +
>  drivers/media/rc/tango-ir.c | 265 
> 
>  3 files changed, 275 insertions(+)
>  create mode 100644 drivers/media/rc/tango-ir.c
>
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index d9ce8ff55d0c..497c838786c8 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -469,6 +469,15 @@ config IR_SIR
>  To compile this driver as a module, choose M here: the module will
>  be called sir-ir.
>
> +config IR_TANGO
> + tristate "Sigma Designs SMP86xx IR decoder"
> + depends on RC_CORE
> + depends on ARCH_TANGO || COMPILE_TEST
> + ---help---
> +Adds support for the HW IR decoder embedded on Sigma Designs
> +Tango-based systems (SMP86xx, SMP87xx).
> +The HW decoder supports NEC, RC-5, RC-6 IR protocols.
> +
>  config IR_ZX
>   tristate "ZTE ZX IR remote control"
>   depends on RC_CORE
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index 9bc6a3980ed0..643797dc971b 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -44,3 +44,4 @@ obj-$(CONFIG_IR_SERIAL) += serial_ir.o
>  obj-$(CONFIG_IR_SIR) += sir_ir.o
>  obj-$(CONFIG_IR_MTK) += mtk-cir.o
>  obj-$(CONFIG_IR_ZX) += zx-irdec.o
> +obj-$(CONFIG_IR_TANGO) += tango-ir.o
> diff --git a/drivers/media/rc/tango-ir.c b/drivers/media/rc/tango-ir.c
> new file mode 100644
> index ..fe19fd726aba
> --- /dev/null
> +++ b/drivers/media/rc/tango-ir.c
> @@ -0,0 +1,265 @@
> +/*
> + * Copyright (C) 2015 Mans Rullgard 
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define IR_NEC_CTRL  0x00
> +#define IR_NEC_DATA  0x04
> +#define IR_CTRL  0x08
> +#define IR_RC5_CLK_DIV   0x0c
> +#define IR_RC5_DATA  0x10
> +#define IR_INT   0x14
> +
> +#define NEC_TIME_BASE560
> +#define RC5_TIME_BASE1778
> +
> +#define RC6_CTRL 0x00
> +#define RC6_CLKDIV   0x04
> +#define RC6_DATA00x08
> +#define RC6_DATA10x0c
> +#define RC6_DATA20x10
> +#define RC6_DATA30x14
> +#define RC6_DATA40x18
> +
> +#define RC6_CARRIER  36000
> +#define RC6_TIME_BASE16
> +
> +struct tango_ir {
> + void __iomem *rc5_base;
> + void __iomem *rc6_base;
> + struct rc_dev *rc;
> + struct clk *clk;
> +};
> +
> +static void tango_ir_handle_nec(struct tango_ir *ir)
> +{
> + u32 v, code;
> + enum rc_proto proto;
> +
> + v = readl_relaxed(ir->rc5_base + IR_NEC_DATA);
> + if (!v) {
> + rc_repeat(ir->rc);
> + return;
> + }
> +
> + code = ir_nec_bytes_to_scancode(v, v >> 8, v >> 16, v >> 24, );
> + rc_keydown(ir->rc, proto, code, 0);
> +}
> +
> +static void tango_ir_handle_rc5(struct tango_ir *ir)
> +{
> + u32 data, field, toggle, addr, cmd, code;
> +
> + data = readl_relaxed(ir->rc5_base + IR_RC5_DATA);
> + if (data & BIT(31))
> + return;
> +
> + field = data >> 12 & 1;
> + toggle = data >> 11 & 1;
> + addr = data >> 6 & 0x1f;
> + cmd = (data & 0x3f) | (field ^ 1) << 6;
> +
> + code = RC_SCANCODE_RC5(addr, cmd);
> + rc_keydown(ir->rc, RC_PROTO_RC5, code, toggle);
> +}
> +
> +static void tango_ir_handle_rc6(struct tango_ir *ir)
> +{
> + u32 data0, data1, toggle, mode, addr, cmd, code;
> +
> + data0 = readl_relaxed(ir->rc6_base + RC6_DATA0);
> + data1 = readl_relaxed(ir->rc6_base + RC6_DATA1);
> +
> + mode = data0 >> 1 & 7;
> + if (mode != 0)
> + return;
> +
> + toggle = data0 & 1;
> + addr = data0 >> 16;
> + cmd = data1;
> +
> + code = RC_SCANCODE_RC6_0(addr, cmd);
> + rc_keydown(ir->rc, RC_PROTO_RC6_0, code, toggle);
> +}
> +
> +static irqreturn_t tango_ir_irq(int irq, void *dev_id)
> +{
> + struct tango_ir *ir = dev_id;
> + unsigned int rc5_stat;
> + unsigned int rc6_stat;
> +
> + rc5_stat = readl_relaxed(ir->rc5_base + IR_INT);
> + writel_relaxed(rc5_stat, ir->rc5_base + IR_INT);
> +
> + rc6_stat = readl_relaxed(ir->rc6_base + RC6_CTRL);
> + writel_relaxed(rc6_stat, ir->rc6_base + RC6_CTRL);
> +
> + if (!(rc5_stat & 3) && !(rc6_stat & BIT(31)))
> + return 

usb/media/hdpvr: trying to register non-static key in hdpvr_probe

[Andrey Konovalov]

Hi!

I've got the following report while fuzzing the kernel with syzkaller.

On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).

INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 0 PID: 24 Comm: kworker/0:1 Not tainted 4.14.0-rc1-42251-gebb2c2437d80 #215
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
 __dump_stack lib/dump_stack.c:16
 dump_stack+0x292/0x395 lib/dump_stack.c:52
 register_lock_class+0x6c4/0x1a00 kernel/locking/lockdep.c:769
 __lock_acquire+0x27e/0x4550 kernel/locking/lockdep.c:3385
 lock_acquire+0x259/0x620 kernel/locking/lockdep.c:4002
 flush_work+0xf0/0x8c0 kernel/workqueue.c:2886
 hdpvr_probe+0x233/0x20d0 drivers/media/usb/hdpvr/hdpvr-core.c:400
 usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
 __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
 bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
 __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
 device_add+0xd0b/0x1660 drivers/base/core.c:1835
 usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
 generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
 usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
 __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
 bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
 __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
 device_add+0xd0b/0x1660 drivers/base/core.c:1835
 usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
 hub_port_connect drivers/usb/core/hub.c:4903
 hub_port_connect_change drivers/usb/core/hub.c:5009
 port_event drivers/usb/core/hub.c:5115
 hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
 process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
 worker_thread+0x221/0x1850 kernel/workqueue.c:2253
 kthread+0x3a1/0x470 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431
hdpvr: probe of 1-1:8.217 failed with error -12

usb/media/smsusb: use-after-free in worker_thread

[Andrey Konovalov]

Hi!

I've got the following report while fuzzing the kernel with syzkaller.

On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).

smsusb:smsusb_probe: board id=1, interface number 0
smsusb:siano_media_device_register: media controller created
smsusb:smsusb1_detectmode: product string not found
smsmdtv:smscore_set_device_mode: return error code -22.
smsmdtv:smscore_start_device: set device mode failed , rc -22
smsusb:smsusb_init_device: smscore_start_device(...) failed
smsusb:smsusb_onresponse: error, urb status -2, 0 bytes
smsusb:smsusb_onresponse: error, urb status -71, 0 bytes
smsusb:smsusb_onresponse: error, urb status -71, 0 bytes
smsusb:smsusb_onresponse: error, urb status -71, 0 bytes
smsusb:smsusb_onresponse: error, urb status -71, 0 bytes
smsusb:smsusb_onresponse: error, urb status -71, 0 bytes
smsusb:smsusb_onresponse: error, urb status -71, 0 bytes
smsusb:smsusb_onresponse: error, urb status -71, 0 bytes
smsusb:smsusb_onresponse: error, urb status -71, 0 bytes
smsusb:smsusb_onresponse: error, urb status -71, 0 bytes
smsusb:smsusb_probe: Device initialized with return code -22
==
BUG: KASAN: use-after-free in worker_thread+0x1468/0x1850
Read of size 8 at addr 880063be11f0 by task kworker/1:1/1152

CPU: 1 PID: 1152 Comm: kworker/1:1 Not tainted
4.14.0-rc1-42251-gebb2c2437d80 #215
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:16
 dump_stack+0x292/0x395 lib/dump_stack.c:52
 print_address_description+0x78/0x280 mm/kasan/report.c:252
 kasan_report_error mm/kasan/report.c:351
 kasan_report+0x22f/0x340 mm/kasan/report.c:409
 __asan_report_load8_noabort+0x19/0x20 mm/kasan/report.c:430
 worker_thread+0x1468/0x1850 kernel/workqueue.c:2251
 kthread+0x3a1/0x470 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431

Allocated by task 1848:
 save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
 save_stack+0x43/0xd0 mm/kasan/kasan.c:447
 set_track mm/kasan/kasan.c:459
 kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
 kmem_cache_alloc_trace+0x11e/0x2d0 mm/slub.c:2772
 kmalloc ./include/linux/slab.h:493
 kzalloc ./include/linux/slab.h:666
 smsusb_init_device+0xd5/0xd10 drivers/media/usb/siano/smsusb.c:407
 smsusb_probe+0x4f5/0xdc0 drivers/media/usb/siano/smsusb.c:571
 usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
 __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
 bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
 __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
 device_add+0xd0b/0x1660 drivers/base/core.c:1835
 usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
 generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
 usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
 __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
 bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
 __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
 device_add+0xd0b/0x1660 drivers/base/core.c:1835
 usb_new_device+0x7b8/0x1020 drivers/usb/core/hub.c:2457
 hub_port_connect drivers/usb/core/hub.c:4903
 hub_port_connect_change drivers/usb/core/hub.c:5009
 port_event drivers/usb/core/hub.c:5115
 hub_event+0x194d/0x3740 drivers/usb/core/hub.c:5195
 process_one_work+0xc7f/0x1db0 kernel/workqueue.c:2119
 worker_thread+0x221/0x1850 kernel/workqueue.c:2253
 kthread+0x3a1/0x470 kernel/kthread.c:231
 ret_from_fork+0x2a/0x40 arch/x86/entry/entry_64.S:431

Freed by task 1848:
 save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
 save_stack+0x43/0xd0 mm/kasan/kasan.c:447
 set_track mm/kasan/kasan.c:459
 kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:524
 slab_free_hook mm/slub.c:1390
 slab_free_freelist_hook mm/slub.c:1412
 slab_free mm/slub.c:2988
 kfree+0xf6/0x2f0 mm/slub.c:3919
 smsusb_term_device+0xd2/0x130 drivers/media/usb/siano/smsusb.c:363
 smsusb_init_device+0xd03/0xd10 drivers/media/usb/siano/smsusb.c:492
 smsusb_probe+0x4f5/0xdc0 drivers/media/usb/siano/smsusb.c:571
 usb_probe_interface+0x35d/0x8e0 drivers/usb/core/driver.c:361
 really_probe drivers/base/dd.c:413
 driver_probe_device+0x610/0xa00 drivers/base/dd.c:557
 __device_attach_driver+0x230/0x290 drivers/base/dd.c:653
 bus_for_each_drv+0x161/0x210 drivers/base/bus.c:463
 __device_attach+0x26e/0x3d0 drivers/base/dd.c:710
 device_initial_probe+0x1f/0x30 drivers/base/dd.c:757
 bus_probe_device+0x1eb/0x290 drivers/base/bus.c:523
 device_add+0xd0b/0x1660 drivers/base/core.c:1835
 usb_set_configuration+0x104e/0x1870 drivers/usb/core/message.c:1932
 generic_probe+0x73/0xe0 

Re: [PATCH 03/25] media: dvbdev: convert DVB device types into an enum

[Mauro Carvalho Chehab]

Em Thu, 21 Sep 2017 09:06:54 -0400
Michael Ira Krufky  escreveu:

> On Wed, Sep 20, 2017 at 3:11 PM, Mauro Carvalho Chehab
>  wrote:

> > +enum dvb_device_type {
> > +   DVB_DEVICE_SEC,
> > +   DVB_DEVICE_FRONTEND,
> > +   DVB_DEVICE_DEMUX,
> > +   DVB_DEVICE_DVR,
> > +   DVB_DEVICE_CA,
> > +   DVB_DEVICE_NET,
> > +
> > +   DVB_DEVICE_VIDEO,
> > +   DVB_DEVICE_AUDIO,
> > +   DVB_DEVICE_OSD,
> > +};  
> 
> maybe instead:
> ```
> enum dvb_device_type {
>  DVB_DEVICE_SEC  = 0,
>  DVB_DEVICE_FRONTEND = 1,
>  DVB_DEVICE_DEMUX= 2,
>  DVB_DEVICE_DVR  = 3,
>  DVB_DEVICE_CA   = 4,
>  DVB_DEVICE_NET  = 5,
> 
>  DVB_DEVICE_VIDEO= 6,
>  DVB_DEVICE_AUDIO= 7,
>  DVB_DEVICE_OSD  = 8,
> };
> ```
> 
> ...and then maybe `nums2minor()` can be optimized to take advantage of
> that assignment.

That will break userspace :-) The DVB minor ranges are
(from Documentation/drivers.txt):

 212 char   LinuxTV.org DVB driver subsystem
  0 = /dev/dvb/adapter0/video0first video decoder of first c
ard
  1 = /dev/dvb/adapter0/audio0first audio decoder of first c
ard
  2 = /dev/dvb/adapter0/sec0  (obsolete/unused)
  3 = /dev/dvb/adapter0/frontend0 first frontend device of first
 card
  4 = /dev/dvb/adapter0/demux0first demux device of first ca
rd
  5 = /dev/dvb/adapter0/dvr0  first digital video recoder de
vice of first card
  6 = /dev/dvb/adapter0/ca0   first common access port of fi
rst card
  7 = /dev/dvb/adapter0/net0  first network device of first 
card
  8 = /dev/dvb/adapter0/osd0  first on-screen-display device
 of first card

Btw, the main reason to add a switch at nums2minor() is due to that:
it requires an specific number for each device type, and it is very easy
to forget about that when looking at the implementation.

As some day we may get rid of video/audio/osd types, it sounded worth to 
add an extra code. Please notice, however, that nums2minor only exists
if !CONFIG_DVB_DYNAMIC_MINORS.

I suspect that, except for some embedded devices, the default is
to have it enabled everywhere.

Yet, after revisiting it and checking the generated assembly code,
I guess we could fold the enclosed patch, in order to simplify the
static minor support of the DVB core:


--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -68,22 +68,20 @@ static const char * const dnames[] = {
 #else
 #define DVB_MAX_IDS4
 
-static int nums2minor(int num, enum dvb_device_type type, int id)
-{
-   int n = (num << 6) | (id << 4);
+static const u8 minor_type[] = {
+   [DVB_DEVICE_VIDEO]  = 0,
+   [DVB_DEVICE_AUDIO]  = 1,
+   [DVB_DEVICE_SEC]= 2,
+   [DVB_DEVICE_FRONTEND]   = 3,
+   [DVB_DEVICE_DEMUX]  = 4,
+   [DVB_DEVICE_DVR]= 5,
+   [DVB_DEVICE_CA] = 6,
+   [DVB_DEVICE_NET]= 7,
+   [DVB_DEVICE_OSD]= 8,
+};
 
-   switch (type) {
-   case DVB_DEVICE_VIDEO:  return n;
-   case DVB_DEVICE_AUDIO:  return n | 1;
-   case DVB_DEVICE_SEC:return n | 2;
-   case DVB_DEVICE_FRONTEND:   return n | 3;
-   case DVB_DEVICE_DEMUX:  return n | 4;
-   case DVB_DEVICE_DVR:return n | 5;
-   case DVB_DEVICE_CA: return n | 6;
-   case DVB_DEVICE_NET:return n | 7;
-   case DVB_DEVICE_OSD:return n | 8;
-   }
-}
+#define nums2minor(num, type, id) \
+   (((num) << 6) | ((id) << 4) | minor_type[type])
 
 #define MAX_DVB_MINORS (DVB_MAX_ADAPTERS*64)
 #endif

With the above change, it seems that the code is likely only a few bytes
longer than the original code, and make it clearer about the
static minor numbering requirements.

The generated i386 asm code at the read only data segment is:

minor_type:
.byte   2
.byte   3
.byte   4
.byte   5
.byte   6
.byte   7
.byte   0
.byte   1
.byte   8

And at the code segment:

# drivers/media/dvb-core/dvbdev.c:511:  minor = nums2minor(adap->num, type, id);
.loc 1 511 0
movl-20(%ebp), %eax # %sfp, adap
.LVL295:
movl(%eax), %eax# adap_29(D)->num, adap_29(D)->num
.LVL296:
movl%eax, -24(%ebp) # adap_29(D)->num, %sfp
movl%eax, %edx  # adap_29(D)->num, adap_29(D)->num
movl12(%ebp), %eax  # type, tmp265
sall$6, %edx#, adap_29(D)->num
movzbl  minor_type(%eax), %eax  # minor_type, tmp193
orl %eax, %edx  # tmp193, tmp194
movl-16(%ebp), %eax # %sfp, tmp195
movl%edx, %edi  # tmp194, tmp194
sall$4, %eax#, tmp195
orl %eax, %edi  # tmp195, tmp194

That 

[PATCH] [media] tc358743: set entity function to video interface bridge

[Philipp Zabel]

The TC358743 is an HDMI to MIPI CSI2-2 bridge.

Signed-off-by: Philipp Zabel 
---
 drivers/media/i2c/tc358743.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index b7285e45b908a..82927ef0cd913 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1947,6 +1947,7 @@ static int tc358743_probe(struct i2c_client *client,
}
 
state->pad.flags = MEDIA_PAD_FL_SOURCE;
+   sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
err = media_entity_pads_init(>entity, 1, >pad);
if (err < 0)
goto err_hdl;
-- 
2.11.0

[PATCH] [media] tc358743: validate lane count and order

[Philipp Zabel]

The TC358743 does not support reordering lanes, or more than 4 data
lanes.

Signed-off-by: Philipp Zabel 
---
 drivers/media/i2c/tc358743.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index a35043cefe128..b7285e45b908a 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1743,6 +1743,7 @@ static int tc358743_probe_of(struct tc358743_state *state)
struct clk *refclk;
u32 bps_pr_lane;
int ret = -EINVAL;
+   int i;
 
refclk = devm_clk_get(dev, "refclk");
if (IS_ERR(refclk)) {
@@ -1771,6 +1772,21 @@ static int tc358743_probe_of(struct tc358743_state 
*state)
goto free_endpoint;
}
 
+   if (endpoint->bus.mipi_csi2.num_data_lanes > 4) {
+   dev_err(dev, "invalid number of lanes\n");
+   goto free_endpoint;
+   }
+
+   for (i = 0; i < endpoint->bus.mipi_csi2.num_data_lanes; i++) {
+   if (endpoint->bus.mipi_csi2.data_lanes[i] != i + 1)
+   break;
+   }
+   if (i != endpoint->bus.mipi_csi2.num_data_lanes ||
+   endpoint->bus.mipi_csi2.clock_lane != 0) {
+   dev_err(dev, "invalid lane order\n");
+   goto free_endpoint;
+   }
+
state->bus = endpoint->bus.mipi_csi2;
 
ret = clk_prepare_enable(refclk);
-- 
2.11.0

[PATCH v2 2/2] [media] imx: ask source subdevice for number of active data lanes

[Philipp Zabel]

Temporarily use g_mbus_config() to determine the number of active data
lanes used by the transmitter. If g_mbus_config is not supported or
does not return the number of active lines, default to using all
connected data lines.

Signed-off-by: Philipp Zabel 
---
New in v2:
 - Use the active lanes reported via g_mbus_config(), if available, to
   configure the CSI2_N_LANES register correctly.
---
 drivers/staging/media/imx/imx6-mipi-csi2.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c 
b/drivers/staging/media/imx/imx6-mipi-csi2.c
index 5061f3f524fd5..cd19730d0159c 100644
--- a/drivers/staging/media/imx/imx6-mipi-csi2.c
+++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
@@ -135,10 +135,8 @@ static void csi2_enable(struct csi2_dev *csi2, bool enable)
}
 }
 
-static void csi2_set_lanes(struct csi2_dev *csi2)
+static void csi2_set_lanes(struct csi2_dev *csi2, int lanes)
 {
-   int lanes = csi2->bus.num_data_lanes;
-
writel(lanes - 1, csi2->base + CSI2_N_LANES);
 }
 
@@ -301,6 +299,9 @@ static void csi2ipu_gasket_init(struct csi2_dev *csi2)
 
 static int csi2_start(struct csi2_dev *csi2)
 {
+   const u32 mask = V4L2_MBUS_CSI2_LANE_MASK;
+   struct v4l2_mbus_config cfg;
+   int lanes = 0;
int ret;
 
ret = clk_prepare_enable(csi2->pix_clk);
@@ -316,7 +317,10 @@ static int csi2_start(struct csi2_dev *csi2)
goto err_disable_clk;
 
/* Step 4 */
-   csi2_set_lanes(csi2);
+   ret = v4l2_subdev_call(csi2->src_sd, video, g_mbus_config, );
+   if (ret == 0)
+   lanes = (cfg.flags & mask) >> __ffs(mask);
+   csi2_set_lanes(csi2, lanes ?: csi2->bus.num_data_lanes);
csi2_enable(csi2, true);
 
/* Step 5 */
-- 
2.11.0

[PATCH v2 1/2] [media] tc358743: fix connected/active CSI-2 lane reporting

[Philipp Zabel]

g_mbus_config was supposed to indicate all supported lane numbers, not
only the number of those currently in active use. Since the TC358743
can dynamically reduce the number of active lanes if the required
bandwidth allows for it, report all lane numbers up to the connected
number of lanes as supported in pdata mode.
In device tree mode, do not report lane count and clock mode at all, as
the receiver driver can determine these from the device tree.

To allow communicating the number of currently active lanes, add a new
bitfield to the v4l2_mbus_config flags. This is a temporary fix, to be
used only until a better solution is found.

Signed-off-by: Philipp Zabel 
---
Changes since v1:
 - Check csi_lanes_in_use <= num_data_lanes before writing to cfg.
 - Increase size of lane mask to 4 bits and always explicitly report
   number of lanes in use.
 - Clear clock and connected lane flags in DT mode.
---
 drivers/media/i2c/tc358743.c  | 30 --
 include/media/v4l2-mediabus.h |  8 
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index e6f5c363ccab5..a35043cefe128 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1458,28 +1458,29 @@ static int tc358743_g_mbus_config(struct v4l2_subdev 
*sd,
 struct v4l2_mbus_config *cfg)
 {
struct tc358743_state *state = to_state(sd);
+   const u32 mask = V4L2_MBUS_CSI2_LANE_MASK;
+
+   if (state->csi_lanes_in_use > state->bus.num_data_lanes)
+   return -EINVAL;
 
cfg->type = V4L2_MBUS_CSI2;
+   cfg->flags = (state->csi_lanes_in_use << __ffs(mask)) & mask;
 
-   /* Support for non-continuous CSI-2 clock is missing in the driver */
-   cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+   /* In DT mode, only report the number of active lanes */
+   if (sd->dev->of_node)
+   return 0;
 
-   switch (state->csi_lanes_in_use) {
-   case 1:
+   /* Support for non-continuous CSI-2 clock is missing in pdata mode */
+   cfg->flags |= V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+
+   if (state->bus.num_data_lanes > 0)
cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
-   break;
-   case 2:
+   if (state->bus.num_data_lanes > 1)
cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
-   break;
-   case 3:
+   if (state->bus.num_data_lanes > 2)
cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
-   break;
-   case 4:
+   if (state->bus.num_data_lanes > 3)
cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
-   break;
-   default:
-   return -EINVAL;
-   }
 
return 0;
 }
@@ -1885,6 +1886,7 @@ static int tc358743_probe(struct i2c_client *client,
if (pdata) {
state->pdata = *pdata;
state->bus.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+   state->bus.num_data_lanes = 4;
} else {
err = tc358743_probe_of(state);
if (err == -ENODEV)
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 93f8afcb7a220..fc106c902bf47 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -63,6 +63,14 @@
 V4L2_MBUS_CSI2_3_LANE | 
V4L2_MBUS_CSI2_4_LANE)
 #define V4L2_MBUS_CSI2_CHANNELS(V4L2_MBUS_CSI2_CHANNEL_0 | 
V4L2_MBUS_CSI2_CHANNEL_1 | \
 V4L2_MBUS_CSI2_CHANNEL_2 | 
V4L2_MBUS_CSI2_CHANNEL_3)
+/*
+ * Number of lanes in use, 0 == use all available lanes (default)
+ *
+ * This is a temporary fix for devices that need to reduce the number of active
+ * lanes for certain modes, until g_mbus_config() can be replaced with a better
+ * solution.
+ */
+#define V4L2_MBUS_CSI2_LANE_MASK(0xf << 10)
 
 /**
  * enum v4l2_mbus_type - media bus type
-- 
2.11.0

[PATCH 4/4] [media] usbvision-core: Replace four printk() calls by dev_err()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 16:47:28 +0200

* Replace the local variable "proc" by the identifier "__func__".

* Use the interface "dev_err" instead of "printk" in these functions.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/usbvision/usbvision-core.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/usbvision/usbvision-core.c 
b/drivers/media/usb/usbvision/usbvision-core.c
index 54db35b03106..2c98805244df 100644
--- a/drivers/media/usb/usbvision/usbvision-core.c
+++ b/drivers/media/usb/usbvision/usbvision-core.c
@@ -1619,7 +1619,6 @@ static int usbvision_init_webcam(struct usb_usbvision 
*usbvision)
  */
 static int usbvision_set_video_format(struct usb_usbvision *usbvision, int 
format)
 {
-   static const char proc[] = "usbvision_set_video_format";
unsigned char *value = usbvision->ctrl_urb_buffer;
int rc;
 
@@ -1631,8 +1630,9 @@ static int usbvision_set_video_format(struct 
usb_usbvision *usbvision, int forma
if ((format != ISOC_MODE_YUV422)
&& (format != ISOC_MODE_YUV420)
&& (format != ISOC_MODE_COMPRESS)) {
-   printk(KERN_ERR "usbvision: unknown video format %02x, using 
default YUV420",
-  format);
+   dev_err(>dev->dev,
+   "%s: unknown video format %02x, using default YUV420\n",
+   __func__, format);
format = ISOC_MODE_YUV420;
}
value[0] = 0x0A;  /* TODO: See the effect of the filter */
@@ -1643,8 +1643,9 @@ static int usbvision_set_video_format(struct 
usb_usbvision *usbvision, int forma
 USB_RECIP_ENDPOINT, 0,
 (__u16) USBVISION_FILT_CONT, value, 2, HZ);
if (rc < 0)
-   printk(KERN_ERR "%s: ERROR=%d. USBVISION stopped - reconnect or 
reload driver.\n",
-  proc, rc);
+   dev_err(>dev->dev,
+   "%s: ERROR=%d. USBVISION stopped - reconnect or reload 
driver.\n",
+   __func__, rc);
 
usbvision->isoc_mode = format;
return rc;
@@ -2180,7 +2181,8 @@ int usbvision_restart_isoc(struct usb_usbvision 
*usbvision)
 int usbvision_audio_off(struct usb_usbvision *usbvision)
 {
if (usbvision_write_reg(usbvision, USBVISION_IOPIN_REG, 
USBVISION_AUDIO_MUTE) < 0) {
-   printk(KERN_ERR "usbvision_audio_off: can't write reg\n");
+   dev_err(>dev->dev,
+   "%s: can't write reg\n", __func__);
return -1;
}
usbvision->audio_mute = 0;
@@ -2192,7 +2194,9 @@ int usbvision_set_audio(struct usb_usbvision *usbvision, 
int audio_channel)
 {
if (!usbvision->audio_mute) {
if (usbvision_write_reg(usbvision, USBVISION_IOPIN_REG, 
audio_channel) < 0) {
-   printk(KERN_ERR "usbvision_set_audio: can't write iopin 
register for audio switching\n");
+   dev_err(>dev->dev,
+   "%s: can't write iopin register for audio 
switching\n",
+   __func__);
return -1;
}
}
-- 
2.14.1

[PATCH 3/4] [media] usbvision-core: Delete unnecessary braces in 11 functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 16:24:20 +0200

Do not use curly brackets at some source code places
where a single statement should be sufficient.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/usbvision/usbvision-core.c | 71 
 1 file changed, 31 insertions(+), 40 deletions(-)

diff --git a/drivers/media/usb/usbvision/usbvision-core.c 
b/drivers/media/usb/usbvision/usbvision-core.c
index bb6f4f69165f..54db35b03106 100644
--- a/drivers/media/usb/usbvision/usbvision-core.c
+++ b/drivers/media/usb/usbvision/usbvision-core.c
@@ -188,10 +188,9 @@ static int scratch_free(struct usb_usbvision *usbvision)
int free = usbvision->scratch_read_ptr - usbvision->scratch_write_ptr;
if (free <= 0)
free += scratch_buf_size;
-   if (free) {
+   if (free)
free -= 1;  
/* at least one byte in the buffer must */

/* left blank, otherwise there is no chance to differ between full and empty */
-   }
PDEBUG(DBG_SCRATCH, "return %d\n", free);
 
return free;
@@ -699,11 +698,12 @@ static enum parse_state usbvision_parse_compress(struct 
usb_usbvision *usbvision
 
frame = usbvision->cur_frame;
image_size = frame->frmwidth * frame->frmheight;
-   if ((frame->v4l2_format.format == V4L2_PIX_FMT_YUV422P) ||
-   (frame->v4l2_format.format == V4L2_PIX_FMT_YVU420)) {   /* this 
is a planar format */
+   if (frame->v4l2_format.format == V4L2_PIX_FMT_YUV422P ||
+   frame->v4l2_format.format == V4L2_PIX_FMT_YVU420)
+   /* this is a planar format */
/* ... v4l2_linesize not used here. */
f = frame->data + (frame->width * frame->curline);
-   } else
+   else
f = frame->data + (frame->v4l2_linesize * frame->curline);
 
if (frame->v4l2_format.format == V4L2_PIX_FMT_YUYV) { /* initialise u 
and v pointers */
@@ -734,22 +734,19 @@ static enum parse_state usbvision_parse_compress(struct 
usb_usbvision *usbvision
return parse_state_next_frame;
}
 
-   if (frame->curline != (int)strip_header[2]) {
+   if (frame->curline != (int)strip_header[2])
/* line number mismatch error */
usbvision->strip_line_number_errors++;
-   }
 
strip_len = 2 * (unsigned int)strip_header[1];
-   if (strip_len > USBVISION_STRIP_LEN_MAX) {
+   if (strip_len > USBVISION_STRIP_LEN_MAX)
/* strip overrun */
/* I think this never happens */
usbvision_request_intra(usbvision);
-   }
 
-   if (scratch_len(usbvision) < strip_len) {
+   if (scratch_len(usbvision) < strip_len)
/* there is not enough data for the strip */
return parse_state_out;
-   }
 
if (usbvision->intra_frame_buffer) {
Y = usbvision->intra_frame_buffer + frame->frmwidth * 
frame->curline;
@@ -1306,11 +1303,11 @@ static void usbvision_isoc_irq(struct urb *urb)
/* If we collected enough data let's parse! */
if (scratch_len(usbvision) > USBVISION_HEADER_LENGTH &&
!list_empty(&(usbvision->inqueue))) {
-   if (!(*f)) {
+   if (!(*f))
(*f) = list_entry(usbvision->inqueue.next,
  struct usbvision_frame,
  frame);
-   }
+
usbvision_parse_data(usbvision);
} else {
/* If we don't have a frame
@@ -1334,12 +1331,10 @@ static void usbvision_isoc_irq(struct urb *urb)
urb->status = 0;
urb->dev = usbvision->dev;
err_code = usb_submit_urb(urb, GFP_ATOMIC);
-
-   if (err_code) {
+   if (err_code)
dev_err(>dev->dev,
"%s: usb_submit_urb failed: error %d\n",
__func__, err_code);
-   }
 
return;
 }
@@ -1398,11 +1393,10 @@ int usbvision_write_reg(struct usb_usbvision 
*usbvision, unsigned char reg,
USB_DIR_OUT | USB_TYPE_VENDOR |
USB_RECIP_ENDPOINT, 0, (__u16) reg,
usbvision->ctrl_urb_buffer, 1, HZ);
-
-   if (err_code < 0) {
+   if (err_code < 0)
dev_err(>dev->dev,
"%s: failed: error %d\n", __func__, err_code);
-   }
+
return err_code;
 }
 
@@ -1443,10 +1437,10 @@ static int usbvision_write_reg_irq(struct usb_usbvision 
*usbvision, int address,
memcpy(usbvision->ctrl_urb_buffer, data, len);
 
err_code = 

[PATCH 2/4] [media] usbvision-core: Use common error handling code in usbvision_set_compress_params()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 12:45:49 +0200

* Add a jump target so that a bit of exception handling can be better
  reused at the end of this function.

* Replace the local variable "proc" by the identifier "__func__".

* Use the interface "dev_err" instead of "printk".

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/usbvision/usbvision-core.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/media/usb/usbvision/usbvision-core.c 
b/drivers/media/usb/usbvision/usbvision-core.c
index 16b76c85eeec..bb6f4f69165f 100644
--- a/drivers/media/usb/usbvision/usbvision-core.c
+++ b/drivers/media/usb/usbvision/usbvision-core.c
@@ -1857,7 +1857,6 @@ int usbvision_stream_interrupt(struct usb_usbvision 
*usbvision)
 
 static int usbvision_set_compress_params(struct usb_usbvision *usbvision)
 {
-   static const char proc[] = "usbvision_set_compresion_params: ";
int rc;
unsigned char *value = usbvision->ctrl_urb_buffer;
 
@@ -1882,12 +1881,8 @@ static int usbvision_set_compress_params(struct 
usb_usbvision *usbvision)
 USB_DIR_OUT | USB_TYPE_VENDOR |
 USB_RECIP_ENDPOINT, 0,
 (__u16) USBVISION_INTRA_CYC, value, 5, HZ);
-
-   if (rc < 0) {
-   printk(KERN_ERR "%sERROR=%d. USBVISION stopped - reconnect or 
reload driver.\n",
-  proc, rc);
-   return rc;
-   }
+   if (rc < 0)
+   goto report_failure;
 
if (usbvision->bridge_type == BRIDGE_NT1004) {
value[0] =  20; /* PCM Threshold 1 */
@@ -1913,11 +1908,12 @@ static int usbvision_set_compress_params(struct 
usb_usbvision *usbvision)
 USB_DIR_OUT | USB_TYPE_VENDOR |
 USB_RECIP_ENDPOINT, 0,
 (__u16) USBVISION_PCM_THR1, value, 6, HZ);
+   if (rc < 0)
+report_failure:
+   dev_err(>dev->dev,
+   "%s: ERROR=%d. USBVISION stopped - reconnect or reload 
driver.\n",
+   __func__, rc);
 
-   if (rc < 0) {
-   printk(KERN_ERR "%sERROR=%d. USBVISION stopped - reconnect or 
reload driver.\n",
-  proc, rc);
-   }
return rc;
 }
 
-- 
2.14.1

[PATCH 1/4] [media] usbvision-core: Use common error handling code in usbvision_set_input()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 11:50:54 +0200

* Add a jump target so that a bit of exception handling can be better
  reused at the end of this function.

  This issue was detected by using the Coccinelle software.

* Replace the local variable "proc" by the identifier "__func__".

* Use the interface "dev_err" instead of "printk".

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/usbvision/usbvision-core.c | 23 ++-
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/media/usb/usbvision/usbvision-core.c 
b/drivers/media/usb/usbvision/usbvision-core.c
index 3f87fbc80be2..16b76c85eeec 100644
--- a/drivers/media/usb/usbvision/usbvision-core.c
+++ b/drivers/media/usb/usbvision/usbvision-core.c
@@ -1931,7 +1931,6 @@ static int usbvision_set_compress_params(struct 
usb_usbvision *usbvision)
  */
 int usbvision_set_input(struct usb_usbvision *usbvision)
 {
-   static const char proc[] = "usbvision_set_input: ";
int rc;
unsigned char *value = usbvision->ctrl_urb_buffer;
unsigned char dvi_yuv_value;
@@ -1953,12 +1952,8 @@ int usbvision_set_input(struct usb_usbvision *usbvision)
}
 
rc = usbvision_write_reg(usbvision, USBVISION_VIN_REG1, value[0]);
-   if (rc < 0) {
-   printk(KERN_ERR "%sERROR=%d. USBVISION stopped - reconnect or 
reload driver.\n",
-  proc, rc);
-   return rc;
-   }
-
+   if (rc < 0)
+   goto report_failure;
 
if (usbvision->tvnorm_id & V4L2_STD_PAL) {
value[0] = 0xC0;
@@ -2019,12 +2014,8 @@ int usbvision_set_input(struct usb_usbvision *usbvision)
 USBVISION_OP_CODE, /* USBVISION specific code */
 USB_DIR_OUT | USB_TYPE_VENDOR | 
USB_RECIP_ENDPOINT, 0,
 (__u16) USBVISION_LXSIZE_I, value, 8, HZ);
-   if (rc < 0) {
-   printk(KERN_ERR "%sERROR=%d. USBVISION stopped - reconnect or 
reload driver.\n",
-  proc, rc);
-   return rc;
-   }
-
+   if (rc < 0)
+   goto report_failure;
 
dvi_yuv_value = 0x00;   /* U comes after V, Ya comes after U/V, Yb 
comes after Yb */
 
@@ -2036,6 +2027,12 @@ int usbvision_set_input(struct usb_usbvision *usbvision)
}
 
return usbvision_write_reg(usbvision, USBVISION_DVI_YUV, dvi_yuv_value);
+
+report_failure:
+   dev_err(>dev->dev,
+   "%s: ERROR=%d. USBVISION stopped - reconnect or reload 
driver.\n",
+   __func__, rc);
+   return rc;
 }
 
 
-- 
2.14.1

[PATCH 0/4] [media] usbvision-core: Fine-tuning for some function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 21 Sep 2017 17:00:17 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Use common error handling code in usbvision_set_input()
  Use common error handling code in usbvision_set_compress_params()
  Delete unnecessary braces in 11 functions
  Replace four printk() calls by dev_err()

 drivers/media/usb/usbvision/usbvision-core.c | 128 ---
 1 file changed, 58 insertions(+), 70 deletions(-)

-- 
2.14.1

[PATCH 1/1] ov13858: Use do_div() for dividing a 64-bit number

[Sakari Ailus]

ov13858 contained a 64-bit division. Use do_div() for calculating it.

Signed-off-by: Sakari Ailus 
---
 drivers/media/i2c/ov13858.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index 2bd659976c30..5030f4ebe056 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -951,7 +951,12 @@ static const char * const ov13858_test_pattern_menu[] = {
  * pixel_rate = link_freq * data-rate * nr_of_lanes / bits_per_sample
  * data rate => double data rate; number of lanes => 4; bits per pixel => 10
  */
-#define LINK_FREQ_TO_PIXEL_RATE(f) (((f) * 2 * 4) / 10)
+#define LINK_FREQ_TO_PIXEL_RATE(f) \
+   ({  \
+   u64 __link_freq_to_pixel_rate_tmp = (f) * 2 * 4;\
+   do_div(__link_freq_to_pixel_rate_tmp, 10);  \
+   __link_freq_to_pixel_rate_tmp;  \
+   })
 
 /* Menu items for LINK_FREQ V4L2 control */
 static const s64 link_freq_menu_items[OV13858_NUM_OF_LINK_FREQS] = {
-- 
2.11.0

Re: [PATCH 18/25] media: dvb_frontend: get rid of dtv_get_property_dump()

[Shuah Khan]

On 09/20/2017 01:11 PM, Mauro Carvalho Chehab wrote:
> Simplify the get property handling and move it to the existing
> code at dtv_property_process_get() directly.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/dvb-core/dvb_frontend.c | 43 
> ++-
>  1 file changed, 12 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_frontend.c 
> b/drivers/media/dvb-core/dvb_frontend.c
> index b7094c7a405f..607eaf3db052 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1107,36 +1107,6 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] 
> = {
>   _DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
>  };
>  
> -static void dtv_get_property_dump(struct dvb_frontend *fe,
> -   struct dtv_property *tvp)
> -{
> - int i;
> -
> - if (tvp->cmd <= 0 || tvp->cmd > DTV_MAX_COMMAND) {
> - dev_warn(fe->dvb->device, "%s: GET tvp.cmd = 0x%08x undefined\n"
> - , __func__,
> - tvp->cmd);
> - return;
> - }
> -
> - dev_dbg(fe->dvb->device, "%s: GET tvp.cmd= 0x%08x (%s)\n", __func__,
> - tvp->cmd,
> - dtv_cmds[tvp->cmd].name);
> -
> - if (dtv_cmds[tvp->cmd].buffer) {
> - dev_dbg(fe->dvb->device, "%s: tvp.u.buffer.len = 0x%02x\n",
> - __func__, tvp->u.buffer.len);
> -
> - for(i = 0; i < tvp->u.buffer.len; i++)
> - dev_dbg(fe->dvb->device,
> - "%s: tvp.u.buffer.data[0x%02x] = 
> 0x%02x\n",
> - __func__, i, tvp->u.buffer.data[i]);
> - } else {
> - dev_dbg(fe->dvb->device, "%s: tvp.u.data = 0x%08x\n", __func__,
> - tvp->u.data);
> - }
> -}
> -
>  /* Synchronise the legacy tuning parameters into the cache, so that 
> demodulator
>   * drivers can use a single set_frontend tuning function, regardless of 
> whether
>   * it's being used for the legacy or new API, reducing code and complexity.
> @@ -1529,7 +1499,18 @@ static int dtv_property_process_get(struct 
> dvb_frontend *fe,
>   return -EINVAL;
>   }
>  
> - dtv_get_property_dump(fe, tvp);
> + if (!dtv_cmds[tvp->cmd].buffer)
> + dev_dbg(fe->dvb->device,
> + "%s: GET cmd 0x%08x (%s) = 0x%08x\n",
> + __func__, tvp->cmd, dtv_cmds[tvp->cmd].name,
> + tvp->u.data);
> + else
> + dev_dbg(fe->dvb->device,
> + "%s: GET cmd 0x%08x (%s) len %d: %*ph\n",
> + __func__,
> + tvp->cmd, dtv_cmds[tvp->cmd].name,
> + tvp->u.buffer.len,
> + tvp->u.buffer.len, tvp->u.buffer.data);
>  
>   return 0;
>  }
> 

Why not keep common dtv_property_dum(0 and make these enhancements to add
more information to the dump in a common routine so both get and set are
covered.

I think this change coupled with the change in 17/25 is moving away from
common simpler code to embedded debug code. I am not clear on the value
it adds.

thanks,
-- Shuah 

[PATCH v3 2/2] media: rc: Add driver for tango HW IR decoder

[Marc Gonzalez]

From: Mans Rullgard 

The tango HW IR decoder supports NEC, RC-5, RC-6 protocols.

Signed-off-by: Marc Gonzalez 
---
 drivers/media/rc/Kconfig|   9 ++
 drivers/media/rc/Makefile   |   1 +
 drivers/media/rc/tango-ir.c | 265 
 3 files changed, 275 insertions(+)
 create mode 100644 drivers/media/rc/tango-ir.c

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index d9ce8ff55d0c..497c838786c8 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -469,6 +469,15 @@ config IR_SIR
   To compile this driver as a module, choose M here: the module will
   be called sir-ir.
 
+config IR_TANGO
+   tristate "Sigma Designs SMP86xx IR decoder"
+   depends on RC_CORE
+   depends on ARCH_TANGO || COMPILE_TEST
+   ---help---
+  Adds support for the HW IR decoder embedded on Sigma Designs
+  Tango-based systems (SMP86xx, SMP87xx).
+  The HW decoder supports NEC, RC-5, RC-6 IR protocols.
+
 config IR_ZX
tristate "ZTE ZX IR remote control"
depends on RC_CORE
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index 9bc6a3980ed0..643797dc971b 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -44,3 +44,4 @@ obj-$(CONFIG_IR_SERIAL) += serial_ir.o
 obj-$(CONFIG_IR_SIR) += sir_ir.o
 obj-$(CONFIG_IR_MTK) += mtk-cir.o
 obj-$(CONFIG_IR_ZX) += zx-irdec.o
+obj-$(CONFIG_IR_TANGO) += tango-ir.o
diff --git a/drivers/media/rc/tango-ir.c b/drivers/media/rc/tango-ir.c
new file mode 100644
index ..fe19fd726aba
--- /dev/null
+++ b/drivers/media/rc/tango-ir.c
@@ -0,0 +1,265 @@
+/*
+ * Copyright (C) 2015 Mans Rullgard 
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IR_NEC_CTRL0x00
+#define IR_NEC_DATA0x04
+#define IR_CTRL0x08
+#define IR_RC5_CLK_DIV 0x0c
+#define IR_RC5_DATA0x10
+#define IR_INT 0x14
+
+#define NEC_TIME_BASE  560
+#define RC5_TIME_BASE  1778
+
+#define RC6_CTRL   0x00
+#define RC6_CLKDIV 0x04
+#define RC6_DATA0  0x08
+#define RC6_DATA1  0x0c
+#define RC6_DATA2  0x10
+#define RC6_DATA3  0x14
+#define RC6_DATA4  0x18
+
+#define RC6_CARRIER36000
+#define RC6_TIME_BASE  16
+
+struct tango_ir {
+   void __iomem *rc5_base;
+   void __iomem *rc6_base;
+   struct rc_dev *rc;
+   struct clk *clk;
+};
+
+static void tango_ir_handle_nec(struct tango_ir *ir)
+{
+   u32 v, code;
+   enum rc_proto proto;
+
+   v = readl_relaxed(ir->rc5_base + IR_NEC_DATA);
+   if (!v) {
+   rc_repeat(ir->rc);
+   return;
+   }
+
+   code = ir_nec_bytes_to_scancode(v, v >> 8, v >> 16, v >> 24, );
+   rc_keydown(ir->rc, proto, code, 0);
+}
+
+static void tango_ir_handle_rc5(struct tango_ir *ir)
+{
+   u32 data, field, toggle, addr, cmd, code;
+
+   data = readl_relaxed(ir->rc5_base + IR_RC5_DATA);
+   if (data & BIT(31))
+   return;
+
+   field = data >> 12 & 1;
+   toggle = data >> 11 & 1;
+   addr = data >> 6 & 0x1f;
+   cmd = (data & 0x3f) | (field ^ 1) << 6;
+
+   code = RC_SCANCODE_RC5(addr, cmd);
+   rc_keydown(ir->rc, RC_PROTO_RC5, code, toggle);
+}
+
+static void tango_ir_handle_rc6(struct tango_ir *ir)
+{
+   u32 data0, data1, toggle, mode, addr, cmd, code;
+
+   data0 = readl_relaxed(ir->rc6_base + RC6_DATA0);
+   data1 = readl_relaxed(ir->rc6_base + RC6_DATA1);
+
+   mode = data0 >> 1 & 7;
+   if (mode != 0)
+   return;
+
+   toggle = data0 & 1;
+   addr = data0 >> 16;
+   cmd = data1;
+
+   code = RC_SCANCODE_RC6_0(addr, cmd);
+   rc_keydown(ir->rc, RC_PROTO_RC6_0, code, toggle);
+}
+
+static irqreturn_t tango_ir_irq(int irq, void *dev_id)
+{
+   struct tango_ir *ir = dev_id;
+   unsigned int rc5_stat;
+   unsigned int rc6_stat;
+
+   rc5_stat = readl_relaxed(ir->rc5_base + IR_INT);
+   writel_relaxed(rc5_stat, ir->rc5_base + IR_INT);
+
+   rc6_stat = readl_relaxed(ir->rc6_base + RC6_CTRL);
+   writel_relaxed(rc6_stat, ir->rc6_base + RC6_CTRL);
+
+   if (!(rc5_stat & 3) && !(rc6_stat & BIT(31)))
+   return IRQ_NONE;
+
+   if (rc5_stat & BIT(0))
+   tango_ir_handle_rc5(ir);
+
+   if (rc5_stat & BIT(1))
+   tango_ir_handle_nec(ir);
+
+   if (rc6_stat & BIT(31))
+   tango_ir_handle_rc6(ir);
+
+   return IRQ_HANDLED;
+}
+
+#define DISABLE_NEC(BIT(4) | BIT(8))
+#define ENABLE_RC5 (BIT(0) | BIT(9))
+#define ENABLE_RC6 (BIT(0) | BIT(7))
+

[PATCH v3 1/2] media: dt: bindings: Add binding for tango HW IR decoder

[Marc Gonzalez]

Add DT binding for the HW IR decoder embedded in SMP86xx/SMP87xx.

Signed-off-by: Marc Gonzalez 
---
 .../devicetree/bindings/media/tango-ir.txt  | 21 +
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/tango-ir.txt

diff --git a/Documentation/devicetree/bindings/media/tango-ir.txt 
b/Documentation/devicetree/bindings/media/tango-ir.txt
new file mode 100644
index ..a9f00c2bf897
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/tango-ir.txt
@@ -0,0 +1,21 @@
+Sigma Designs Tango IR NEC/RC-5/RC-6 decoder (SMP86xx and SMP87xx)
+
+Required properties:
+
+- compatible: "sigma,smp8642-ir"
+- reg: address/size of NEC+RC5 area, address/size of RC6 area
+- interrupts: spec for IR IRQ
+- clocks: spec for IR clock (typically the crystal oscillator)
+
+Optional properties:
+
+- linux,rc-map-name: see Documentation/devicetree/bindings/media/rc.txt
+
+Example:
+
+   ir@10518 {
+   compatible = "sigma,smp8642-ir";
+   reg = <0x10518 0x18>, <0x105e0 0x1c>;
+   interrupts = <21 IRQ_TYPE_EDGE_RISING>;
+   clocks = <>;
+   };
-- 
2.11.0

Re: [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling

[Jonathan Cameron]

On Thu, 21 Sep 2017 16:15:28 +0200
Wolfram Sang  wrote:

> > > > +/**
> > > > + * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync 
> > > > with i2c_msg
> > > > + * @msg: the message to be synced with
> > > > + * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be 
> > > > NULL.
> > > > + */
> > > > +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
> > > > +{
> > > > +   if (!buf || buf == msg->buf)
> > > > +   return;
> > > > +
> > > > +   if (msg->flags & I2C_M_RD)
> > > > +   memcpy(msg->buf, buf, msg->len);
> > > > +
> > > > +   kfree(buf);  
> > 
> > Only free when you actually allocated it.  Seems to me like you need
> > to check if (!(msg->flags & I2C_M_DMA_SAFE)) before kfree.
> > 
> > Otherwise the logic to do this will be needed in every driver
> > which will get irritating fast.  
> 
> Well, I return early if (buf == msg->buf) which is only true for
> I2C_M_DMA_SAFE. If not, I allocated the buffer. Am I missing something?
> It would be very strange to call this function if the caller allocated
> the buffer manually.
> 
> Thanks for the review!

Doh missed that check and my comment was bonkers even if it hadn't been there.
I come back to the claim of insufficient caffeine.

You are quite correct.  Please ignore previous comment - the code is
fine as is. 

Jonathan
> 
> 

Re: [PATCH 17/25] media: dvb_frontend: dtv_property_process_set() cleanups

[Shuah Khan]

On 09/20/2017 01:11 PM, Mauro Carvalho Chehab wrote:
> From: Satendra Singh Thakur 
> 
> Since all properties in the func dtv_property_process_set() use
> at most 4 bytes arguments, change the code to pass
> u32 cmd and u32 data as function arguments, instead of passing a
> pointer to the entire struct dtv_property *tvp.
> 
> Instead of having a generic dtv_property_dump(), added its own
> properties debug logic at dtv_property_process_set().

Nit: in the dtv_property_process_set()

> 
> Signed-off-by: Satendra Singh Thakur 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/dvb-core/dvb_frontend.c | 125 
> --
>  1 file changed, 72 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/media/dvb-core/dvb_frontend.c 
> b/drivers/media/dvb-core/dvb_frontend.c
> index bd60a490ce0f..b7094c7a405f 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -1107,22 +1107,19 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 
> 1] = {
>   _DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
>  };
>  
> -static void dtv_property_dump(struct dvb_frontend *fe,
> -   bool is_set,
> +static void dtv_get_property_dump(struct dvb_frontend *fe,
> struct dtv_property *tvp)
>  {
>   int i;
>  
>   if (tvp->cmd <= 0 || tvp->cmd > DTV_MAX_COMMAND) {
> - dev_warn(fe->dvb->device, "%s: %s tvp.cmd = 0x%08x undefined\n",
> - __func__,
> - is_set ? "SET" : "GET",
> + dev_warn(fe->dvb->device, "%s: GET tvp.cmd = 0x%08x undefined\n"
> + , __func__,
>   tvp->cmd);
>   return;
>   }
>  
> - dev_dbg(fe->dvb->device, "%s: %s tvp.cmd= 0x%08x (%s)\n", __func__,
> - is_set ? "SET" : "GET",
> + dev_dbg(fe->dvb->device, "%s: GET tvp.cmd= 0x%08x (%s)\n", __func__,
>   tvp->cmd,
>   dtv_cmds[tvp->cmd].name);
>  
> @@ -1532,7 +1529,7 @@ static int dtv_property_process_get(struct dvb_frontend 
> *fe,
>   return -EINVAL;
>   }
>  
> - dtv_property_dump(fe, false, tvp);
> + dtv_get_property_dump(fe, tvp);
>  
>   return 0;
>  }
> @@ -1755,16 +1752,36 @@ static int dvbv3_set_delivery_system(struct 
> dvb_frontend *fe)
>   return emulate_delivery_system(fe, delsys);
>  }
>  
> +/**
> + * dtv_property_process_set -  Sets a single DTV property
> + * @fe:  Pointer to  dvb_frontend

Nit: extra tab looks like:

> + * @file:Pointer to  file
> + * @cmd: Digital TV command
> + * @data:An unsigned 32-bits number
> + *
> + * This routine assigns the property
> + * value to the corresponding member of
> + *  dtv_frontend_properties
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
>  static int dtv_property_process_set(struct dvb_frontend *fe,
> - struct dtv_property *tvp,
> - struct file *file)
> + struct file *file,
> + u32 cmd, u32 data)
>  {
>   int r = 0;
>   struct dtv_frontend_properties *c = >dtv_property_cache;
>  
> - dtv_property_dump(fe, true, tvp);

Why not keep dtv_property_dump() the way it is? I am not seeing the
value of this change.

> -
> - switch(tvp->cmd) {
> + /** Dump DTV command name and value*/
> + if (!cmd || cmd > DTV_MAX_COMMAND)
> + dev_warn(fe->dvb->device, "%s: SET cmd 0x%08x undefined\n",
> +  __func__, cmd);
> + else
> + dev_dbg(fe->dvb->device,
> + "%s: SET cmd 0x%08x (%s) to 0x%08x\n",
> + __func__, cmd, dtv_cmds[cmd].name, data);

The code looked simpler before this change? Why add almost identical duplicate
code here?

> + switch (cmd) {
>   case DTV_CLEAR:
>   /*
>* Reset a cache of data specific to the frontend here. This 
> does
> @@ -1784,133 +1801,133 @@ static int dtv_property_process_set(struct 
> dvb_frontend *fe,
>   r = dtv_set_frontend(fe);
>   break;
>   case DTV_FREQUENCY:
> - c->frequency = tvp->u.data;
> + c->frequency = data;
>   break;
>   case DTV_MODULATION:
> - c->modulation = tvp->u.data;
> + c->modulation = data;
>   break;
>   case DTV_BANDWIDTH_HZ:
> - c->bandwidth_hz = tvp->u.data;
> + c->bandwidth_hz = data;
>   break;
>   case DTV_INVERSION:
> - c->inversion = tvp->u.data;
> + c->inversion = data;
>   break;
>   case DTV_SYMBOL_RATE:
> - c->symbol_rate = tvp->u.data;
> + 

Re: [RFC PATCH v5 6/6] i2c: dev: mark RDWR buffers as DMA_SAFE

[Wolfram Sang]

On Thu, Sep 21, 2017 at 03:17:44PM +0100, Jonathan Cameron wrote:
> On Wed, 20 Sep 2017 20:59:56 +0200
> Wolfram Sang  wrote:
> 
> > Signed-off-by: Wolfram Sang 
> 
> Makes sense as do the other drivers.
> 
> Feel free to add
> 
> Reviewed-by: Jonathan Cameron 
> 
> to all of them (though they hardly took a lot of reviewing given how simple
> the patches were :)

Well, bugs can slip in everywhere, so thanks for the review!



signature.asc
Description: PGP signature

Re: [PATCH 02/25] media: dvb_frontend: fix return values for FE_SET_PROPERTY

[Shuah Khan]

On 09/20/2017 01:11 PM, Mauro Carvalho Chehab wrote:
> There are several problems with regards to the return of
> FE_SET_PROPERTY. The original idea were to return per-property
> return codes via tvp->result field, and to return an updated
> set of values.
> 
> However, that never worked. What's actually implemented is:
> 
> - the FE_SET_PROPERTY implementation doesn't call .get_frontend
>   callback in order to get the actual parameters after return;
> 
> - the tvp->result field is only filled if there's no error.
>   So, it is always filled with zero;
> 
> - FE_SET_PROPERTY doesn't call memdup_user() nor any other
>   copy_to_user() function. So, any changes at the properies

Spelling - properties

Nit: to the properties

>   will be lost;
> 
> - FE_SET_PROPERTY is declared as a write-only ioctl (IOW).
> 
> While we could fix the above, it could cause regressions.
> 
> So, let's just assume what the code really does, updating
> the documentation accordingly and removing the logic that
> would update the discarded tvp->result.
> 
> Signed-off-by: Mauro Carvalho Chehab 

The rest looks good.

Reviewed-by: Shuah Khan 

> ---
>  Documentation/media/uapi/dvb/fe-get-property.rst | 7 +--
>  drivers/media/dvb-core/dvb_frontend.c| 2 --
>  include/uapi/linux/dvb/frontend.h| 2 +-
>  3 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/media/uapi/dvb/fe-get-property.rst 
> b/Documentation/media/uapi/dvb/fe-get-property.rst
> index 948d2ba84f2c..b69741d9cedf 100644
> --- a/Documentation/media/uapi/dvb/fe-get-property.rst
> +++ b/Documentation/media/uapi/dvb/fe-get-property.rst
> @@ -48,8 +48,11 @@ depends on the delivery system and on the device:
>  
> -  This call requires read/write access to the device.
>  
> -   -  At return, the values are updated to reflect the actual parameters
> -  used.
> +.. note::
> +
> +   At return, the values aren't updated to reflect the actual
> +   parameters used. If the actual parameters are needed, an explicit
> +   call to ``FE_GET_PROPERTY`` is needed.
>  
>  -  ``FE_GET_PROPERTY:``
>  
> diff --git a/drivers/media/dvb-core/dvb_frontend.c 
> b/drivers/media/dvb-core/dvb_frontend.c
> index 7dda5acea3f2..bd60a490ce0f 100644
> --- a/drivers/media/dvb-core/dvb_frontend.c
> +++ b/drivers/media/dvb-core/dvb_frontend.c
> @@ -2142,7 +2142,6 @@ static int dvb_frontend_handle_ioctl(struct file *file,
>   kfree(tvp);
>   return err;
>   }
> - (tvp + i)->result = err;
>   }
>   kfree(tvp);
>   break;
> @@ -2187,7 +2186,6 @@ static int dvb_frontend_handle_ioctl(struct file *file,
>   kfree(tvp);
>   return err;
>   }
> - (tvp + i)->result = err;
>   }
>  
>   if (copy_to_user((void __user *)tvps->props, tvp,
> diff --git a/include/uapi/linux/dvb/frontend.h 
> b/include/uapi/linux/dvb/frontend.h
> index 861cacd5711f..6bc26f35217b 100644
> --- a/include/uapi/linux/dvb/frontend.h
> +++ b/include/uapi/linux/dvb/frontend.h
> @@ -830,7 +830,7 @@ struct dtv_fe_stats {
>   * @cmd: Digital TV command.
>   * @reserved:Not used.
>   * @u:   Union with the values for the command.
> - * @result:  Result of the command set (currently unused).
> + * @result:  Unused
>   *
>   * The @u union may have either one of the values below:
>   *
> 

thanks,
-- Shuah

Re: [RFC PATCH v5 6/6] i2c: dev: mark RDWR buffers as DMA_SAFE

[Jonathan Cameron]

On Wed, 20 Sep 2017 20:59:56 +0200
Wolfram Sang  wrote:

> Signed-off-by: Wolfram Sang 

Makes sense as do the other drivers.

Feel free to add

Reviewed-by: Jonathan Cameron 

to all of them (though they hardly took a lot of reviewing given how simple
the patches were :)

> ---
>  drivers/i2c/i2c-dev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
> index 6f638bbc922db4..bbc7aadb4c899d 100644
> --- a/drivers/i2c/i2c-dev.c
> +++ b/drivers/i2c/i2c-dev.c
> @@ -280,6 +280,8 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client 
> *client,
>   res = PTR_ERR(rdwr_pa[i].buf);
>   break;
>   }
> + /* memdup_user allocates with GFP_KERNEL, so DMA is ok */
> + rdwr_pa[i].flags |= I2C_M_DMA_SAFE;
>  
>   /*
>* If the message length is received from the slave (similar

Re: [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling

[Wolfram Sang]

> > > +/**
> > > + * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync with 
> > > i2c_msg
> > > + * @msg: the message to be synced with
> > > + * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be 
> > > NULL.
> > > + */
> > > +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
> > > +{
> > > + if (!buf || buf == msg->buf)
> > > + return;
> > > +
> > > + if (msg->flags & I2C_M_RD)
> > > + memcpy(msg->buf, buf, msg->len);
> > > +
> > > + kfree(buf);
> 
> Only free when you actually allocated it.  Seems to me like you need
> to check if (!(msg->flags & I2C_M_DMA_SAFE)) before kfree.
> 
> Otherwise the logic to do this will be needed in every driver
> which will get irritating fast.

Well, I return early if (buf == msg->buf) which is only true for
I2C_M_DMA_SAFE. If not, I allocated the buffer. Am I missing something?
It would be very strange to call this function if the caller allocated
the buffer manually.

Thanks for the review!



signature.asc
Description: PGP signature

Re: [RFC PATCH v5 3/6] i2c: add docs to clarify DMA handling

[Jonathan Cameron]

On Wed, 20 Sep 2017 16:56:48 -0300
Mauro Carvalho Chehab  wrote:

> Em Wed, 20 Sep 2017 20:59:53 +0200
> Wolfram Sang  escreveu:
> 
> > Signed-off-by: Wolfram Sang   
> 
> Documentation looks OK on my eyes. So:
> 
> Reviewed-by: Mauro Carvalho Chehab 

Really minor suggestion inline. I don't really care either way as
what you had is perfectly comprehensible. 

Reviewed-by: Jonathan Cameron 

> 
> > ---
> >  Documentation/i2c/DMA-considerations | 58 
> > 
> >  1 file changed, 58 insertions(+)
> >  create mode 100644 Documentation/i2c/DMA-considerations
> > 
> > diff --git a/Documentation/i2c/DMA-considerations 
> > b/Documentation/i2c/DMA-considerations
> > new file mode 100644
> > index 00..5a63355c6a9b6f
> > --- /dev/null
> > +++ b/Documentation/i2c/DMA-considerations
> > @@ -0,0 +1,58 @@
> > +=
> > +Linux I2C and DMA
> > +=
> > +
> > +Given that I2C is a low-speed bus where largely small messages are 
> > transferred,

Slightly nicer as:

Given that i2c is a low-speed bus, over which the majority of messages 
transferred are small,

> > +it is not considered a prime user of DMA access. At this time of writing, 
> > only
> > +10% of I2C bus master drivers have DMA support implemented. And the vast
> > +majority of transactions are so small that setting up DMA for it will 
> > likely
> > +add more overhead than a plain PIO transfer.
> > +
> > +Therefore, it is *not* mandatory that the buffer of an I2C message is DMA 
> > safe.
> > +It does not seem reasonable to apply additional burdens when the feature 
> > is so
> > +rarely used. However, it is recommended to use a DMA-safe buffer if your
> > +message size is likely applicable for DMA. Most drivers have this threshold
> > +around 8 bytes (as of today, this is mostly an educated guess, however). 
> > For
> > +any message of 16 byte or larger, it is probably a really good idea. Please
> > +note that other subsystems you use might add requirements. E.g., if your
> > +I2C bus master driver is using USB as a bridge, then you need to have DMA
> > +safe buffers always, because USB requires it.
> > +
> > +For clients, if you use a DMA safe buffer in i2c_msg, set the 
> > I2C_M_DMA_SAFE
> > +flag with it. Then, the I2C core and drivers know they can safely operate 
> > DMA
> > +on it. Note that using this flag is optional. I2C host drivers which are 
> > not
> > +updated to use this flag will work like before. And like before, they risk
> > +using an unsafe DMA buffer. To improve this situation, using 
> > I2C_M_DMA_SAFE in
> > +more and more clients and host drivers is the planned way forward. Note 
> > also
> > +that setting this flag makes only sense in kernel space. User space data is
> > +copied into kernel space anyhow. The I2C core makes sure the destination
> > +buffers in kernel space are always DMA capable.
> > +
> > +FIXME: Need to implement i2c_master_{send|receive}_dma and proper buffers 
> > for i2c_smbus_xfer_emulated.
> > +
> > +Drivers wishing to implement safe DMA can use helper functions from the I2C
> > +core. One gives you a DMA-safe buffer for a given i2c_msg as long as a 
> > certain
> > +threshold is met::
> > +
> > +   dma_buf = i2c_get_dma_safe_msg_buf(msg, threshold_in_byte);
> > +
> > +If a buffer is returned, it is either msg->buf for the I2C_M_DMA_SAFE case 
> > or a
> > +bounce buffer. But you don't need to care about that detail, just use the
> > +returned buffer. If NULL is returned, the threshold was not met or a bounce
> > +buffer could not be allocated. Fall back to PIO in that case.
> > +
> > +In any case, a buffer obtained from above needs to be released. It ensures 
> > data
> > +is copied back to the message and a potentially used bounce buffer is 
> > freed::
> > +
> > +   i2c_release_dma_safe_msg_buf(msg, dma_buf);
> > +
> > +The bounce buffer handling from the core is generic and simple. It will 
> > always
> > +allocate a new bounce buffer. If you want a more sophisticated handling 
> > (e.g.
> > +reusing pre-allocated buffers), you are free to implement your own.
> > +
> > +Please also check the in-kernel documentation for details. The 
> > i2c-sh_mobile
> > +driver can be used as a reference example how to use the above helpers.
> > +
> > +Final note: If you plan to use DMA with I2C (or with anything else, 
> > actually)
> > +make sure you have CONFIG_DMA_API_DEBUG enabled during development. It can 
> > help
> > +you find various issues which can be complex to debug otherwise.  
> 
> 
> 
> Thanks,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling

[Jonathan Cameron]

On Thu, 21 Sep 2017 14:59:22 +0100
Jonathan Cameron  wrote:

> On Wed, 20 Sep 2017 20:59:52 +0200
> Wolfram Sang  wrote:
> 
> > One helper checks if DMA is suitable and optionally creates a bounce
> > buffer, if not. The other function returns the bounce buffer and makes
> > sure the data is properly copied back to the message.
> > 
> > Signed-off-by: Wolfram Sang   
> 
> One minor suggestion for wording. Otherwise looks good to me.
> 
> Reviewed-by: Jonathan Cameron 
>

Need more coffee. See below.
 
> > ---
> >  drivers/i2c/i2c-core-base.c | 45 
> > +
> >  include/linux/i2c.h |  3 +++
> >  2 files changed, 48 insertions(+)
> > 
> > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> > index 56e46581b84bdb..925a22794d3ced 100644
> > --- a/drivers/i2c/i2c-core-base.c
> > +++ b/drivers/i2c/i2c-core-base.c
> > @@ -2241,6 +2241,51 @@ void i2c_put_adapter(struct i2c_adapter *adap)
> >  }
> >  EXPORT_SYMBOL(i2c_put_adapter);
> >  
> > +/**
> > + * i2c_get_dma_safe_msg_buf() - get a DMA safe buffer for the given i2c_msg
> > + * @msg: the message to be checked
> > + * @threshold: the amount of byte from which using DMA makes sense  
> 
> the minimum number of bytes for which using DMA makes sense.
> 
> > + *
> > + * Return: NULL if a DMA safe buffer was not obtained. Use msg->buf with 
> > PIO.
> > + *
> > + *Or a valid pointer to be used with DMA. Note that it can either be
> > + *msg->buf or a bounce buffer. After use, release it by calling
> > + *i2c_release_dma_safe_msg_buf().
> > + *
> > + * This function must only be called from process context!
> > + */
> > +u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold)
> > +{
> > +   if (msg->len < threshold)
> > +   return NULL;
> > +
> > +   if (msg->flags & I2C_M_DMA_SAFE)
> > +   return msg->buf;
> > +
> > +   if (msg->flags & I2C_M_RD)
> > +   return kzalloc(msg->len, GFP_KERNEL);
> > +   else
> > +   return kmemdup(msg->buf, msg->len, GFP_KERNEL);
> > +}
> > +EXPORT_SYMBOL_GPL(i2c_get_dma_safe_msg_buf);
> > +
> > +/**
> > + * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync with 
> > i2c_msg
> > + * @msg: the message to be synced with
> > + * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL.
> > + */
> > +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
> > +{
> > +   if (!buf || buf == msg->buf)
> > +   return;
> > +
> > +   if (msg->flags & I2C_M_RD)
> > +   memcpy(msg->buf, buf, msg->len);
> > +
> > +   kfree(buf);

Only free when you actually allocated it.  Seems to me like you need
to check if (!(msg->flags & I2C_M_DMA_SAFE)) before kfree.

Otherwise the logic to do this will be needed in every driver
which will get irritating fast.


> > +}
> > +EXPORT_SYMBOL_GPL(i2c_release_dma_safe_msg_buf);
> > +
> >  MODULE_AUTHOR("Simon G. Vogl ");
> >  MODULE_DESCRIPTION("I2C-Bus main module");
> >  MODULE_LICENSE("GPL");
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index d501d3956f13f0..1e99342f180f45 100644
> > --- a/include/linux/i2c.h
> > +++ b/include/linux/i2c.h
> > @@ -767,6 +767,9 @@ static inline u8 i2c_8bit_addr_from_msg(const struct 
> > i2c_msg *msg)
> > return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
> >  }
> >  
> > +u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold);
> > +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf);
> > +
> >  int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short 
> > addr);
> >  /**
> >   * module_i2c_driver() - Helper macro for registering a modular I2C driver 
> >  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] tc358743: fix connected/active CSI-2 lane reporting

[Hans Verkuil]

On 09/21/17 14:35, Philipp Zabel wrote:
> Hi Ian,
> 
> On Thu, 2017-09-21 at 12:06 +0100, Ian Arkver wrote:
> [...]
>>> My understanding of Hans' comment:
>>> "I'd also add a comment that all other flags must be 0 if the device 
>>> tree is used. This to avoid mixing the two."
>>>
>>> is that all the above should only happen if (!!state->pdata).
>>
>> Except that state->pdata is a copy of the pdata, not a pointer, but you 
>> know what I mean. Some other check for DT needed here.
> 
> Yes, I'll change this to zero all V4L2_MBUS_CSI2_[1-4]_LANE in the DT
> case. I suppose the V4L2_MBUS_CSI2_CONTINUOUS_CLOCK and
> V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK bits should be zeroed as well, then?

Yes. Just zero all bits except those in the V4L2_MBUS_CSI2_LANE_MASK. And
changing that to (0xf << 10) makes sense.

Regards,

Hans

> 
>>> I don't know if this would break any existing DT-using bridge drivers.
> 
> The only current users of g_mbus_config are the pxa_camera and
> sh_mobile_ceu_camera soc_camera drivers. Neither supports MIPI CSI-2, as
> far as I can tell.
> 
> regards
> Philipp
> 

Re: [PATCH] [media] cx231xx-cards: fix NULL-deref on missing association descriptor

[Andrey Konovalov]

On Thu, Sep 21, 2017 at 10:40 AM, Johan Hovold  wrote:
> Make sure to check that we actually have an Interface Association
> Descriptor before dereferencing it during probe to avoid dereferencing a
> NULL-pointer.
>
> Fixes: e0d3bafd0258 ("V4L/DVB (10954): Add cx231xx USB driver")
> Cc: stable  # 2.6.30
> Cc: Sri Deevi 
> Reported-by: Andrey Konovalov 
> Signed-off-by: Johan Hovold 

Tested-by: Andrey Konovalov 

Thanks!

> ---
>  drivers/media/usb/cx231xx/cx231xx-cards.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c 
> b/drivers/media/usb/cx231xx/cx231xx-cards.c
> index e0daa9b6c2a0..9b742d569fb5 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-cards.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
> @@ -1684,7 +1684,7 @@ static int cx231xx_usb_probe(struct usb_interface 
> *interface,
> nr = dev->devno;
>
> assoc_desc = udev->actconfig->intf_assoc[0];
> -   if (assoc_desc->bFirstInterface != ifnum) {
> +   if (!assoc_desc || assoc_desc->bFirstInterface != ifnum) {
> dev_err(d, "Not found matching IAD interface\n");
> retval = -ENODEV;
> goto err_if;
> --
> 2.14.1
>

Re: [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling

[Jonathan Cameron]

On Wed, 20 Sep 2017 20:59:52 +0200
Wolfram Sang  wrote:

> One helper checks if DMA is suitable and optionally creates a bounce
> buffer, if not. The other function returns the bounce buffer and makes
> sure the data is properly copied back to the message.
> 
> Signed-off-by: Wolfram Sang 

One minor suggestion for wording. Otherwise looks good to me.

Reviewed-by: Jonathan Cameron 

> ---
>  drivers/i2c/i2c-core-base.c | 45 
> +
>  include/linux/i2c.h |  3 +++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 56e46581b84bdb..925a22794d3ced 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -2241,6 +2241,51 @@ void i2c_put_adapter(struct i2c_adapter *adap)
>  }
>  EXPORT_SYMBOL(i2c_put_adapter);
>  
> +/**
> + * i2c_get_dma_safe_msg_buf() - get a DMA safe buffer for the given i2c_msg
> + * @msg: the message to be checked
> + * @threshold: the amount of byte from which using DMA makes sense

the minimum number of bytes for which using DMA makes sense.

> + *
> + * Return: NULL if a DMA safe buffer was not obtained. Use msg->buf with PIO.
> + *
> + *  Or a valid pointer to be used with DMA. Note that it can either be
> + *  msg->buf or a bounce buffer. After use, release it by calling
> + *  i2c_release_dma_safe_msg_buf().
> + *
> + * This function must only be called from process context!
> + */
> +u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold)
> +{
> + if (msg->len < threshold)
> + return NULL;
> +
> + if (msg->flags & I2C_M_DMA_SAFE)
> + return msg->buf;
> +
> + if (msg->flags & I2C_M_RD)
> + return kzalloc(msg->len, GFP_KERNEL);
> + else
> + return kmemdup(msg->buf, msg->len, GFP_KERNEL);
> +}
> +EXPORT_SYMBOL_GPL(i2c_get_dma_safe_msg_buf);
> +
> +/**
> + * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync with 
> i2c_msg
> + * @msg: the message to be synced with
> + * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL.
> + */
> +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
> +{
> + if (!buf || buf == msg->buf)
> + return;
> +
> + if (msg->flags & I2C_M_RD)
> + memcpy(msg->buf, buf, msg->len);
> +
> + kfree(buf);
> +}
> +EXPORT_SYMBOL_GPL(i2c_release_dma_safe_msg_buf);
> +
>  MODULE_AUTHOR("Simon G. Vogl ");
>  MODULE_DESCRIPTION("I2C-Bus main module");
>  MODULE_LICENSE("GPL");
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index d501d3956f13f0..1e99342f180f45 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -767,6 +767,9 @@ static inline u8 i2c_8bit_addr_from_msg(const struct 
> i2c_msg *msg)
>   return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
>  }
>  
> +u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold);
> +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf);
> +
>  int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short 
> addr);
>  /**
>   * module_i2c_driver() - Helper macro for registering a modular I2C driver

Re: [linux-sunxi] [PATCH v2 1/3] media: V3s: Add support for Allwinner CSI.

[Ondřej Jirman]

Hello Yong,

I noticed one issue in the register macros. See below.

Yong Deng píše v Čt 27. 07. 2017 v 13:01 +0800:
> Allwinner V3s SoC have two CSI module. CSI0 is used for MIPI interface
> and CSI1 is used for parallel interface. This is not documented in
> datasheet but by testing 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.
> 
> Signed-off-by: Yong Deng 
> ---
> 

[snip]

> +
> +#define CSI_CH_INT_EN_REG0x70
> +#define CSI_CH_INT_EN_VS_INT_EN  BIT(7)
> +#define CSI_CH_INT_EN_HB_OF_INT_EN   BIT(6)
> +#define CSI_CH_INT_EN_MUL_ERR_INT_EN BIT(5)
> +#define CSI_CH_INT_EN_FIFO2_OF_INT_ENBIT(4)
> +#define CSI_CH_INT_EN_FIFO1_OF_INT_ENBIT(3)
> +#define CSI_CH_INT_EN_FIFO0_OF_INT_ENBIT(2)
> +#define CSI_CH_INT_EN_FD_INT_EN  BIT(1)
> +#define CSI_CH_INT_EN_CD_INT_EN  BIT(0)
> +
> +#define CSI_CH_INT_STA_REG   0x74
> +#define CSI_CH_INT_STA_VS_PD BIT(7)
> +#define CSI_CH_INT_STA_HB_OF_PD  BIT(6)
> +#define CSI_CH_INT_STA_MUL_ERR_PDBIT(5)
> +#define CSI_CH_INT_STA_FIFO2_OF_PD   BIT(4)
> +#define CSI_CH_INT_STA_FIFO1_OF_PD   BIT(3)
> +#define CSI_CH_INT_STA_FIFO0_OF_PD   BIT(2)
> +#define CSI_CH_INT_STA_FD_PD BIT(1)
> +#define CSI_CH_INT_STA_CD_PD BIT(0)
> +
> +#define CSI_CH_FLD1_VSIZE_REG0x74

This register should be 0x78 according to the V3s manual. Though it's
not used in your driver yet, so it is not yet causing any issues.

> +#define CSI_CH_HSIZE_REG 0x80
> +#define CSI_CH_HSIZE_HOR_LEN_MASKGENMASK(28, 16)
> +#define CSI_CH_HSIZE_HOR_LEN(len)((len << 16) & 
> CSI_CH_HSIZE_HOR_LEN_MASK)
> +#define CSI_CH_HSIZE_HOR_START_MASK  GENMASK(12, 0)
> +#define CSI_CH_HSIZE_HOR_START(start)((start << 0) & 
> CSI_CH_HSIZE_HOR_START_MASK)
> +
> +#define CSI_CH_VSIZE_REG 0x84
> +#define CSI_CH_VSIZE_VER_LEN_MASKGENMASK(28, 16)
> +#define CSI_CH_VSIZE_VER_LEN(len)((len << 16) & 
> CSI_CH_VSIZE_VER_LEN_MASK)
> +#define CSI_CH_VSIZE_VER_START_MASK  GENMASK(12, 0)
> +#define CSI_CH_VSIZE_VER_START(start)((start << 0) & 
> CSI_CH_VSIZE_VER_START_MASK)
> +
> +#define CSI_CH_BUF_LEN_REG   0x88
> +#define CSI_CH_BUF_LEN_BUF_LEN_C_MASKGENMASK(29, 16)
> +#define CSI_CH_BUF_LEN_BUF_LEN_C(len)((len << 16) & 
> CSI_CH_BUF_LEN_BUF_LEN_C_MASK)
> +#define CSI_CH_BUF_LEN_BUF_LEN_Y_MASKGENMASK(13, 0)
> +#define CSI_CH_BUF_LEN_BUF_LEN_Y(len)((len << 0) & 
> CSI_CH_BUF_LEN_BUF_LEN_Y_MASK)
> +
> +#define CSI_CH_FLIP_SIZE_REG 0x8c
> +#define CSI_CH_FLIP_SIZE_VER_LEN_MASKGENMASK(28, 16)
> +#define CSI_CH_FLIP_SIZE_VER_LEN(len)((len << 16) & 
> CSI_CH_FLIP_SIZE_VER_LEN_MASK)
> +#define CSI_CH_FLIP_SIZE_VALID_LEN_MASK  GENMASK(12, 0)
> +#define CSI_CH_FLIP_SIZE_VALID_LEN(len)  ((len << 0) & 
> CSI_CH_FLIP_SIZE_VALID_LEN_MASK)
> +
> +#define CSI_CH_FRM_CLK_CNT_REG   0x90
> +#define CSI_CH_ACC_ITNL_CLK_CNT_REG  0x94
> +#define CSI_CH_FIFO_STAT_REG 0x98
> +#define CSI_CH_PCLK_STAT_REG 0x9c
> +
> +/*
> + * csi input data format
> + */
> +enum csi_input_fmt
> +{
> + CSI_INPUT_FORMAT_RAW= 0,
> + CSI_INPUT_FORMAT_YUV422 = 3,
> + CSI_INPUT_FORMAT_YUV420 = 4,
> +};
> +
> +/*
> + * csi output data format
> + */
> +enum csi_output_fmt
> +{
> + /* only when input format is RAW */
> + CSI_FIELD_RAW_8 = 0,
> + CSI_FIELD_RAW_10= 1,
> + CSI_FIELD_RAW_12= 2,
> + CSI_FIELD_RGB565= 4,
> + CSI_FIELD_RGB888= 5,
> + CSI_FIELD_PRGB888   = 6,
> + CSI_FRAME_RAW_8 = 8,
> + CSI_FRAME_RAW_10= 9,
> + CSI_FRAME_RAW_12= 10,
> + CSI_FRAME_RGB565= 12,
> + CSI_FRAME_RGB888= 13,
> + CSI_FRAME_PRGB888   = 14,
> +
> + /* only when input format is YUV422/YUV420 */

Other limitation is that when input is YUV420 output can only be YUV420
and not YUV422.

> + CSI_FIELD_PLANAR_YUV422 = 0,
> + CSI_FIELD_PLANAR_YUV420 = 1,
> + CSI_FRAME_PLANAR_YUV420 = 2,
> + CSI_FRAME_PLANAR_YUV422 = 3,
> + CSI_FIELD_UV_CB_YUV422  = 4,
> + CSI_FIELD_UV_CB_YUV420  = 5,
> + CSI_FRAME_UV_CB_YUV420  = 6,
> + CSI_FRAME_UV_CB_YUV422  = 7,
> + CSI_FIELD_MB_YUV422 = 8,
> + CSI_FIELD_MB_YUV420 = 9,
> + 

Re: [PATCH v2] media: rc: Add driver for tango IR decoder

[Marc Gonzalez]

On 21/09/2017 13:25, Sean Young wrote:

> On Wed, Sep 20, 2017 at 10:39:11AM +0200, Marc Gonzalez wrote:
> 
>> From: Mans Rullgard 
>>
>> The tango IR decoder supports NEC, RC-5, RC-6 protocols.
>>
>> Signed-off-by: Marc Gonzalez 
> 
> This needs a signed-off-by from all the authors.

Mans, the ball is in your court :-)

In the mean time, I might work on the universal IR receiver,
or the IR blaster.

>>  .../devicetree/bindings/media/tango-ir.txt |  21 ++
>>  drivers/media/rc/Kconfig   |   5 +
>>  drivers/media/rc/Makefile  |   1 +
>>  drivers/media/rc/tango-ir.c| 265 
>> +
>>  4 files changed, 292 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/tango-ir.txt
>>  create mode 100644 drivers/media/rc/tango-ir.c
> 
> You should add an entry to the MAINTAINERS file.

It's already taken care of, with a file regex pattern for
ARM/TANGO ARCHITECTURE (N: tango)

>> diff --git a/Documentation/devicetree/bindings/media/tango-ir.txt 
>> b/Documentation/devicetree/bindings/media/tango-ir.txt
>> new file mode 100644
>> index ..a9f00c2bf897
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/tango-ir.txt
>> @@ -0,0 +1,21 @@
>> +Sigma Designs Tango IR NEC/RC-5/RC-6 decoder (SMP86xx and SMP87xx)
>> +
>> +Required properties:
>> +
>> +- compatible: "sigma,smp8642-ir"
>> +- reg: address/size of NEC+RC5 area, address/size of RC6 area
>> +- interrupts: spec for IR IRQ
>> +- clocks: spec for IR clock (typically the crystal oscillator)
>> +
>> +Optional properties:
>> +
>> +- linux,rc-map-name: see Documentation/devicetree/bindings/media/rc.txt
>> +
>> +Example:
>> +
>> +ir@10518 {
>> +compatible = "sigma,smp8642-ir";
>> +reg = <0x10518 0x18>, <0x105e0 0x1c>;
>> +interrupts = <21 IRQ_TYPE_EDGE_RISING>;
>> +clocks = <>;
>> +};
> 
> This needs to be a separate commit/patch.

OK, I will send a v3 series. Could you explain the rationale behind
having separate patches? (I don't think Rob minds having a binding
description pushed through a different tree.)

>> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
>> index d9ce8ff55d0c..f84923289964 100644
>> --- a/drivers/media/rc/Kconfig
>> +++ b/drivers/media/rc/Kconfig
>> @@ -469,6 +469,11 @@ config IR_SIR
>> To compile this driver as a module, choose M here: the module will
>> be called sir-ir.
>>  
>> +config IR_TANGO
>> +tristate "Sigma Designs SMP86xx IR decoder"
>> +depends on RC_CORE
>> +depends on ARCH_TANGO || COMPILE_TEST
> 
> This needs --help-- a section, even if it is mostly boilerplate.
> 
> This will be catched by ./scripts/checkpatch.pl, please run this script
> on your patches.

OK.

Regards.

Re: [PATCH] ov9655: fix potential integer overflow

[kbuild test robot]

Hi Gustavo,

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

url:
https://github.com/0day-ci/linux/commits/Gustavo-A-R-Silva/ov9655-fix-potential-integer-overflow/20170921-174735
base:   git://linuxtv.org/media_tree.git master
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

>> ERROR: "__udivdi3" [drivers/media/i2c/ov9650.ko] undefined!

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


.config.gz
Description: application/gzip

Re: [PATCH 03/25] media: dvbdev: convert DVB device types into an enum

[Michael Ira Krufky]

On Wed, Sep 20, 2017 at 3:11 PM, Mauro Carvalho Chehab
 wrote:
> Enums can be documented via kernel-doc. So, convert the
> DVB_DEVICE_* macros to an enum.
>
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/dvb-core/dvbdev.c | 34 +++
>  drivers/media/dvb-core/dvbdev.h | 51 
> -
>  2 files changed, 64 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 41aad0f99d73..7b4cdcfbb02c 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -51,8 +51,15 @@ static LIST_HEAD(dvb_adapter_list);
>  static DEFINE_MUTEX(dvbdev_register_lock);
>
>  static const char * const dnames[] = {
> -   "video", "audio", "sec", "frontend", "demux", "dvr", "ca",
> -   "net", "osd"
> +   [DVB_DEVICE_VIDEO] ="video",
> +   [DVB_DEVICE_AUDIO] ="audio",
> +   [DVB_DEVICE_SEC] =  "sec",
> +   [DVB_DEVICE_FRONTEND] = "frontend",
> +   [DVB_DEVICE_DEMUX] ="demux",
> +   [DVB_DEVICE_DVR] =  "dvr",
> +   [DVB_DEVICE_CA] =   "ca",
> +   [DVB_DEVICE_NET] =  "net",
> +   [DVB_DEVICE_OSD] =  "osd"
>  };
>
>  #ifdef CONFIG_DVB_DYNAMIC_MINORS
> @@ -60,7 +67,24 @@ static const char * const dnames[] = {
>  #define DVB_MAX_IDSMAX_DVB_MINORS
>  #else
>  #define DVB_MAX_IDS4
> -#define nums2minor(num, type, id)  ((num << 6) | (id << 4) | type)
> +
> +static int nums2minor(int num, enum dvb_device_type type, int id)
> +{
> +   int n = (num << 6) | (id << 4);
> +
> +   switch (type) {
> +   case DVB_DEVICE_VIDEO:  return n;
> +   case DVB_DEVICE_AUDIO:  return n | 1;
> +   case DVB_DEVICE_SEC:return n | 2;
> +   case DVB_DEVICE_FRONTEND:   return n | 3;
> +   case DVB_DEVICE_DEMUX:  return n | 4;
> +   case DVB_DEVICE_DVR:return n | 5;
> +   case DVB_DEVICE_CA: return n | 6;
> +   case DVB_DEVICE_NET:return n | 7;
> +   case DVB_DEVICE_OSD:return n | 8;
> +   }
> +}
> +
>  #define MAX_DVB_MINORS (DVB_MAX_ADAPTERS*64)
>  #endif
>
> @@ -426,8 +450,8 @@ static int dvb_register_media_device(struct dvb_device 
> *dvbdev,
>  }
>
>  int dvb_register_device(struct dvb_adapter *adap, struct dvb_device 
> **pdvbdev,
> -   const struct dvb_device *template, void *priv, int 
> type,
> -   int demux_sink_pads)
> +   const struct dvb_device *template, void *priv,
> +   enum dvb_device_type type, int demux_sink_pads)
>  {
> struct dvb_device *dvbdev;
> struct file_operations *dvbdevfops;
> diff --git a/drivers/media/dvb-core/dvbdev.h b/drivers/media/dvb-core/dvbdev.h
> index 49189392cf3b..53058da83873 100644
> --- a/drivers/media/dvb-core/dvbdev.h
> +++ b/drivers/media/dvb-core/dvbdev.h
> @@ -35,15 +35,37 @@
>
>  #define DVB_UNSET (-1)
>
> -#define DVB_DEVICE_VIDEO  0
> -#define DVB_DEVICE_AUDIO  1
> -#define DVB_DEVICE_SEC2
> -#define DVB_DEVICE_FRONTEND   3
> -#define DVB_DEVICE_DEMUX  4
> -#define DVB_DEVICE_DVR5
> -#define DVB_DEVICE_CA 6
> -#define DVB_DEVICE_NET7
> -#define DVB_DEVICE_OSD8
> +/* List of DVB device types */
> +
> +/**
> + * enum dvb_device_type - type of the Digital TV device
> + *
> + * @DVB_DEVICE_SEC:Digital TV standalone Common Interface (CI)
> + * @DVB_DEVICE_FRONTEND:   Digital TV frontend.
> + * @DVB_DEVICE_DEMUX:  Digital TV demux.
> + * @DVB_DEVICE_DVR:Digital TV digital video record (DVR).
> + * @DVB_DEVICE_CA: Digital TV Conditional Access (CA).
> + * @DVB_DEVICE_NET:Digital TV network.
> + *
> + * @DVB_DEVICE_VIDEO:  Digital TV video decoder.
> + * Deprecated. Used only on av7110-av.
> + * @DVB_DEVICE_AUDIO:  Digital TV audio decoder.
> + * Deprecated. Used only on av7110-av.
> + * @DVB_DEVICE_OSD:Digital TV On Screen Display (OSD).
> + * Deprecated. Used only on av7110.
> + */
> +enum dvb_device_type {
> +   DVB_DEVICE_SEC,
> +   DVB_DEVICE_FRONTEND,
> +   DVB_DEVICE_DEMUX,
> +   DVB_DEVICE_DVR,
> +   DVB_DEVICE_CA,
> +   DVB_DEVICE_NET,
> +
> +   DVB_DEVICE_VIDEO,
> +   DVB_DEVICE_AUDIO,
> +   DVB_DEVICE_OSD,
> +};

maybe instead:
```
enum dvb_device_type {
 DVB_DEVICE_SEC  = 0,
 DVB_DEVICE_FRONTEND = 1,
 DVB_DEVICE_DEMUX= 2,
 DVB_DEVICE_DVR  = 3,
 DVB_DEVICE_CA   = 4,
 DVB_DEVICE_NET  = 5,

 DVB_DEVICE_VIDEO= 6,
 DVB_DEVICE_AUDIO= 7,
 DVB_DEVICE_OSD  = 8,
};
```

...and then maybe `nums2minor()` can be 

Re: [PATCH] scripts: kernel-doc: fix nexted handling

[Markus Heiser]

> Jon,
> 
> While documenting some DVB demux  headers, I noticed the above bug.
> 
> scripts/kernel-doc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 9d3eafea58f0..15f934a23d1d 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -2173,7 +2173,7 @@ sub dump_struct($$) {
>   my $members = $3;
> 
>   # ignore embedded structs or unions
> - $members =~ s/({.*})//g;
> + $members =~ s/({[^\}]*})//g;
>   $nested = $1;
> 
>   # ignore members marked private:

Hi Mauro,

I tested this patch. Feel free to add my

 Tested-by: Markus Heiser 

FYI: I also migrated the patch to my python kernel-doc parser:

   https://github.com/return42/linuxdoc/commit/5dbb93f

And here is the impact of this patch on the whole sources:

   https://github.com/return42/sphkerneldoc/commit/7be0fa85

In the last link, you see that your patch is a great improvement / Thanks!!

-- Markus --

Re: [PATCH] tc358743: fix connected/active CSI-2 lane reporting

[Philipp Zabel]

Hi Ian,

On Thu, 2017-09-21 at 12:06 +0100, Ian Arkver wrote:
[...]
> > My understanding of Hans' comment:
> > "I'd also add a comment that all other flags must be 0 if the device 
> > tree is used. This to avoid mixing the two."
> > 
> > is that all the above should only happen if (!!state->pdata).
> 
> Except that state->pdata is a copy of the pdata, not a pointer, but you 
> know what I mean. Some other check for DT needed here.

Yes, I'll change this to zero all V4L2_MBUS_CSI2_[1-4]_LANE in the DT
case. I suppose the V4L2_MBUS_CSI2_CONTINUOUS_CLOCK and
V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK bits should be zeroed as well, then?

> > I don't know if this would break any existing DT-using bridge drivers.

The only current users of g_mbus_config are the pxa_camera and
sh_mobile_ceu_camera soc_camera drivers. Neither supports MIPI CSI-2, as
far as I can tell.

regards
Philipp

Re: [PATCH] tc358743: fix connected/active CSI-2 lane reporting

[Philipp Zabel]

On Thu, 2017-09-21 at 12:41 +0100, Dave Stevenson wrote:
> Hi Philipp
> 
> On 21 September 2017 at 11:24, Philipp Zabel 
> wrote:
[...]
> > +   if (state->csi_lanes_in_use > 4)
> 
> One could suggest
> if (state->csi_lanes_in_use > state->bus.num_data_lanes)
> here. Needing to use more lanes than are configured is surely an
> error, although that may be detectable at the other end. See below
> too.

True. The OF parser could be improved to make sure that
num_data_lanes <= 4, and also that all lanes are in the correct order.

[...]
> > +/*
> > + * Number of lanes in use, 0 == use all available lanes (default)
> > + *
> > + * This is a temporary fix for devices that need to reduce the number of 
> > active
> > + * lanes for certain modes, until g_mbus_config() can be replaced with a 
> > better
> > + * solution.
> > + */
> > +#define V4L2_MBUS_CSI2_LANE_MASK(3 << 10)
> 
> I know this was Hans' suggested define, but are we saying 4 lanes is
> not a valid value? If it is then the mask needs to be at least (7 <<
> 10).

0 must map to "all lanes" for backwards compatibility, but I see no
reason why we shouldn't add another bit here to support reporting 4
lanes explicitly.

> 4 lanes is not necessarily "all available lanes".

Correct.

> - There are now CSI2 devices supporting up to 8 lanes (although
> V4L2_FWNODE_CSI2_MAX_DATA_LANES limits you to 4 at the moment).

So how about we just add two bits, then?

#define V4L2_MBUS_CSI2_LANE_MASK(0xf << 10)

> - Or you could have 2 lanes configured in DT and ask TC358743 for (eg)
> 1080P60 UYVY at 594Mbps (needs 4 lanes) which passes the current logic
> in the TC358743 driver and would return 0, when it is actually going
> to use 4 lanes. That could be classed as a driver bug though.

Right, the driver shouldn't try to start streaming at all if it knows
that the available CSI-2 bandwidth will be exceeded.

> My view is that if a driver is going to report how many lanes to use
> then it should always report it explicitly. The default 0 value should
> only be used for devices that will never change it from the DT
> settings. Perhaps others disagree
> 
> Otherwise the patch works for me.

I'm fine with changing this as you suggest.

regards
Philipp

Re: [PATCH v4] drm/bridge/sii8620: add remote control support

[Sean Young]

On Mon, Sep 18, 2017 at 04:37:52PM +0200, Hans Verkuil wrote:
> On 09/18/2017 04:15 PM, Maciej Purski wrote:
> > Hi Hans,
> > some time ago in reply to your email I described what messages does
> > the MHL driver receive and at what time intervals.
> > Regarding that information, do you think that a similar solution as
> > in [1] is required? Would it be OK, if I just set REP_DELAY and REP_PERIOD
> > to values, which I presented in my last email?
> 
> Based on the timings you measured I would say that there is a 99% chance that 
> MHL
> uses exactly the same "Press and Hold" mechanism as CEC. Since you already 
> specify
> RC_PROTO_BIT_CEC in the driver, it will set REP_DELAY and REP_PERIOD to the 
> right
> values automatically.
> 
> You will have to implement the same code as in cec-adap.c for the key press 
> handling,
> though. It's a bit tricky to get it right.
> 
> Since you will have to do the same thing as I do in CEC, I wonder if it would 
> make
> more sense to move this code to the RC core. Basically it ensures that repeat 
> mode
> doesn't turn on until you get two identical key presses 550ms or less apart. 
> This
> is independent of REP_DELAY which can be changed by userspace.
> 
> Sean, what do you think?

Yes, this makes sense. In fact, since IR protocols have different repeat
times, I would like to make this protocol dependant anyway.

To be honest I find REP_DELAY of 500ms too long and REP_PERIOD of 125ms
too short; IOW it takes too long for a key to start repeating, and once
it starts repeating it is very hard to control. If I try to increase the
volume with my remote it first hardly changes at all and then after 500ms
the volume shoots up far too quickly, same thing when navigating menus.

CEC dictates a delay period of 550ms, which is not great for the user IMO.

Anyway I'll have a look at this over the weekend.


Sean

Re: [PATCH] tc358743: fix connected/active CSI-2 lane reporting

[Dave Stevenson]

Hi Philipp

On 21 September 2017 at 11:24, Philipp Zabel  wrote:
> g_mbus_config was supposed to indicate all supported lane numbers, not
> only the number of those currently in active use. Since the tc358743
> can dynamically reduce the number of active lanes if the required
> bandwidth allows for it, report all lane numbers up to the connected
> number of lanes as supported.
> To allow communicating the number of currently active lanes, add a new
> bitfield to the v4l2_mbus_config flags. This is a temporary fix, until
> a better solution is found.
>
> Signed-off-by: Philipp Zabel 
> ---
>  drivers/media/i2c/tc358743.c  | 22 --
>  include/media/v4l2-mediabus.h |  8 
>  2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
> index e6f5c363ccab5..e2a9e6a18a49d 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -1464,21 +1464,22 @@ static int tc358743_g_mbus_config(struct v4l2_subdev 
> *sd,
> /* Support for non-continuous CSI-2 clock is missing in the driver */
> cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
>
> -   switch (state->csi_lanes_in_use) {
> -   case 1:
> +   if (state->bus.num_data_lanes > 0)
> cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
> -   break;
> -   case 2:
> +   if (state->bus.num_data_lanes > 1)
> cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
> -   break;
> -   case 3:
> +   if (state->bus.num_data_lanes > 2)
> cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
> -   break;
> -   case 4:
> +   if (state->bus.num_data_lanes > 3)
> cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
> -   break;
> -   default:
> +
> +   if (state->csi_lanes_in_use > 4)

One could suggest
if (state->csi_lanes_in_use > state->bus.num_data_lanes)
here. Needing to use more lanes than are configured is surely an
error, although that may be detectable at the other end. See below
too.

> return -EINVAL;
> +
> +   if (state->csi_lanes_in_use < state->bus.num_data_lanes) {
> +   const u32 mask = V4L2_MBUS_CSI2_LANE_MASK;
> +
> +   cfg->flags |= (state->csi_lanes_in_use << __ffs(mask)) & mask;
> }
>
> return 0;
> @@ -1885,6 +1886,7 @@ static int tc358743_probe(struct i2c_client *client,
> if (pdata) {
> state->pdata = *pdata;
> state->bus.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> +   state->bus.num_data_lanes = 4;
> } else {
> err = tc358743_probe_of(state);
> if (err == -ENODEV)
> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> index 93f8afcb7a220..3467d97be5f5b 100644
> --- a/include/media/v4l2-mediabus.h
> +++ b/include/media/v4l2-mediabus.h
> @@ -63,6 +63,14 @@
>  V4L2_MBUS_CSI2_3_LANE | 
> V4L2_MBUS_CSI2_4_LANE)
>  #define V4L2_MBUS_CSI2_CHANNELS(V4L2_MBUS_CSI2_CHANNEL_0 | 
> V4L2_MBUS_CSI2_CHANNEL_1 | \
>  V4L2_MBUS_CSI2_CHANNEL_2 | 
> V4L2_MBUS_CSI2_CHANNEL_3)
> +/*
> + * Number of lanes in use, 0 == use all available lanes (default)
> + *
> + * This is a temporary fix for devices that need to reduce the number of 
> active
> + * lanes for certain modes, until g_mbus_config() can be replaced with a 
> better
> + * solution.
> + */
> +#define V4L2_MBUS_CSI2_LANE_MASK(3 << 10)

I know this was Hans' suggested define, but are we saying 4 lanes is
not a valid value? If it is then the mask needs to be at least (7 <<
10).

4 lanes is not necessarily "all available lanes".
- There are now CSI2 devices supporting up to 8 lanes (although
V4L2_FWNODE_CSI2_MAX_DATA_LANES limits you to 4 at the moment).
- Or you could have 2 lanes configured in DT and ask TC358743 for (eg)
1080P60 UYVY at 594Mbps (needs 4 lanes) which passes the current logic
in the TC358743 driver and would return 0, when it is actually going
to use 4 lanes. That could be classed as a driver bug though.

My view is that if a driver is going to report how many lanes to use
then it should always report it explicitly. The default 0 value should
only be used for devices that will never change it from the DT
settings. Perhaps others disagree

Otherwise the patch works for me.

  Dave.

>  /**
>   * enum v4l2_mbus_type - media bus type
> --
> 2.11.0
>

Re: [PATCH v2] media: rc: Add driver for tango IR decoder

[Sean Young]

On Wed, Sep 20, 2017 at 10:39:11AM +0200, Marc Gonzalez wrote:
> From: Mans Rullgard 
> 
> The tango IR decoder supports NEC, RC-5, RC-6 protocols.
> 
> Signed-off-by: Marc Gonzalez 

This needs a signed-off-by from all the authors.

> ---
> Changes from v1 to v2
> o Fixup a syntax error introduced by search & replace
> o Bring back support for linux,rc-map-name property
> o Provide binding description
> ---
>  .../devicetree/bindings/media/tango-ir.txt |  21 ++
>  drivers/media/rc/Kconfig   |   5 +
>  drivers/media/rc/Makefile  |   1 +
>  drivers/media/rc/tango-ir.c| 265 
> +
>  4 files changed, 292 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/tango-ir.txt
>  create mode 100644 drivers/media/rc/tango-ir.c

You should add an entry to the MAINTAINERS file.

> 
> diff --git a/Documentation/devicetree/bindings/media/tango-ir.txt 
> b/Documentation/devicetree/bindings/media/tango-ir.txt
> new file mode 100644
> index ..a9f00c2bf897
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/tango-ir.txt
> @@ -0,0 +1,21 @@
> +Sigma Designs Tango IR NEC/RC-5/RC-6 decoder (SMP86xx and SMP87xx)
> +
> +Required properties:
> +
> +- compatible: "sigma,smp8642-ir"
> +- reg: address/size of NEC+RC5 area, address/size of RC6 area
> +- interrupts: spec for IR IRQ
> +- clocks: spec for IR clock (typically the crystal oscillator)
> +
> +Optional properties:
> +
> +- linux,rc-map-name: see Documentation/devicetree/bindings/media/rc.txt
> +
> +Example:
> +
> + ir@10518 {
> + compatible = "sigma,smp8642-ir";
> + reg = <0x10518 0x18>, <0x105e0 0x1c>;
> + interrupts = <21 IRQ_TYPE_EDGE_RISING>;
> + clocks = <>;
> + };

This needs to be a separate commit/patch.

> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index d9ce8ff55d0c..f84923289964 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -469,6 +469,11 @@ config IR_SIR
>  To compile this driver as a module, choose M here: the module will
>  be called sir-ir.
>  
> +config IR_TANGO
> + tristate "Sigma Designs SMP86xx IR decoder"
> + depends on RC_CORE
> + depends on ARCH_TANGO || COMPILE_TEST

This needs --help-- a section, even if it is mostly boilerplate.

This will be catched by ./scripts/checkpatch.pl, please run this script
on your patches.

>  config IR_ZX
>   tristate "ZTE ZX IR remote control"
>   depends on RC_CORE
> diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> index 9bc6a3980ed0..643797dc971b 100644
> --- a/drivers/media/rc/Makefile
> +++ b/drivers/media/rc/Makefile
> @@ -44,3 +44,4 @@ obj-$(CONFIG_IR_SERIAL) += serial_ir.o
>  obj-$(CONFIG_IR_SIR) += sir_ir.o
>  obj-$(CONFIG_IR_MTK) += mtk-cir.o
>  obj-$(CONFIG_IR_ZX) += zx-irdec.o
> +obj-$(CONFIG_IR_TANGO) += tango-ir.o
> diff --git a/drivers/media/rc/tango-ir.c b/drivers/media/rc/tango-ir.c
> new file mode 100644
> index ..fe19fd726aba
> --- /dev/null
> +++ b/drivers/media/rc/tango-ir.c
> @@ -0,0 +1,265 @@
> +/*
> + * Copyright (C) 2015 Mans Rullgard 
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define IR_NEC_CTRL  0x00
> +#define IR_NEC_DATA  0x04
> +#define IR_CTRL  0x08
> +#define IR_RC5_CLK_DIV   0x0c
> +#define IR_RC5_DATA  0x10
> +#define IR_INT   0x14
> +
> +#define NEC_TIME_BASE560
> +#define RC5_TIME_BASE1778
> +
> +#define RC6_CTRL 0x00
> +#define RC6_CLKDIV   0x04
> +#define RC6_DATA00x08
> +#define RC6_DATA10x0c
> +#define RC6_DATA20x10
> +#define RC6_DATA30x14
> +#define RC6_DATA40x18
> +
> +#define RC6_CARRIER  36000
> +#define RC6_TIME_BASE16
> +
> +struct tango_ir {
> + void __iomem *rc5_base;
> + void __iomem *rc6_base;
> + struct rc_dev *rc;
> + struct clk *clk;
> +};
> +
> +static void tango_ir_handle_nec(struct tango_ir *ir)
> +{
> + u32 v, code;
> + enum rc_proto proto;
> +
> + v = readl_relaxed(ir->rc5_base + IR_NEC_DATA);
> + if (!v) {
> + rc_repeat(ir->rc);
> + return;
> + }
> +
> + code = ir_nec_bytes_to_scancode(v, v >> 8, v >> 16, v >> 24, );
> + rc_keydown(ir->rc, proto, code, 0);
> +}
> +
> +static void tango_ir_handle_rc5(struct tango_ir *ir)
> +{
> + u32 data, field, toggle, addr, cmd, code;
> +
> + data = readl_relaxed(ir->rc5_base + IR_RC5_DATA);
> + if (data & BIT(31))
> + return;
> +
> 

Re: [media] s2255drv: Adjust 13 checks for null pointers

[SF Markus Elfring]

>> Would you like to clarify corresponding concerns any more?
>>
> 
> Look at the `git log`

I did this also for a moment.


> and it just copies those lines:

The Git software preserves these three message fields
(when special characters were used in the commit message).

Can you accept such software functionality?

Regards,
Markus

Re: [PATCH v1] media: rc: Add driver for tango IR decoder

[Sean Young]

On Tue, Sep 19, 2017 at 02:43:17PM +0200, Marc Gonzalez wrote:
> + Rob & Mark for the DT bindings question.
> 
> On 19/09/2017 14:21, Måns Rullgård wrote:
> 
> > Marc Gonzalez writes:
> > 
> >> On 18/09/2017 17:33, Måns Rullgård wrote:
> >>
> >>> What have you changed compared to my original code?
> >>
> >> I forgot to mention one change you may not approve of, so we should
> >> probably discuss it.
> >>
> >> Your driver supported an optional DT property "linux,rc-map-name"
> >> to override the RC_MAP_EMPTY map. Since the IR decoder supports
> >> multiple protocols, I found it odd to specify a scancode map in
> >> something as low-level as the device tree.
> >>
> >> I saw only one board using that property:
> >> $ git grep "linux,rc-map-name" arch/
> >> arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts: 
> >> linux,rc-map-name = "rc-geekbox";
> >>
> >> So I removed support for "linux,rc-map-name" and used ir-keytable
> >> to load a given map from user-space, depending on which RC I use.
> >>
> >> Mans, Sean, what do you think?
> > 
> > The property is documented as common for IR receivers although only a
> > few drivers seem to actually implement the feature.  Since driver
> > support is trivial, I see no reason to skip it.  Presumably someone
> > had a use for it, or it wouldn't have been added.
> 
> I do not dispute the usefulness of the "linux,rc-map-name" property
> in general, e.g. for boards that support a single remote control.
> 
> I am arguing that the person writing the device tree has no way of
> knowing which rc-map a given user will be using, because it depends
> on the actual remote control being used.
> 
> Maybe I'm missing something.

The device tree for a board can be for a specific product, which ships
with a specific remote. It makes sense to support it, so that any
product that uses the tango-ir can select the remote it ships with.


Sean

Re: [PATCH] tc358743: fix connected/active CSI-2 lane reporting

[Ian Arkver]

On 21/09/17 12:04, Ian Arkver wrote:

Hi Philipp

On 21/09/17 11:24, Philipp Zabel wrote:

g_mbus_config was supposed to indicate all supported lane numbers, not
only the number of those currently in active use. Since the tc358743
can dynamically reduce the number of active lanes if the required
bandwidth allows for it, report all lane numbers up to the connected
number of lanes as supported.
To allow communicating the number of currently active lanes, add a new
bitfield to the v4l2_mbus_config flags. This is a temporary fix, until
a better solution is found.

Signed-off-by: Philipp Zabel 
---
  drivers/media/i2c/tc358743.c  | 22 --
  include/media/v4l2-mediabus.h |  8 
  2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index e6f5c363ccab5..e2a9e6a18a49d 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1464,21 +1464,22 @@ static int tc358743_g_mbus_config(struct 
v4l2_subdev *sd,
  /* Support for non-continuous CSI-2 clock is missing in the 
driver */

  cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
-    switch (state->csi_lanes_in_use) {
-    case 1:
+    if (state->bus.num_data_lanes > 0)
  cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
-    break;
-    case 2:
+    if (state->bus.num_data_lanes > 1)
  cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
-    break;
-    case 3:
+    if (state->bus.num_data_lanes > 2)
  cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
-    break;
-    case 4:
+    if (state->bus.num_data_lanes > 3)
  cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
-    break;
-    default:
+
+    if (state->csi_lanes_in_use > 4)
  return -EINVAL;
+


My understanding of Hans' comment:
"I'd also add a comment that all other flags must be 0 if the device 
tree is used. This to avoid mixing the two."


is that all the above should only happen if (!!state->pdata).


Except that state->pdata is a copy of the pdata, not a pointer, but you 
know what I mean. Some other check for DT needed here.



I don't know if this would break any existing DT-using bridge drivers.

Regards,
Ian

[snip]

Re: [PATCH] tc358743: fix connected/active CSI-2 lane reporting

[Ian Arkver]

Hi Philipp

On 21/09/17 11:24, Philipp Zabel wrote:

g_mbus_config was supposed to indicate all supported lane numbers, not
only the number of those currently in active use. Since the tc358743
can dynamically reduce the number of active lanes if the required
bandwidth allows for it, report all lane numbers up to the connected
number of lanes as supported.
To allow communicating the number of currently active lanes, add a new
bitfield to the v4l2_mbus_config flags. This is a temporary fix, until
a better solution is found.

Signed-off-by: Philipp Zabel 
---
  drivers/media/i2c/tc358743.c  | 22 --
  include/media/v4l2-mediabus.h |  8 
  2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index e6f5c363ccab5..e2a9e6a18a49d 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1464,21 +1464,22 @@ static int tc358743_g_mbus_config(struct v4l2_subdev 
*sd,
/* Support for non-continuous CSI-2 clock is missing in the driver */
cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
  
-	switch (state->csi_lanes_in_use) {

-   case 1:
+   if (state->bus.num_data_lanes > 0)
cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
-   break;
-   case 2:
+   if (state->bus.num_data_lanes > 1)
cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
-   break;
-   case 3:
+   if (state->bus.num_data_lanes > 2)
cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
-   break;
-   case 4:
+   if (state->bus.num_data_lanes > 3)
cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
-   break;
-   default:
+
+   if (state->csi_lanes_in_use > 4)
return -EINVAL;
+


My understanding of Hans' comment:
"I'd also add a comment that all other flags must be 0 if the device 
tree is used. This to avoid mixing the two."


is that all the above should only happen if (!!state->pdata).

I don't know if this would break any existing DT-using bridge drivers.

Regards,
Ian


+   if (state->csi_lanes_in_use < state->bus.num_data_lanes) {
+   const u32 mask = V4L2_MBUS_CSI2_LANE_MASK;
+
+   cfg->flags |= (state->csi_lanes_in_use << __ffs(mask)) & mask;
}
  
  	return 0;

@@ -1885,6 +1886,7 @@ static int tc358743_probe(struct i2c_client *client,
if (pdata) {
state->pdata = *pdata;
state->bus.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+   state->bus.num_data_lanes = 4;
} else {
err = tc358743_probe_of(state);
if (err == -ENODEV)
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 93f8afcb7a220..3467d97be5f5b 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -63,6 +63,14 @@
 V4L2_MBUS_CSI2_3_LANE | 
V4L2_MBUS_CSI2_4_LANE)
  #define V4L2_MBUS_CSI2_CHANNELS   (V4L2_MBUS_CSI2_CHANNEL_0 | 
V4L2_MBUS_CSI2_CHANNEL_1 | \
 V4L2_MBUS_CSI2_CHANNEL_2 | 
V4L2_MBUS_CSI2_CHANNEL_3)
+/*
+ * Number of lanes in use, 0 == use all available lanes (default)
+ *
+ * This is a temporary fix for devices that need to reduce the number of active
+ * lanes for certain modes, until g_mbus_config() can be replaced with a better
+ * solution.
+ */
+#define V4L2_MBUS_CSI2_LANE_MASK(3 << 10)
  
  /**

   * enum v4l2_mbus_type - media bus type

[PATCH] tc358743: fix connected/active CSI-2 lane reporting

[Philipp Zabel]

g_mbus_config was supposed to indicate all supported lane numbers, not
only the number of those currently in active use. Since the tc358743
can dynamically reduce the number of active lanes if the required
bandwidth allows for it, report all lane numbers up to the connected
number of lanes as supported.
To allow communicating the number of currently active lanes, add a new
bitfield to the v4l2_mbus_config flags. This is a temporary fix, until
a better solution is found.

Signed-off-by: Philipp Zabel 
---
 drivers/media/i2c/tc358743.c  | 22 --
 include/media/v4l2-mediabus.h |  8 
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c
index e6f5c363ccab5..e2a9e6a18a49d 100644
--- a/drivers/media/i2c/tc358743.c
+++ b/drivers/media/i2c/tc358743.c
@@ -1464,21 +1464,22 @@ static int tc358743_g_mbus_config(struct v4l2_subdev 
*sd,
/* Support for non-continuous CSI-2 clock is missing in the driver */
cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
 
-   switch (state->csi_lanes_in_use) {
-   case 1:
+   if (state->bus.num_data_lanes > 0)
cfg->flags |= V4L2_MBUS_CSI2_1_LANE;
-   break;
-   case 2:
+   if (state->bus.num_data_lanes > 1)
cfg->flags |= V4L2_MBUS_CSI2_2_LANE;
-   break;
-   case 3:
+   if (state->bus.num_data_lanes > 2)
cfg->flags |= V4L2_MBUS_CSI2_3_LANE;
-   break;
-   case 4:
+   if (state->bus.num_data_lanes > 3)
cfg->flags |= V4L2_MBUS_CSI2_4_LANE;
-   break;
-   default:
+
+   if (state->csi_lanes_in_use > 4)
return -EINVAL;
+
+   if (state->csi_lanes_in_use < state->bus.num_data_lanes) {
+   const u32 mask = V4L2_MBUS_CSI2_LANE_MASK;
+
+   cfg->flags |= (state->csi_lanes_in_use << __ffs(mask)) & mask;
}
 
return 0;
@@ -1885,6 +1886,7 @@ static int tc358743_probe(struct i2c_client *client,
if (pdata) {
state->pdata = *pdata;
state->bus.flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
+   state->bus.num_data_lanes = 4;
} else {
err = tc358743_probe_of(state);
if (err == -ENODEV)
diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
index 93f8afcb7a220..3467d97be5f5b 100644
--- a/include/media/v4l2-mediabus.h
+++ b/include/media/v4l2-mediabus.h
@@ -63,6 +63,14 @@
 V4L2_MBUS_CSI2_3_LANE | 
V4L2_MBUS_CSI2_4_LANE)
 #define V4L2_MBUS_CSI2_CHANNELS(V4L2_MBUS_CSI2_CHANNEL_0 | 
V4L2_MBUS_CSI2_CHANNEL_1 | \
 V4L2_MBUS_CSI2_CHANNEL_2 | 
V4L2_MBUS_CSI2_CHANNEL_3)
+/*
+ * Number of lanes in use, 0 == use all available lanes (default)
+ *
+ * This is a temporary fix for devices that need to reduce the number of active
+ * lanes for certain modes, until g_mbus_config() can be replaced with a better
+ * solution.
+ */
+#define V4L2_MBUS_CSI2_LANE_MASK(3 << 10)
 
 /**
  * enum v4l2_mbus_type - media bus type
-- 
2.11.0

Re: [RESEND RFC PATCH 0/7] sun8i H3 HDMI glue driver for DW HDMI

[Hans Verkuil]

On 09/21/17 11:39, Jose Abreu wrote:
> Hi Jernej,
> 
> On 20-09-2017 21:01, Jernej Skrabec wrote:
>> [added media mailing list due to CEC question]
>>
>> This patch series adds a HDMI glue driver for Allwinner H3 SoC. For now, only
>> video and CEC functionality is supported. Audio needs more tweaks.
>>
>> Series is based on the H3 DE2 patch series available on mailing list:
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.infradead.org_pipermail_linux-2Darm-2Dkernel_2017-2DAugust_522697.html=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw=coyfcQKSr2asrHcaCeWFmAP_9nkFkRK8s7Uw5bmVei4=JCFaMXK1MmZ3jE745_YcqZhZkaqtc6UapGfSSapcz_s=
>>  
>> (ignore patches marked with [NOT FOR REVIEW NOW] tag)
>>
>> Patch 1 adds support for polling plug detection since custom PHY used here
>> doesn't support HPD interrupt.
>>
>> Patch 2 enables overflow workaround for v1.32a. This HDMI controller exhibits
>> same issues as HDMI controller used in iMX6 SoCs.
>>
>> Patch 3 adds CLK_SET_RATE_PARENT to hdmi clock.
>>
>> Patch 4 adds dt bindings documentation.
>>
>> Patch 5 adds actual H3 HDMI glue driver.
>>
>> Patch 6 and 7 add HDMI node to DT and enable it where needed.
>>
>> Allwinner used DW HDMI controller in a non standard way:
>> - register offsets obfuscation layer, which can fortunately be turned off
>> - register read lock, which has to be disabled by magic number
>> - custom PHY, which have to be initialized before DW HDMI controller
>> - non standard clocks
>> - no HPD interrupt
>>
>> Because of that, I have two questions:
>> - Since HPD have to be polled, is it enough just to enable poll mode? I'm
>>   mainly concerned about invalidating CEC address here.
> 
> You mean you get no interrupt when HPD status changes? Hans can
> answer this better but then you will need to invalidate the cec
> physical address yourself because right now its invalidated in
> the dw-hdmi irq handler (see dw_hdmi_irq()).

That's correct. When the HPD goes low you need to call 
cec_notifier_phys_addr_invalidate()
to invalidate the physical address. This is not terribly time sensitive, i.e.
checking this once a second would be quick enough.

Regards,

Hans

Re: [RESEND RFC PATCH 0/7] sun8i H3 HDMI glue driver for DW HDMI

[Jose Abreu]

Hi Jernej,

On 20-09-2017 21:01, Jernej Skrabec wrote:
> [added media mailing list due to CEC question]
>
> This patch series adds a HDMI glue driver for Allwinner H3 SoC. For now, only
> video and CEC functionality is supported. Audio needs more tweaks.
>
> Series is based on the H3 DE2 patch series available on mailing list:
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.infradead.org_pipermail_linux-2Darm-2Dkernel_2017-2DAugust_522697.html=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw=coyfcQKSr2asrHcaCeWFmAP_9nkFkRK8s7Uw5bmVei4=JCFaMXK1MmZ3jE745_YcqZhZkaqtc6UapGfSSapcz_s=
>  
> (ignore patches marked with [NOT FOR REVIEW NOW] tag)
>
> Patch 1 adds support for polling plug detection since custom PHY used here
> doesn't support HPD interrupt.
>
> Patch 2 enables overflow workaround for v1.32a. This HDMI controller exhibits
> same issues as HDMI controller used in iMX6 SoCs.
>
> Patch 3 adds CLK_SET_RATE_PARENT to hdmi clock.
>
> Patch 4 adds dt bindings documentation.
>
> Patch 5 adds actual H3 HDMI glue driver.
>
> Patch 6 and 7 add HDMI node to DT and enable it where needed.
>
> Allwinner used DW HDMI controller in a non standard way:
> - register offsets obfuscation layer, which can fortunately be turned off
> - register read lock, which has to be disabled by magic number
> - custom PHY, which have to be initialized before DW HDMI controller
> - non standard clocks
> - no HPD interrupt
>
> Because of that, I have two questions:
> - Since HPD have to be polled, is it enough just to enable poll mode? I'm
>   mainly concerned about invalidating CEC address here.

You mean you get no interrupt when HPD status changes? Hans can
answer this better but then you will need to invalidate the cec
physical address yourself because right now its invalidated in
the dw-hdmi irq handler (see dw_hdmi_irq()).

> - PHY has to be initialized before DW HDMI controller to disable offset
>   obfuscation and read lock among other things. This means that all clocks 
> have
>   to be enabled in glue driver. This poses a problem, since when using
>   component model, dw-hdmi bridge uses drvdata for it's own private data and
>   prevents glue layer to pass a pointer to unbind function, where clocks 
> should
>   be disabled. I noticed same issue in meson DW HDMI glue driver, where clocks
>   are also not disabled when unbind callback is called. I noticed that when H3
>   SoC is shutdown, HDMI output is still enabled and lastest image is shown on
>   monitor until it is unplugged from power supply. Is there any simple 
> solution
>   to this?

I don't know if you can use an empty platform device created with
platform_device_alloc(). Perhaps it would be better fix this in
the dw-hdmi driver. I see two solutions:

- Either you return the dw-hdmi private structure in the bind
callback, store it and pass it in the unbind
- Or, you pass your own private data to the dw-hdmi bind, the
dw-hdmi stores it and you just create a public function in the
dw-hdmi driver called like dw_hdmi_get_auxdata(struct device
*dev) which returns your private data.

I think first option is nice, maybe anyone else can suggest
something better?

Best regards,
Jose Miguel Abreu

>
> Chen-Yu,
> TL Lim was unable to obtain any answer from Allwinner about HDMI clocks. I 
> think
> it is safe to assume that divider in HDMI clock doesn't have any effect.
>
> Branch based on linux-next from 1. September with integrated patches is
> available here:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_jernejsk_linux-2D1_tree_h3-5Fhdmi-5Frfc=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw=coyfcQKSr2asrHcaCeWFmAP_9nkFkRK8s7Uw5bmVei4=lDAnd3egsc2sxqVM-Ya_Me9ozWXKWvxxvsdV3Jn3vpA=
>  
>
> Some additonal info about H3 HDMI:
> https://urldefense.proofpoint.com/v2/url?u=https-3A__linux-2Dsunxi.org_DWC-5FHDMI-5FController=DwIBAg=DPL6_X_6JkXFx7AXWqB0tg=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw=coyfcQKSr2asrHcaCeWFmAP_9nkFkRK8s7Uw5bmVei4=d9iEgk23RCLJL4oXJ4kkt6NyYK90_vFy0mCD3WauJDk=
>  
>
> Thanks to Jens Kuske, who figured out that it is actually DW HDMI controller
> and mapped scrambled register offsets to original ones.
>
> Icenowy Zheng (1):
>   ARM: sun8i: h3: Add DesignWare HDMI controller node
>
> Jernej Skrabec (6):
>   drm: bridge: Enable polling hpd event in dw_hdmi
>   drm: bridge: Enable workaround in dw_hdmi for v1.32a
>   clk: sunxi: Add CLK_SET_RATE_PARENT flag for H3 HDMI clock
>   dt-bindings: Document Allwinner DWC HDMI TX node
>   drm: sun4i: Add a glue for the DesignWare HDMI controller in H3
>   ARM: sun8i: h3: Enable HDMI output on H3 boards
>
>  .../bindings/display/sunxi/sun4i-drm.txt   | 158 ++-
>  arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts|  33 ++
>  arch/arm/boot/dts/sun8i-h3-beelink-x2.dts  |  33 ++
>  arch/arm/boot/dts/sun8i-h3-nanopi-m1.dts   |  33 ++
>  arch/arm/boot/dts/sun8i-h3-orangepi-2.dts  |  33 ++
>  

Re: [PATCH 1/2] dt: bindings: media: Document port and endpoint numbering

[Sakari Ailus]

Hi Rob,

Thanks for the reply.

On Wed, Sep 20, 2017 at 03:53:13PM -0500, Rob Herring wrote:
> On Mon, Sep 18, 2017 at 11:25:04AM +0300, Sakari Ailus wrote:
> > A lot of devices do not need and do not document port or endpoint
> > numbering at all, e.g. in case where there's just a single port and a
> > single endpoint. Whereas this is just common sense, document it to make it
> > explicit.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  Documentation/devicetree/bindings/media/video-interfaces.txt | 12 
> > 
> >  1 file changed, 12 insertions(+)
> 
> This is fine, but bindings should still be explicit. Otherwise, I'm 
> wondering if it's a single port/endpoint or they just forgot to document 
> it. And I shouldn't have to look at the example to infer that.
> 
> Acked-by: Rob Herring 

The purpose of the patch was to actually document port and endpoint
numbering for devices for which it is not documented, not to suggest that
this would be omitted in in binding documentation. The fact is that I
couldn't find documentation for endpoint numbering for a single device
under Documentation/devicetree/bindings/media/ . Yet I haven't come across
DT source where other than zero would have been used. And the drivers
(mostly?) have ignored endpoint numbers so far.

Some bindings have been omitted on the grounds that they're documented in
video-interfaces.txt.

What would you think of the following? I'm not sure it'd really belong
there, but it'd be a small piece of documentation one can easily refer to.


>From e735979005244eb10597fe5333130b93e41d5a38 Mon Sep 17 00:00:00 2001
From: Sakari Ailus 
Date: Mon, 18 Sep 2017 11:15:53 +0300
Subject: [PATCH 1/1] dt: bindings: media: Document practices for DT bindings,
 ports, endpoints

Port and endpoint numbering has been omitted in DT binding documentation
for a large number of devices. Also common properties the device uses have
been missed in binding documentation. Make it explicit that these things
need to be documented.

Signed-off-by: Sakari Ailus 
---
 .../devicetree/bindings/media/video-interfaces.txt| 15 +++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 852041a..3c5382f 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -55,6 +55,21 @@ divided into two separate ITU-R BT.656 8-bit busses.  In 
such case bus-width
 and data-shift properties can be used to assign physical data lines to each
 endpoint node (logical bus).
 
+Documenting bindings for devices
+
+
+All required and optional bindings the device supports shall be explicitly
+documented in device DT binding documentation. This also includes port and
+endpoint numbering for the device.
+
+Port and endpoint numbering
+---
+
+Old binding documentation may have omitted explicitly specifying port and
+endpoint numbers. This often applies to devices that have a single port and a
+single endpoint in that port. In this case, the only valid port number for such
+a device is zero. The same applies for devices for which bindings do not
+document endpoint numbering: only zero is a valid endpoint.
 
 Required properties
 ---
-- 
2.7.4

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

Re: [media] s2255drv: Adjust 13 checks for null pointers

[Dan Carpenter]

On Thu, Sep 21, 2017 at 10:12:56AM +0200, SF Markus Elfring wrote:
> >> MIME-Version: 1.0
> >> Content-Type: text/plain; charset=UTF-8
> >> Content-Transfer-Encoding: 8bit
> >>
> > 
> > You've been told several times that this stuff doesn't work.
> 
> This functionality might not exactly work in the way that you expect so far.
> 
> 
> > Try applying this patch with `git am` and you'll see why.
> 
> I find that these extra message fields work in the way that was designed
> by the Git software developers.
> 
> elfring@Sonne:~/Projekte/Linux/next-patched> LANG=C git checkout -b 
> next_deletion_of_oom_messages_in_s2255drv_test_20170921 
> next_deletion_of_oom_messages-20170905 && LANG=C git am '../[PATCH 2_5] 
> [media] s2255drv: Adjust 13 checks for null pointers.eml'
> Switched to a new branch 
> 'next_deletion_of_oom_messages_in_s2255drv_test_20170921'
> Applying: s2255drv: Adjust 13 checks for null pointers
> 
> 
> Would you like to clarify corresponding concerns any more?
> 

Look at the `git log` and it just copies those lines:

commit 2a47170a824697783d8c2d28355a806f075c76e4 (HEAD)
Author: Markus Elfring 
Date:   Wed Sep 20 16:46:19 2017 +0200

s2255drv: Adjust 13 checks for null pointers

MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written !…

Thus fix the affected source code places.


regards,
dan carpenter

Re: [PATCH 1/3] [media] tc358743: Correct clock mode reported in g_mbus_config

[Philipp Zabel]

On Tue, 2017-09-19 at 14:08 +0100, Dave Stevenson wrote:
> Support for non-continuous clock had previously been added via
> device tree, but a comment and the value reported by g_mbus_config
> still stated that it wasn't supported.
> Remove the comment, and return the correct value in g_mbus_config.
> 
> Signed-off-by: Dave Stevenson 
> ---
>  drivers/media/i2c/tc358743.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/tc358743.c
> b/drivers/media/i2c/tc358743.c
> index e6f5c36..6b0fd07 100644
> --- a/drivers/media/i2c/tc358743.c
> +++ b/drivers/media/i2c/tc358743.c
> @@ -1461,8 +1461,9 @@ static int tc358743_g_mbus_config(struct
> v4l2_subdev *sd,
>  
>   cfg->type = V4L2_MBUS_CSI2;
>  
> - /* Support for non-continuous CSI-2 clock is missing in the
> driver */
> - cfg->flags = V4L2_MBUS_CSI2_CONTINUOUS_CLOCK;
> + cfg->flags = state->bus.flags &
> + (V4L2_MBUS_CSI2_CONTINUOUS_CLOCK |
> +  V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK);

Acked-by: Philipp Zabel 

regards
Philipp

Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.

[Philipp Zabel]

Hi Hans,

On Wed, 2017-09-20 at 15:12 +0200, Hans Verkuil wrote:
[...]
> I don't like it :-)
> 
> Currently g_mbus_config returns (and I quote from v4l2-mediabus.h): "How
> many lanes the client can use". I.e. the capabilities of the HW.
> 
> If we are going to use this to communicate how many lines are currently
> in use, then I would propose that we add a lane mask, i.e. something like
> this:
> 
> /* Number of lanes in use, 0 == use all available lanes (default) */
> #define V4L2_MBUS_CSI2_LANE_MASK(3 << 10)
> 
> And add comments along the lines that this is a temporary fix.
> 
> I would feel a lot happier (or a lot less unhappy) if we'd do it this way.
> Rather than re-interpreting bits that are not quite what they should be.
> 
> I'd also add a comment that all other flags must be 0 if the device tree is
> used. This to avoid mixing the two.

I would like to try this.

Currently the driver sets the V4L2_MBUS_CSI2_[1-4]_LANE bits according
to csi_lanes_in_use, which is wrong as you say.

After moving the csi_lanes_in_use info into a new
V4L2_MBUS_CSI2_LANE_MASK bitfield, the V4L2_MBUS_CSI2_[1-4]_LANE bits
could be either set to zero or to the really connected lanes as
configured in the device tree (csi->bus.num_data_lanes) in the DT case.

What would the bits be set to in the pdata case, though? Should a lane
count setting be added to tc358743_platform_data with, defaulting to all
bits set?

regards
Philipp

Re: [PATCH v13 06/25] omap3isp: Use generic parser for parsing fwnode endpoints

[Sakari Ailus]

Hi Laurent,

On Tue, Sep 19, 2017 at 07:12:14PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tuesday, 19 September 2017 17:47:22 EEST Sakari Ailus wrote:
> > On Tue, Sep 19, 2017 at 03:46:18PM +0300, Laurent Pinchart wrote:
> > > On Tuesday, 19 September 2017 15:43:26 EEST Sakari Ailus wrote:
> > >> On Tue, Sep 19, 2017 at 02:40:29PM +0300, Laurent Pinchart wrote:
> >  @@ -2256,7 +2210,9 @@ static int isp_probe(struct platform_device
> >  *pdev)
> > if (ret)
> > return ret;
> >  
> >  -  ret = isp_fwnodes_parse(>dev, >notifier);
> >  +  ret = v4l2_async_notifier_parse_fwnode_endpoints(
> >  +  >dev, >notifier, sizeof(struct 
> >  isp_async_subdev),
> >  +  isp_fwnode_parse);
> > if (ret < 0)
> > >>> 
> > >>> The documentation in patch 05/25 states that
> > >>> v4l2_async_notifier_release() should be called even if
> > >>> v4l2_async_notifier_parse_fwnode_endpoints() fails. I don't think that's
> > >>> needed here, so you might want to update the documentation (and possibly
> > >>> the implementation of the function).
> > >> 
> > >> It is. If parsing fails, async sub-devices may have been already set up.
> > >> This happens e.g. when the parsing fails after the first one has been
> > >> successfully set up already.
> > > 
> > > But for v4l2_async_notifier_parse_fwnode_endpoints() we could clean up
> > > internally when an error occurs. Otherwise you need to call
> > > v4l2_async_notifier_release() here.
> > 
> > The functions that set up async sub-devices can be called multiple times
> > (on separate references). This is quite alike setting up a control handler
> > really, so I adopted the same pattern.
> > 
> > If there is a failure, how many async sub-devices should be cleaned up, if
> > there have been async sub-devices already set up before calling this
> > function?
> 
> I'm not opposed to that pattern, I just thought that cleanup could be 
> automated for v4l2_async_notifier_parse_fwnode_endpoints() failures, as 
> opposed to v4l2_async_notifier_parse_fwnode_endpoints_by_port() failures.
> 
> As this patch, written by the author of 
> v4l2_async_notifier_parse_fwnode_endpoints() and part of the same patch 
> series, is missing a call to v4l2_async_notifier_release(), I expect driver 
> authors to make the same mistake and was thus wondering how to prevent that.
> 
> I believe that the issue here is that initialization of the notifier is done 
> implicitly by the first call to a parsing function. With explicit 
> notification 
> it should be clear to driver authors that they need to call the cleanup 
> function:
> 
> ret = init()
> if (ret)
>   return ret;
> 
> ret = parse()
> if (ret) {
>   cleanup();
>   return ret;
> }
> 
> ret = parse()
> if (ret) {
>   cleanup();
>   return ret;
> }
> 
> ret = parse()
> if (ret) {
>   cleanup();
>   return ret;
> }
> 
> But with an implicit initialization it's easy to miss cleanup when 
> v4l2_async_notifier_parse_fwnode_endpoints() fails, as that function can be 
> considered as an initilization function that performs cleanup internally.
> 
> I'm not sure what the best pattern would be. At the very least you need to 
> fix 
> this patch, but that wouldn't prevent future mistakes.

The patch was actually originally written before this was apparent. I agree
that it is not a common pattern to require cleanup function to be called on
a failure.

I'll fix the patch for v14.

Going forward, I see no reason why we couldn't automate much of this for
drivers: the bindings are generic after all. The notifier could be
registered through registration of the v4l2_device. It could be simply
released immediately if there would be no async sub-devices around.

It's not a trivial change, and I'd think well out of scope of this set:
we'll need to move link creation to the framework and detach async
sub-deviecs from parsing the endpoint properties as well.

-- 
Kind regards,

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

[PATCH] [media] cx231xx-cards: fix NULL-deref on missing association descriptor

[Johan Hovold]

Make sure to check that we actually have an Interface Association
Descriptor before dereferencing it during probe to avoid dereferencing a
NULL-pointer.

Fixes: e0d3bafd0258 ("V4L/DVB (10954): Add cx231xx USB driver")
Cc: stable  # 2.6.30
Cc: Sri Deevi 
Reported-by: Andrey Konovalov 
Signed-off-by: Johan Hovold 
---
 drivers/media/usb/cx231xx/cx231xx-cards.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c 
b/drivers/media/usb/cx231xx/cx231xx-cards.c
index e0daa9b6c2a0..9b742d569fb5 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -1684,7 +1684,7 @@ static int cx231xx_usb_probe(struct usb_interface 
*interface,
nr = dev->devno;
 
assoc_desc = udev->actconfig->intf_assoc[0];
-   if (assoc_desc->bFirstInterface != ifnum) {
+   if (!assoc_desc || assoc_desc->bFirstInterface != ifnum) {
dev_err(d, "Not found matching IAD interface\n");
retval = -ENODEV;
goto err_if;
-- 
2.14.1

Re: usb/media/cx231xx: null-ptr-deref in cx231xx_usb_probe

[Johan Hovold]

On Wed, Sep 20, 2017 at 08:54:08PM +0200, Andrey Konovalov wrote:
> Hi!
> 
> I've got the following report while fuzzing the kernel with syzkaller.
> 
> On commit ebb2c2437d8008d46796902ff390653822af6cc4 (Sep 18).
> 
> The null-ptr-deref happens on assoc_desc->bFirstInterface, where
> assoc_desc = udev->actconfig->intf_assoc[0]. There seems to be no
> check that the device actually contains an Interface Association
> Descriptor.

That is indeed a bug; I'll respond to this mail with a fix.

Thanks,
Johan

Re: [media] s2255drv: Adjust 13 checks for null pointers

[SF Markus Elfring]

>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
> 
> You've been told several times that this stuff doesn't work.

This functionality might not exactly work in the way that you expect so far.


> Try applying this patch with `git am` and you'll see why.

I find that these extra message fields work in the way that was designed
by the Git software developers.

elfring@Sonne:~/Projekte/Linux/next-patched> LANG=C git checkout -b 
next_deletion_of_oom_messages_in_s2255drv_test_20170921 
next_deletion_of_oom_messages-20170905 && LANG=C git am '../[PATCH 2_5] [media] 
s2255drv: Adjust 13 checks for null pointers.eml'
Switched to a new branch 
'next_deletion_of_oom_messages_in_s2255drv_test_20170921'
Applying: s2255drv: Adjust 13 checks for null pointers


Would you like to clarify corresponding concerns any more?

Regards,
Markus

Hauppauge Nova-TD Stick fails on f26

[Eyal Lebedinsky]

I have an unusual problem with this tuner. On my workstation (f26) it works 
fine and I can watch
TV using vlc or tune with tzap.

On my mythtv server (which was recently upgraded to f26) the card does not 
work. An adapter is not created
in /dev/dvb.

The server has a few other tuners and it may be some interference causing the 
failure, but the reason
is beyond me.

This is what is loaded after the tuner is plugged in:

$ sudo lsmod|grep dvb
dvb_usb_dib0700   151552  0
dib7000m   24576  1 dvb_usb_dib0700
dib009040960  1 dvb_usb_dib0700
dib007020480  1 dvb_usb_dib0700
dib3000mc  20480  1 dvb_usb_dib0700
dibx000_common 16384  4 dib7000p,dvb_usb_dib0700,dib7000m,dib3000mc
dvb_usb24576  1 dvb_usb_dib0700
dvb_usb_af9035 40960  0
dvb_usb_rtl28xxu   40960  0
dvb_usb_v2 40960  2 dvb_usb_af9035,dvb_usb_rtl28xxu
dvb_core  126976  5 dib7000p,dvb_usb,dvb_usb_v2,rtl2832,af9033
rc_core36864  16 
dvb_usb_af9035,ir_nec_decoder,dvb_usb_dib0700,ir_lirc_codec,dvb_usb,lirc_dev,dvb_usb_v2,nuvoton_cir,dvb_usb_rtl28xxu,rc_leadtek_y04g0051

This is the failure on the server:

kernel: usb 8-2: new high-speed USB device number 3 using xhci_hcd
kernel: usb 8-2: New USB device found, idVendor=2040, idProduct=9580
kernel: usb 8-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
kernel: usb 8-2: Product: NovaT 500Stick
kernel: usb 8-2: Manufacturer: Hauppauge
kernel: usb 8-2: SerialNumber: 4027868935
mtp-probe[19770]: checking bus 8, device 3: 
"/sys/devices/pci:00/:00:14.0/usb8/8-2"
mtp-probe[19770]: bus: 8, device: 3 was not an MTP device
kernel: dvb-usb: found a 'Hauppauge Nova-TD Stick/Elgato Eye-TV Diversity' in 
cold state, will try to load a firmware
kernel: dvb-usb: downloading firmware from file 'dvb-usb-dib0700-1.20.fw'
kernel: dib0700: firmware started successfully.
kernel: dvb-usb: found a 'Hauppauge Nova-TD Stick/Elgato Eye-TV Diversity' in 
warm state.
kernel: dvb-usb: will pass the complete MPEG2 transport stream to the software 
demuxer.
kernel: dvbdev: DVB: registering new adapter (Hauppauge Nova-TD Stick/Elgato 
Eye-TV Diversity)
baloo_file[2975]: QObject::connect: invalid null parameter
baloo_file[2975]: QObject::connect: invalid null parameter
baloo_file[2975]: QObject::connect: invalid null parameter
kernel: usb 8-2: DVB: registering adapter 7 frontend 0 (DiBcom 7000PC)...
### note: /dev/dvb/adapter7 was present for about 1 second and then 
disappeared
baloo_file[2975]: QObject::connect: invalid null parameter
kernel: MT2266: successfully identified
kernel: dvb-usb: will pass the complete MPEG2 transport stream to the software 
demuxer.
kernel: dvb-usb: Hauppauge Nova-TD Stick/Elgato Eye-TV Diversity error while 
loading driver (-23)
kernel: usbcore: registered new interface driver dvb_usb_dib0700

For comparo, here is a successful result on the workstation:

kernel: usb 2-3: new high-speed USB device number 3 using ehci-pci
kernel: usb 2-3: New USB device found, idVendor=2040, idProduct=9580
kernel: usb 2-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
kernel: usb 2-3: Product: NovaT 500Stick
kernel: usb 2-3: Manufacturer: Hauppauge
kernel: usb 2-3: SerialNumber: 4027868935
mtp-probe[17719]: checking bus 2, device 3: 
"/sys/devices/pci:00/:00:1d.7/usb2/2-3"
mtp-probe[17719]: bus: 2, device: 3 was not an MTP device
kernel: dvb-usb: found a 'Hauppauge Nova-TD Stick/Elgato Eye-TV Diversity' in 
cold state, will try to load a firmware
kernel: dvb-usb: downloading firmware from file 'dvb-usb-dib0700-1.20.fw'
kernel: dib0700: firmware started successfully.
kernel: dvb-usb: found a 'Hauppauge Nova-TD Stick/Elgato Eye-TV Diversity' in 
warm state.
kernel: dvb-usb: will pass the complete MPEG2 transport stream to the software 
demuxer.
kernel: dvbdev: DVB: registering new adapter (Hauppauge Nova-TD Stick/Elgato 
Eye-TV Diversity)
baloo_file[2874]: QObject::connect: invalid null parameter
baloo_file[2874]: QObject::connect: invalid null parameter
baloo_file[2874]: QObject::connect: invalid null parameter
kernel: usb 2-3: DVB: registering adapter 0 frontend 0 (DiBcom 7000PC)...
baloo_file[2874]: QObject::connect: invalid null parameter
kernel: MT2266: successfully identified
kernel: dvb-usb: will pass the complete MPEG2 transport stream to the software 
demuxer.
kernel: dvbdev: DVB: registering new adapter (Hauppauge Nova-TD Stick/Elgato 
Eye-TV Diversity)
baloo_file[2874]: QObject::connect: invalid null parameter
baloo_file[2874]: QObject::connect: invalid null parameter
baloo_file[2874]: QObject::connect: invalid null parameter
kernel: usb 2-3: DVB: registering adapter 1 frontend 0 (DiBcom 7000PC)...
baloo_file[2874]: QObject::connect: invalid null parameter
kernel: MT2266: successfully identified
kernel: rc rc0: IR-receiver inside an USB DVB receiver as 
/devices/pci:00/:00:1d.7/usb2/2-3/rc/rc0
kernel: Registered IR keymap 

Re: [PATCH] dma-fence: fix dma_fence_get_rcu_safe

[Maarten Lankhorst]

Op 21-09-17 om 09:00 schreef Christian König:
> Am 20.09.2017 um 20:20 schrieb Daniel Vetter:
>> On Mon, Sep 11, 2017 at 01:06:32PM +0200, Christian König wrote:
>>> Am 11.09.2017 um 12:01 schrieb Chris Wilson:
 [SNIP]
> Yeah, but that is illegal with a fence objects.
>
> When anybody allocates fences this way it breaks at least
> reservation_object_get_fences_rcu(),
> reservation_object_wait_timeout_rcu() and
> reservation_object_test_signaled_single().
 Many, many months ago I sent patches to fix them all.
>>> Found those after a bit a searching. Yeah, those patches where proposed more
>>> than a year ago, but never pushed upstream.
>>>
>>> Not sure if we really should go this way. dma_fence objects are shared
>>> between drivers and since we can't judge if it's the correct fence based on
>>> a criteria in the object (only the read counter which is outside) all
>>> drivers need to be correct for this.
>>>
>>> I would rather go the way and change dma_fence_release() to wrap
>>> fence->ops->release into call_rcu() to keep the whole RCU handling outside
>>> of the individual drivers.
>> Hm, I entirely dropped the ball on this, I kinda assumed that we managed
>> to get some agreement on this between i915 and dma_fence. Adding a pile
>> more people.
>
> For the meantime I've send a v2 of this patch to fix at least the buggy 
> return of NULL when we fail to grab the RCU reference but keeping the extra 
> checking for now.
>
> Can I get an rb on this please so that we fix at least the bug at hand?
>
> Thanks,
> Christian. 
Done.

Re: [PATCH] dma-fence: fix dma_fence_get_rcu_safe v2

[Maarten Lankhorst]

Op 15-09-17 om 11:53 schreef Christian König:
> From: Christian König 
>
> When dma_fence_get_rcu() fails to acquire a reference it doesn't necessary
> mean that there is no fence at all.
>
> It usually mean that the fence was replaced by a new one and in this situation
> we certainly want to have the new one as result and *NOT* NULL.
>
> v2: Keep extra check after dma_fence_get_rcu().
>
> Signed-off-by: Christian König 
> Cc: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Sumit Semwal 
> Cc: linux-media@vger.kernel.org
> Cc: dri-de...@lists.freedesktop.org
> Cc: linaro-mm-...@lists.linaro.org
> ---
>  include/linux/dma-fence.h | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 0a186c4..f4f23cb 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -248,9 +248,12 @@ dma_fence_get_rcu_safe(struct dma_fence * __rcu *fencep)
>   struct dma_fence *fence;
>  
>   fence = rcu_dereference(*fencep);
> - if (!fence || !dma_fence_get_rcu(fence))
> + if (!fence)
>   return NULL;
>  
> + if (!dma_fence_get_rcu(fence))
> + continue;
> +
>   /* The atomic_inc_not_zero() inside dma_fence_get_rcu()
>* provides a full memory barrier upon success (such as now).
>* This is paired with the write barrier from assigning

Should be safe from an infinite loop since the old fence is only unreffed after 
the new pointer is written, so we'll always make progress. :)

Reviewed-by: Maarten Lankhorst 

Re: [PATCH] dma-fence: fix dma_fence_get_rcu_safe

[Christian König]

Am 20.09.2017 um 20:20 schrieb Daniel Vetter:

On Mon, Sep 11, 2017 at 01:06:32PM +0200, Christian König wrote:

Am 11.09.2017 um 12:01 schrieb Chris Wilson:

[SNIP]

Yeah, but that is illegal with a fence objects.

When anybody allocates fences this way it breaks at least
reservation_object_get_fences_rcu(),
reservation_object_wait_timeout_rcu() and
reservation_object_test_signaled_single().

Many, many months ago I sent patches to fix them all.

Found those after a bit a searching. Yeah, those patches where proposed more
than a year ago, but never pushed upstream.

Not sure if we really should go this way. dma_fence objects are shared
between drivers and since we can't judge if it's the correct fence based on
a criteria in the object (only the read counter which is outside) all
drivers need to be correct for this.

I would rather go the way and change dma_fence_release() to wrap
fence->ops->release into call_rcu() to keep the whole RCU handling outside
of the individual drivers.

Hm, I entirely dropped the ball on this, I kinda assumed that we managed
to get some agreement on this between i915 and dma_fence. Adding a pile
more people.


For the meantime I've send a v2 of this patch to fix at least the buggy 
return of NULL when we fail to grab the RCU reference but keeping the 
extra checking for now.


Can I get an rb on this please so that we fix at least the bug at hand?

Thanks,
Christian.



Joonas, Tvrtko, I guess we need to fix this one way or the other.
-Daniel

Re: [PATCH 2/3] [media] tc358743: Increase FIFO level to 300.

[Sakari Ailus]

Hi Hans,

On Wed, Sep 20, 2017 at 03:12:03PM +0200, Hans Verkuil wrote:
> On 09/20/17 14:50, Sakari Ailus wrote:
> > Hi Hans and others,
> > 
> > On Wed, Sep 20, 2017 at 01:24:02PM +0200, Hans Verkuil wrote:
> >> On 09/20/17 13:00, Dave Stevenson wrote:
> >>> On 20 September 2017 at 11:23, Philipp Zabel  
> >>> wrote:
>  Hi,
> 
>  On Wed, 2017-09-20 at 10:14 +0100, Dave Stevenson wrote:
> > Hi Mauro & Philipp
> >
> > On 19 September 2017 at 17:49, Mauro Carvalho Chehab
> >  wrote:
> >> Em Tue, 19 Sep 2017 17:24:45 +0200
> >> Philipp Zabel  escreveu:
> >>
> >>> Hi Dave,
> >>>
> >>> On Tue, 2017-09-19 at 14:08 +0100, Dave Stevenson wrote:
>  The existing fixed value of 16 worked for UYVY 720P60 over
>  2 lanes at 594MHz, or UYVY 1080P60 over 4 lanes. (RGB888
>  1080P60 needs 6 lanes at 594MHz).
>  It doesn't allow for lower resolutions to work as the FIFO
>  underflows.
> 
>  Using a value of 300 works for all resolutions down to VGA60,
>  and the increase in frame delay is <4usecs for 1080P60 UYVY
>  (2.55usecs for RGB888).
> 
>  Signed-off-by: Dave Stevenson 
> >>>
> >>> Can we increase this to 320? This would also allow
> >>> 720p60 at 594 Mbps / 4 lanes, according to the xls.
> >
> > Unless I've missed something then the driver would currently request
> > only 2 lanes for 720p60 UYVY, and that works with the existing FIFO
> > setting of 16. Likewise 720p60 RGB888 requests 3 lanes and also works
> > on a FIFO setting of 16.
> > How/why were you thinking we need to run all four lanes for 720p60
> > without other significant driver mods around lane config?
> 
>  The driver currently silently changes the number of active lanes
>  depending on required data rate, with no way to communicate it to the
>  receiver.
> >>>
> >>> It is communicated over the subdevice API - tc358743_g_mbus_config
> >>> reports back the appropriate number of lanes to the receiver
> >>> subdevice.
> >>> A suitable v4l2_subdev_has_op(dev->sensor, video, g_mbus_config) call
> >>> as you're starting streaming therefore gives you the correct
> >>> information. That's what I've just done for the BCM283x Unicam
> >>> driver[1], but admittedly I'm not using the media controller API which
> >>> i.MX6 is.
> >>
> >> Shouldn't this information come from the device tree? The g_mbus_config
> >> op is close to being deprecated or even removed. There are currently only
> >> two obscure V4L2 bridge drivers that call it. It dates from pre-DT times
> >> I rather not see it used in a new bridge driver.
> >>
> >> The problem is that contains data that belongs to the DT (hardware
> >> capabilities). Things that can actually change dynamically should be
> >> communicated via another op. We don't have that, so that should be created.
> > 
> > The DT tells how many lanes are connected in hardware, but up to now that's
> > also been the number of lanes actually used.
> > 
> > The g_mbus_config() is there, and I'd like to replace that with the more
> > generic frame descriptors, with CSI-2 virtual channel and data type
> > support. They're however not quite ready yet nor I've recently worked on
> > them.
> > 
> > I think using g_mbus_config() for the purpose right now is entirely
> > acceptable, we can rework that later on when adding support for frame
> > descriptors.
> > 
> 
> I don't like it :-)
> 
> Currently g_mbus_config returns (and I quote from v4l2-mediabus.h): "How
> many lanes the client can use". I.e. the capabilities of the HW.
> 
> If we are going to use this to communicate how many lines are currently
> in use, then I would propose that we add a lane mask, i.e. something like
> this:
> 
> /* Number of lanes in use, 0 == use all available lanes (default) */
> #define V4L2_MBUS_CSI2_LANE_MASK(3 << 10)
> 
> And add comments along the lines that this is a temporary fix.
> 
> I would feel a lot happier (or a lot less unhappy) if we'd do it this way.
> Rather than re-interpreting bits that are not quite what they should be.
> 
> I'd also add a comment that all other flags must be 0 if the device tree is
> used. This to avoid mixing the two.

That would work for me as well.

There are very few users of the g_mbus_config API and I bet the current
users could get the configuration from DT as well: they're platform drivers
used on ARM. With the possible exception of SoC camera drives.

-- 
Regards,

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