Re: [PATCH 1/1] smiapp: Implement power-on and power-off sequences without runtime PM

[Sakari Ailus]

Hi Alan and others,

On Thu, Nov 24, 2016 at 09:15:39PM -0500, Alan Stern wrote:
> On Fri, 25 Nov 2016, Laurent Pinchart wrote:
> 
> > Dear linux-pm developers, what's the suggested way to ensure that a runtime-
> > pm-enabled driver can run fine on a system with CONFIG_PM disabled ?
> 
> The exact point of your question isn't entirely clear.  In the most 
> literal sense, the best ways to ensure this are (1) audit the code, and 
> (2) actually try it.
> 
> I have a feeling this doesn't quite answer your question, however.  :-)

The question is related to devices that require certain power-up and
power-down sequences that are now implemented as PM runtime hooks that,
without CONFIG_PM defined, will not be executed. Is there a better way than
to handle this than have an implementation in the driver for the PM runtime
and non-PM runtime case separately?

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] media: Drop FSF's postal address from the source code files

[Sakari Ailus]

Hi Laurent,

On 11/25/16 04:03, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Friday 28 Oct 2016 15:43:53 Sakari Ailus wrote:
>> Drop the FSF's postal address from the source code files that typically
>> contain mostly the license text. The patch has been created with the
>> following command without manual edits:
>>
>> git grep -l "675 Mass Ave\|59 Temple Place\|51 Franklin St" -- \
>>  drivers/media/ include/media|while read i; do i=$i perl -e '
>> open(F,"< $ENV{i}");
>> $a=join("", );
>> $a =~ s/[ \t]*\*\n.*You should.*\n.*along with.*\n.*(\n.*USA.*$)?\n//m
>>  && $a =~ s/(^.*)Or, (point your browser to) /$1To obtain the license,
>> $2\n$1/m; close(F);
> 
> Do we really need to keep the "To obtain the license, point your browser 
> to..." text ?

In order not to change the existing text unnecessarily, I did that in
the patch. I'm certainly ok with dropping it as it does not seemingly
really has no value.

-- 
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Media: platform: vsp1: - Do not forget to call

[Shailendra Verma]

v4l2_fh_init is already done.So call the v4l2_fh_exit in error condition
before returing from the function.

Signed-off-by: Shailendra Verma 
---
 drivers/media/platform/vsp1/vsp1_video.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/vsp1/vsp1_video.c 
b/drivers/media/platform/vsp1/vsp1_video.c
index d351b9c..cc58163 100644
--- a/drivers/media/platform/vsp1/vsp1_video.c
+++ b/drivers/media/platform/vsp1/vsp1_video.c
@@ -1044,6 +1044,7 @@ static int vsp1_video_open(struct file *file)
ret = vsp1_device_get(video->vsp1);
if (ret < 0) {
v4l2_fh_del(vfh);
+   v4l2_fh_exit(vfh);
kfree(vfh);
}
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

cron job: media_tree daily build: ERRORS

[Hans Verkuil]

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

Results of the daily build of media_tree:

date:   Fri Nov 25 05:00:20 CET 2016
media-tree git hash:4cc5bed1caeb6d40f2f41c4c5eb83368691fbffb
media_build git hash:   538c0b2692a7ca59253385180954ac6c7d9b7817
v4l-utils git hash: d7f2c410fc35841fe817c4bb9cf94070994e0f01
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.8.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: 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: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
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: OK
linux-3.12.67-i686: OK
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-rc5-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: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
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: OK
linux-3.12.67-x86_64: OK
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: OK
linux-4.9-rc5-x86_64: OK
apps: WARNINGS
spec-git: OK
smatch: ERRORS
sparse: WARNINGS

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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Staging: media: davinci_vpfe: - Fix for memory leak if decoder initialization fails.

[Shailendra Verma]

Fix to avoid possible memory leak if the decoder initialization
got failed.Free the allocated memory for file handle object
before return in case decoder initialization fails.

Signed-off-by: Shailendra Verma 
---
 drivers/staging/media/davinci_vpfe/vpfe_video.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c 
b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 8be9f85..4215445 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -423,6 +423,9 @@ static int vpfe_open(struct file *file)
/* If decoder is not initialized. initialize it */
if (!video->initialized && vpfe_update_pipe_state(video)) {
mutex_unlock(>lock);
+   v4l2_fh_del(>vfh);
+   v4l2_fh_exit(>vfh);
+   kfree(handle);
return -ENODEV;
}
/* Increment device users counter */
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Media: Platform: Exynos-gsc: - Fix for error handling

[Shailendra Verma]

The File handle is not yet added in the vfd list.So no need to call
v4l2_fh_del(>fh) if it fails to create control.

Signed-off-by: Shailendra Verma 
---
 drivers/media/platform/exynos-gsc/gsc-m2m.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c 
b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index 9f03b79..5ea97c1 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -664,8 +664,8 @@ static int gsc_m2m_open(struct file *file)
 
 error_ctrls:
gsc_ctrls_delete(ctx);
-error_fh:
v4l2_fh_del(>fh);
+error_fh:
v4l2_fh_exit(>fh);
kfree(ctx);
 unlock:
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Staging: media: platform: davinci: - Fix for memory leak

[Shailendra Verma]

Fix to avoid possible memory leak if the decoder initialization got failed.
Free the allocated memory for file handle object before return in case
decoder initialization fails.

Signed-off-by: Shailendra Verma 
---
 drivers/media/platform/davinci/vpfe_capture.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c 
b/drivers/media/platform/davinci/vpfe_capture.c
index 6efb2f1..188b333 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -526,6 +526,8 @@ static int vpfe_open(struct file *file)
if (!vpfe_dev->initialized) {
if (vpfe_initialize_device(vpfe_dev)) {
mutex_unlock(_dev->lock);
+   v4l2_fh_exit(>fh);
+   kfree(fh);
return -ENODEV;
}
}
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Staging: Media: Omap4iss: Do not forget to call

[Shailendra Verma]

v4l2_fh_init is already done.So call the v4l2_fh_exit in error condition
before returing from the function.

Signed-off-by: Shailendra Verma 
---
 drivers/staging/media/omap4iss/iss_video.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/omap4iss/iss_video.c 
b/drivers/staging/media/omap4iss/iss_video.c
index c16927a..c46d14d 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -1141,6 +1141,7 @@ static int iss_video_open(struct file *file)
 done:
if (ret < 0) {
v4l2_fh_del(>vfh);
+   v4l2_fh_exit(>vfh);
kfree(handle);
}
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Media: Platform: Sti: Bdisp: - Fix for error handling

[Shailendra Verma]

The File handle is not yet added in the vdev list.So no need to call 
v4l2_fh_del(>fh)if it fails to create control.

Signed-off-by: Shailendra Verma 
---
 drivers/media/platform/sti/bdisp/bdisp-v4l2.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c 
b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
index 45f82b5..fbf302f 100644
--- a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
+++ b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
@@ -632,8 +632,8 @@ static int bdisp_open(struct file *file)
 
 error_ctrls:
bdisp_ctrls_delete(ctx);
-error_fh:
v4l2_fh_del(>fh);
+error_fh:
v4l2_fh_exit(>fh);
bdisp_hw_free_nodes(ctx);
 mem_ctx:
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Media: Platform: Omap3isp: Do not forget to call

[Shailendra Verma]

v4l2_fh_init is already done.So call the v4l2_fh_exit in error condition
before returing from the function.

Signed-off-by: Shailendra Verma 
---
 drivers/media/platform/omap3isp/ispvideo.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/omap3isp/ispvideo.c 
b/drivers/media/platform/omap3isp/ispvideo.c
index 7354469..2822e2f 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -1350,6 +1350,7 @@ static int isp_video_open(struct file *file)
 done:
if (ret < 0) {
v4l2_fh_del(>vfh);
+   v4l2_fh_exit(>vfh);
kfree(handle);
}
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] Media: Platform: Exynos4-is: - Fix for error handling

[Shailendra Verma]

The File handle is not yet added in the vfd list.So no need to call 
v4l2_fh_del(>fh) if it fails to create control.

Signed-off-by: Shailendra Verma 
---
 drivers/media/platform/exynos4-is/fimc-m2m.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c 
b/drivers/media/platform/exynos4-is/fimc-m2m.c
index 6028e4f..d8724fe 100644
--- a/drivers/media/platform/exynos4-is/fimc-m2m.c
+++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
@@ -663,8 +663,8 @@ static int fimc_m2m_open(struct file *file)
v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
 error_c:
fimc_ctrls_delete(ctx);
-error_fh:
v4l2_fh_del(>fh);
+error_fh:
v4l2_fh_exit(>fh);
kfree(ctx);
 unlock:
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] smiapp: Implement power-on and power-off sequences without runtime PM

[Alan Stern]

On Fri, 25 Nov 2016, Laurent Pinchart wrote:

> Dear linux-pm developers, what's the suggested way to ensure that a runtime-
> pm-enabled driver can run fine on a system with CONFIG_PM disabled ?

The exact point of your question isn't entirely clear.  In the most 
literal sense, the best ways to ensure this are (1) audit the code, and 
(2) actually try it.

I have a feeling this doesn't quite answer your question, however.  :-)

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] media: Drop FSF's postal address from the source code files

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Friday 28 Oct 2016 15:43:53 Sakari Ailus wrote:
> Drop the FSF's postal address from the source code files that typically
> contain mostly the license text. The patch has been created with the
> following command without manual edits:
> 
> git grep -l "675 Mass Ave\|59 Temple Place\|51 Franklin St" -- \
>   drivers/media/ include/media|while read i; do i=$i perl -e '
> open(F,"< $ENV{i}");
> $a=join("", );
> $a =~ s/[ \t]*\*\n.*You should.*\n.*along with.*\n.*(\n.*USA.*$)?\n//m
>   && $a =~ s/(^.*)Or, (point your browser to) /$1To obtain the license,
> $2\n$1/m; close(F);

Do we really need to keep the "To obtain the license, point your browser 
to..." text ?

In any case,

Acked-by: Laurent Pinchart 

with a quick review of the loong patch directly from your tree.

> open(F, "> $ENV{i}");
> print F $a;
> close(F);'; done
> 
> Signed-off-by: Sakari Ailus 
> ---
> Hi,
> 
> I found this in a few places and decided to remove them all. The script
> could be useful elsewhere, too.
> 
> The actual patch can be found here, it appears to be too large to be
> accepted by vger:
> 
>  s/fsf-address>

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] smiapp: Implement power-on and power-off sequences without runtime PM

[Laurent Pinchart]

Hello,

(CC'ing the linux-pm mailing list)

On Tuesday 22 Nov 2016 21:58:32 Arnd Bergmann wrote:
> On Tuesday, November 22, 2016 8:31:42 PM CET Laurent Pinchart wrote:
> >>> @@ -2915,7 +2906,11 @@ static int smiapp_probe(struct i2c_client
> >>> *client,
> >>> 
> >>> pm_runtime_enable(>dev);
> >>> 
> >>> +#ifdef CONFIG_PM
> >>> rval = pm_runtime_get_sync(>dev);
> >>> +#else
> >>> +   rval = smiapp_power_on(>dev);
> >>> +#endif
> >>> if (rval < 0) {
> >>> rval = -ENODEV;
> >>> goto out_power_off;
> >> 
> >> I would suggest writing this as
> >> 
> >>   if (IS_ENABLED(CONFIG_PM))
> >>   rval = pm_runtime_get_sync(>dev);
> >>   else
> >>   rval = smiapp_power_on(>dev);
> >> 
> >> though that is a purely cosmetic change.
> > 
> > Are all drivers really supposed to code this kind of construct ? Shouldn't
> > this be handled in the PM core ? A very naive approach would be to call
> > .runtime_resume() and .runtime_suspend() from the non-CONFIG_PM versions
> > of pm_runtime_enable() and pm_runtime_disable() respectively. I assume
> > that would break things, but can't we implement something similar to that
> > that wouldn't require all drivers to open-code it ?
> 
> I know nothing about the details of how the suspend/resume code should
> do this, I was just commenting on the syntax above, preferring an
> IS_ENABLED() check over an #ifdef.

Dear linux-pm developers, what's the suggested way to ensure that a runtime-
pm-enabled driver can run fine on a system with CONFIG_PM disabled ?

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] v4l: vsp1: Prevent commencing pipelines before they are setup

[Laurent Pinchart]

Hi Kieran,

On Wednesday 23 Nov 2016 12:28:15 Kieran Bingham wrote:
> Just FYI,
> 
> Whilst this patch is functional on its own, it is likely to be
> superseded before it gets a chance to be integrated as I am currently
> reworking vsp1_video_start_streaming(), in particular the use of
> vsp1_video_setup_pipeline().
> 
> The re-work will of course also consider and tackle the issue repaired here.

OK, I'll ignore this patch then.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 7/7] Add a libv4l plugin for Exynos4 camera

[Sakari Ailus]

Hi Jacek,

On Thu, Nov 24, 2016 at 05:14:40PM +0100, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> On 11/24/2016 04:11 PM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >Thank you for your continued work on the Exynos plugin patchset!
> >
> >I think we're pretty close to being able to merge the set, if you could
> >still bear with me awhile. :-)
> 
> Your demanding reviewer I must admit. :-)
> Of course, I appreciate all your remarks, they're highly valuable.
> 
> >On Wed, Oct 12, 2016 at 04:35:22PM +0200, Jacek Anaszewski wrote:
> >...
> >>diff --git a/lib/libv4l-exynos4-camera/Makefile.am 
> >>b/lib/libv4l-exynos4-camera/Makefile.am
> >>new file mode 100644
> >>index 000..c38b7f6
> >>--- /dev/null
> >>+++ b/lib/libv4l-exynos4-camera/Makefile.am
> >>@@ -0,0 +1,19 @@
> >>+if WITH_V4L_PLUGINS
> >>+libv4l2plugin_LTLIBRARIES = libv4l-exynos4-camera.la
> >>+endif
> >>+
> >>+media-bus-format-names.h: ../../include/linux/media-bus-format.h
> >>+   sed -e '/#define MEDIA_BUS_FMT/ ! d; s/.*FMT_//; /FIXED/ d; s/\t.*//; 
> >>s/.*/{ \"&\", MEDIA_BUS_FMT_& },/;' \
> >>+   < $< > $@
> >>+
> >>+media-bus-format-codes.h: ../../include/linux/media-bus-format.h
> >>+   sed -e '/#define MEDIA_BUS_FMT/ ! d; s/.*#define //; /FIXED/ d; 
> >>s/\t.*//; s/.*/ &,/;' \
> >>+   < $< > $@
> >>+
> >>+BUILT_SOURCES = media-bus-format-names.h media-bus-format-codes.h
> >>+CLEANFILES = $(BUILT_SOURCES)
> >
> >It'd be nice to be able to use the same generated headers that now are under
> >utils/media-ctl, instead of copying the sed script here in verbatim. If the
> >script is changed or fixed in some way, the other location probably will
> >remain unchanged...
> 
> The problem is that those headers are built after this plugin.

They could be moved to another directory from where they are now, and
explicitly built as a dependency. I don't know how to properly do that with
automake though.

I cc'd Gregor to the thread.

> 
> >I wonder if there's a proper way to generate build time headers such as
> >these.
> >
> >Another less good alternative would be to put these into a separate Makefile
> >and include that Makefile where the headers are needed. But I don't like
> >that much either, it's a hack.
> 
> In this case it seems to be the only feasible optimization.

I'm ok with that but still hope we could have something better. :-)

> 
> >>+
> >>+nodist_libv4l_exynos4_camera_la_SOURCES = $(BUILT_SOURCES)
> >>+libv4l_exynos4_camera_la_SOURCES = libv4l-exynos4-camera.c 
> >>../../utils/media-ctl/libmediactl.c ../../utils/media-ctl/libv4l2subdev.c 
> >>../../utils/media-ctl/mediatext.c
> >>+libv4l_exynos4_camera_la_CFLAGS = -fvisibility=hidden -std=gnu99
> >>+libv4l_exynos4_camera_la_LDFLAGS = -avoid-version -module -shared 
> >>-export-dynamic -lpthread
> >>diff --git a/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c 
> >>b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
> >>new file mode 100644
> >>index 000..c219fe5
> >>--- /dev/null
> >>+++ b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
> >>@@ -0,0 +1,1325 @@
> >>+/*
> >>+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
> >>+ *  http://www.samsung.com
> >>+ *
> >>+ * Author: Jacek Anaszewski 
> >>+ *
> >>+ * This program is free software; you can redistribute it and/or modify
> >>+ * it under the terms of the GNU Lesser General Public License as 
> >>published by
> >>+ * the Free Software Foundation; either version 2.1 of the License, or
> >>+ * (at your option) any later version.
> >>+ *
> >>+ * This program is distributed in the hope that it will be useful,
> >>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >>+ * Lesser General Public License for more details.
> >>+ */
> >>+
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+#include 
> >>+
> >>+#include "../../utils/media-ctl/mediactl.h"
> >>+#include "../../utils/media-ctl/mediatext.h"
> >>+#include "../../utils/media-ctl/v4l2subdev.h"
> >>+#include "libv4l-plugin.h"
> >>+
> >>+#ifdef DEBUG
> >>+#define V4L2_EXYNOS4_DBG(format, ARG...)\
> >>+   printf("[%s:%d] [%s] " format " \n", __FILE__, __LINE__, __func__, 
> >>##ARG)
> >>+#else
> >>+#define V4L2_EXYNOS4_DBG(format, ARG...)
> >>+#endif
> >>+
> >>+#define V4L2_EXYNOS4_ERR(format, ARG...)\
> >>+   fprintf(stderr, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
> >>+
> >>+#define V4L2_EXYNOS4_LOG(format, ARG...)\
> >>+   fprintf(stdout, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
> >>+
> >>+#define VIDIOC_CTRL(type)  \
> >>+   ((type) == VIDIOC_S_CTRL ? "VIDIOC_S_CTRL" :\
> >>+  "VIDIOC_G_CTRL")
> >>+
> >>+#if HAVE_VISIBILITY
> >>+#define PLUGIN_PUBLIC __attribute__ ((visibility("default")))
> >>+#else
> >>+#define PLUGIN_PUBLIC
> >>+#endif
> >>+
> >>+#define SYS_IOCTL(fd, cmd, 

Re: Enabling peer to peer device transactions for PCIe devices

[Logan Gunthorpe]

On 24/11/16 09:42 AM, Jason Gunthorpe wrote:
> There are three cases to worry about:
>  - Coherent long lived page table mirroring (RDMA ODP MR)
>  - Non-coherent long lived page table mirroring (RDMA MR)
>  - Short lived DMA mapping (everything else)
> 
> Like you say below we have to handle short lived in the usual way, and
> that covers basically every device except IB MRs, including the
> command queue on a NVMe drive.

Yes, this makes sense to me. Though I thought regular IB MRs with
regular memory currently pinned the pages (despite being long lived)
that's why we can run up against the "max locked memory" limit. It
doesn't seem so terrible if GPU memory had a similar restriction until
ODP like solutions get implemented.

>> Yeah, we've had RDMA and O_DIRECT transfers to PCIe backed ZONE_DEVICE
>> memory working for some time. I'd say it's a good fit. The main question
>> we've had is how to expose PCIe bars to userspace to be used as MRs and
>> such.

> Is there any progress on that?

Well, I guess there's some consensus building to do. The existing
options are:

* Device DAX: which could work but the problem I see with it is that it
only allows one application to do these transfers. Or there would have
to be some user-space coordination to figure which application gets what
memeroy.

* Regular DAX in the FS doesn't work at this time because the FS can
move the file you think your transfer to out from under you. Though I
understand there's been some work with XFS to solve that issue.

Though, we've been considering that the backed memory would be
non-volatile which adds some of this complexity. If the memory were
volatile the kernel would just need to do some relatively straight
forward allocation to user-space when asked. For example, with NVMe, the
kernel could give chunks of the CMB buffer to userspace via an mmap call
to /dev/nvmeX. Though I think there's been some push back against things
like that as well.

> I still don't quite get what iopmem was about.. I thought the
> objection to uncachable ZONE_DEVICE & DAX made sense, so running DAX
> over iopmem and still ending up with uncacheable mmaps still seems
> like a non-starter to me...

The latest incarnation of iopmem simply created a block device backed by
ZONE_DEVICE memory on a PCIe BAR. We then put a DAX FS on it and
user-space could mmap the files and send them to other devices to do P2P
transfers.

I don't think there was a hard objection to uncachable ZONE_DEVICE and
DAX. We did try our experimental hardware with cached ZONE_DEVICE and it
did work but the performance was beyond unusable (which may be a
hardware issue). In the end I feel the driver would have to decide the
most appropriate caching for the hardware and I don't understand why WC
or UC wouldn't work with ZONE_DEVICE.

Logan
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Enabling peer to peer device transactions for PCIe devices

[Logan Gunthorpe]

Hey,

On 24/11/16 02:45 AM, Christian König wrote:
> E.g. it can happen that PCI device A exports it's BAR using ZONE_DEVICE.
> Not PCI device B (a SATA device) can directly read/write to it because
> it is on the same bus segment, but PCI device C (a network card for
> example) can't because it is on a different bus segment and the bridge
> can't handle P2P transactions.

Yeah, that could be an issue but in our experience we have yet to see
it. We've tested with two separate PCI buses on different CPUs connected
through QPI links and it works fine. (It is rather slow but I understand
Intel has improved the bottleneck in newer CPUs than the ones we tested.)

It may just be older hardware that has this issue. I expect that as long
as a failed transfer can be handled gracefully by the initiator I don't
see a need to predetermine whether a device can see another devices memory.


Logan
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Problem with media_build install

[Jose Alberto Reguero]

#make install
make -C /usr/src/media_build/v4l install
make[1]: Entering directory '/usr/src/media_build/v4l'
make[1]: *** No rule to make target 'mm-install' needed by 'install'. Alto.
make[1]: Leaving directory '/usr/src/media_build/v4l'
Makefile:15: recipe for target 'install' failed
make: *** [install] Error 2

Jose Alberto

Re: Enabling peer to peer device transactions for PCIe devices

[Serguei Sagalovitch]

On 2016-11-24 11:26 AM, Jason Gunthorpe wrote:

On Thu, Nov 24, 2016 at 10:45:18AM +0100, Christian König wrote:

Am 24.11.2016 um 00:25 schrieb Jason Gunthorpe:

There is certainly nothing about the hardware that cares
about ZONE_DEVICE vs System memory.

Well that is clearly not so simple. When your ZONE_DEVICE pages describe a
PCI BAR and another PCI device initiates a DMA to this address the DMA
subsystem must be able to check if the interconnection really works.

I said the hardware doesn't care.. You are right, we still have an
outstanding problem in Linux of how to generically DMA map a P2P
address - which is a different issue from getting the P2P address from
a __user pointer...

Jason
I agreed but the problem is that one issue immediately introduce another 
one

to solve and so on (if we do not want to cut corners). I would think  that
a lot of them interconnected because the way how one problem could be
solved may impact solution for another.

btw: about "DMA map a p2p address": Right now to enable  p2p between 
devices
it is required/recommended to disable iommu support  (e.g. intel iommu 
driver

has special logic for graphics and  comment "Reserve all PCI MMIO to avoid
peer-to-peer access").
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Enabling peer to peer device transactions for PCIe devices

[Jason Gunthorpe]

On Wed, Nov 23, 2016 at 06:25:21PM -0700, Logan Gunthorpe wrote:
> 
> 
> On 23/11/16 02:55 PM, Jason Gunthorpe wrote:
> >>> Only ODP hardware allows changing the DMA address on the fly, and it
> >>> works at the page table level. We do not need special handling for
> >>> RDMA.
> >>
> >> I am aware of ODP but, noted by others, it doesn't provide a general
> >> solution to the points above.
> > 
> > How do you mean?
> 
> I was only saying it wasn't general in that it wouldn't work for IB
> hardware that doesn't support ODP or other hardware  that doesn't do
> similar things (like an NVMe drive).

There are three cases to worry about:
 - Coherent long lived page table mirroring (RDMA ODP MR)
 - Non-coherent long lived page table mirroring (RDMA MR)
 - Short lived DMA mapping (everything else)

Like you say below we have to handle short lived in the usual way, and
that covers basically every device except IB MRs, including the
command queue on a NVMe drive.

> any complex allocators (GPU or otherwise) should respect that. And that
> seems like it should be the default way most of this works -- and I
> think it wouldn't actually take too much effort to make it all work now
> as is. (Our iopmem work is actually quite small and simple.)

Yes, absolutely, some kind of page pinning like locking is a hard
requirement.

> Yeah, we've had RDMA and O_DIRECT transfers to PCIe backed ZONE_DEVICE
> memory working for some time. I'd say it's a good fit. The main question
> we've had is how to expose PCIe bars to userspace to be used as MRs and
> such.

Is there any progress on that?

I still don't quite get what iopmem was about.. I thought the
objection to uncachable ZONE_DEVICE & DAX made sense, so running DAX
over iopmem and still ending up with uncacheable mmaps still seems
like a non-starter to me...

Serguei, what is your plan in GPU land for migration? Ie if I have a
CPU mapped page and the GPU moves it to VRAM, it becomes non-cachable
- do you still allow the CPU to access it? Or do you swap it back to
cachable memory if the CPU touches it?

One approach might be to mmap the uncachable ZONE_DEVICE memory and
mark it inaccessible to the CPU - DMA could still translate. If the
CPU needs it then the kernel migrates it to system memory so it
becomes cachable. ??

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Enabling peer to peer device transactions for PCIe devices

[Jason Gunthorpe]

On Thu, Nov 24, 2016 at 10:45:18AM +0100, Christian König wrote:
> Am 24.11.2016 um 00:25 schrieb Jason Gunthorpe:
> >There is certainly nothing about the hardware that cares
> >about ZONE_DEVICE vs System memory.
> Well that is clearly not so simple. When your ZONE_DEVICE pages describe a
> PCI BAR and another PCI device initiates a DMA to this address the DMA
> subsystem must be able to check if the interconnection really works.

I said the hardware doesn't care.. You are right, we still have an
outstanding problem in Linux of how to generically DMA map a P2P
address - which is a different issue from getting the P2P address from
a __user pointer...

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] [media] b2c2: use IS_REACHABLE() instead of open-coding it

[Arnd Bergmann]

The FE_SUPPORTED() macro is basically the same as IS_REACHABLE, except
that it causes a warning with gcc-7:

common/b2c2/flexcop-fe-tuner.c:30:1: error: this use of "defined" may not be 
portable [-Werror=expansion-to-defined]
common/b2c2/flexcop-fe-tuner.c:30:1: error: this use of "defined" may not be 
portable [-Werror=expansion-to-defined]
common/b2c2/flexcop-fe-tuner.c:30:1: error: this use of "defined" may not be 
portable [-Werror=expansion-to-defined]

Using IS_REACHABLE() to define it avoids the warning.

Fixes: 3785bc170f79 ("[media] b2c2: break it into common/pci/usb directories")
Signed-off-by: Arnd Bergmann 
---
 drivers/media/common/b2c2/flexcop-fe-tuner.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/common/b2c2/flexcop-fe-tuner.c 
b/drivers/media/common/b2c2/flexcop-fe-tuner.c
index f5956402fc69..5f10151ecec9 100644
--- a/drivers/media/common/b2c2/flexcop-fe-tuner.c
+++ b/drivers/media/common/b2c2/flexcop-fe-tuner.c
@@ -24,8 +24,7 @@
 
 /* Can we use the specified front-end?  Remember that if we are compiled
  * into the kernel we can't call code that's in modules.  */
-#define FE_SUPPORTED(fe) (defined(CONFIG_DVB_##fe) || \
-   (defined(CONFIG_DVB_##fe##_MODULE) && defined(MODULE)))
+#define FE_SUPPORTED(fe) IS_REACHABLE(CONFIG_DVB_ ## fe)
 
 #if FE_SUPPORTED(BCM3510) || (FE_SUPPORTED(CX24120) && FE_SUPPORTED(ISL6421))
 static int flexcop_fe_request_firmware(struct dvb_frontend *fe,
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Enabling peer to peer device transactions for PCIe devices

[Jason Gunthorpe]

On Thu, Nov 24, 2016 at 12:40:37AM +, Sagalovitch, Serguei wrote:
> On Wed, Nov 23, 2016 at 02:11:29PM -0700, Logan Gunthorpe wrote:
> 
> > Perhaps I am not following what Serguei is asking for, but I
> > understood the desire was for a complex GPU allocator that could
> > migrate pages between GPU and CPU memory under control of the GPU
> > driver, among other things. The desire is for DMA to continue to work
> > even after these migrations happen.
> 
> The main issue is to  how to solve use cases when p2p is 
> requested/initiated via CPU pointers where such pointers could 
> point to non-system memory location e.g.  VRAM.  

Okay, but your list is conflating a whole bunch of problems..

 1) How to go from a __user pointer to a p2p DMA address
  a) How to validate, setup iommu and maybe worst case bounce buffer
 these p2p DMAs
 2) How to allow drivers (ie GPU allocator) dynamically
remap pages in a VMA to/from p2p DMA addresses
 3) How to expose uncachable p2p DMA address to user space via mmap

> to allow "get_user_pages"  to work transparently similar 
> how it is/was done for "DAX Device" case. Unfortunately 
> based on my understanding "DAX Device" implementation 
> deal only with permanently  "locked" memory  (fixed location) 
> unrelated to "get_user_pages"/"put_page" scope  
> which doesn't satisfy requirements  for "eviction" / "moving" of 
> memory keeping CPU address intact.  

Hurm, isn't that issue with DAX only to do with being coherent with
the page cache?

A GPU allocator would not use the page cache, it would have to
construct VMAs some other way.

> My understanding is that It will not solve RDMA MR issue where "lock" 
> could be during the whole  application life but  (a) it will not make 
> RDMA MR case worse  (b) should be enough for all other cases for 
> "get_user_pages"/"put_page" controlled by  kernel.

Right. There is no solution to the RDMA MR issue on old hardware. Apps
that are using GPU+RDMA+Old hardware will have to use short lived MRs
and pay that performance cost, or give up on migration.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 7/7] Add a libv4l plugin for Exynos4 camera

[Jacek Anaszewski]

Hi Sakari,

On 11/24/2016 04:11 PM, Sakari Ailus wrote:

Hi Jacek,

Thank you for your continued work on the Exynos plugin patchset!

I think we're pretty close to being able to merge the set, if you could
still bear with me awhile. :-)


Your demanding reviewer I must admit. :-)
Of course, I appreciate all your remarks, they're highly valuable.


On Wed, Oct 12, 2016 at 04:35:22PM +0200, Jacek Anaszewski wrote:
...

diff --git a/lib/libv4l-exynos4-camera/Makefile.am 
b/lib/libv4l-exynos4-camera/Makefile.am
new file mode 100644
index 000..c38b7f6
--- /dev/null
+++ b/lib/libv4l-exynos4-camera/Makefile.am
@@ -0,0 +1,19 @@
+if WITH_V4L_PLUGINS
+libv4l2plugin_LTLIBRARIES = libv4l-exynos4-camera.la
+endif
+
+media-bus-format-names.h: ../../include/linux/media-bus-format.h
+   sed -e '/#define MEDIA_BUS_FMT/ ! d; s/.*FMT_//; /FIXED/ d; s/\t.*//; s/.*/{ 
\"&\", MEDIA_BUS_FMT_& },/;' \
+   < $< > $@
+
+media-bus-format-codes.h: ../../include/linux/media-bus-format.h
+   sed -e '/#define MEDIA_BUS_FMT/ ! d; s/.*#define //; /FIXED/ d; s/\t.*//; 
s/.*/ &,/;' \
+   < $< > $@
+
+BUILT_SOURCES = media-bus-format-names.h media-bus-format-codes.h
+CLEANFILES = $(BUILT_SOURCES)


It'd be nice to be able to use the same generated headers that now are under
utils/media-ctl, instead of copying the sed script here in verbatim. If the
script is changed or fixed in some way, the other location probably will
remain unchanged...


The problem is that those headers are built after this plugin.


I wonder if there's a proper way to generate build time headers such as
these.

Another less good alternative would be to put these into a separate Makefile
and include that Makefile where the headers are needed. But I don't like
that much either, it's a hack.


In this case it seems to be the only feasible optimization.


+
+nodist_libv4l_exynos4_camera_la_SOURCES = $(BUILT_SOURCES)
+libv4l_exynos4_camera_la_SOURCES = libv4l-exynos4-camera.c 
../../utils/media-ctl/libmediactl.c ../../utils/media-ctl/libv4l2subdev.c 
../../utils/media-ctl/mediatext.c
+libv4l_exynos4_camera_la_CFLAGS = -fvisibility=hidden -std=gnu99
+libv4l_exynos4_camera_la_LDFLAGS = -avoid-version -module -shared 
-export-dynamic -lpthread
diff --git a/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c 
b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
new file mode 100644
index 000..c219fe5
--- /dev/null
+++ b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
@@ -0,0 +1,1325 @@
+/*
+ * Copyright (c) 2016 Samsung Electronics Co., Ltd.
+ *  http://www.samsung.com
+ *
+ * Author: Jacek Anaszewski 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "../../utils/media-ctl/mediactl.h"
+#include "../../utils/media-ctl/mediatext.h"
+#include "../../utils/media-ctl/v4l2subdev.h"
+#include "libv4l-plugin.h"
+
+#ifdef DEBUG
+#define V4L2_EXYNOS4_DBG(format, ARG...)\
+   printf("[%s:%d] [%s] " format " \n", __FILE__, __LINE__, __func__, 
##ARG)
+#else
+#define V4L2_EXYNOS4_DBG(format, ARG...)
+#endif
+
+#define V4L2_EXYNOS4_ERR(format, ARG...)\
+   fprintf(stderr, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
+
+#define V4L2_EXYNOS4_LOG(format, ARG...)\
+   fprintf(stdout, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
+
+#define VIDIOC_CTRL(type)  \
+   ((type) == VIDIOC_S_CTRL ? "VIDIOC_S_CTRL" :  \
+  "VIDIOC_G_CTRL")
+
+#if HAVE_VISIBILITY
+#define PLUGIN_PUBLIC __attribute__ ((visibility("default")))
+#else
+#define PLUGIN_PUBLIC
+#endif
+
+#define SYS_IOCTL(fd, cmd, arg) \
+   syscall(SYS_ioctl, (int)(fd), (unsigned long)(cmd), (void *)(arg))
+
+#define SIMPLE_CONVERT_IOCTL(fd, cmd, arg, __struc) ({  \
+   int __ret;  \
+   struct __struc *req = arg;  \
+   uint32_t type = req->type;  \
+   req->type = convert_type(type); \
+   __ret = SYS_IOCTL(fd, cmd, arg);\
+   req->type = type;   \
+   __ret;  \
+   })
+
+#ifndef min
+#define min(a, b) (((a) < (b)) ? (a) : (b))
+#endif
+
+#ifndef max
+#define max(a, b) (((a) > (b)) ? (a) : (b))
+#endif
+
+#define EXYNOS4_FIMC_DRV   "exynos4-fimc"
+#define 

Re: [PATCH v3] media: i2c-polling: add i2c-polling driver

[Laurent Pinchart]

Hi Matt,

On Thursday 24 Nov 2016 00:04:24 Matt Ranostay wrote:
> On Wed, Nov 23, 2016 at 10:31 PM, Matt Ranostay wrote:
> > On Wed, Nov 23, 2016 at 8:30 AM, Laurent Pinchart wrote:
> >> On Tuesday 22 Nov 2016 17:18:40 Matt Ranostay wrote:
> >>> There are several thermal sensors that only have a low-speed bus
> >>> interface but output valid video data. This patchset enables support
> >>> for the AMG88xx "Grid-Eye" sensor family.
> >>> 
> >>> Cc: Attila Kinali 
> >>> Cc: Marek Vasut 
> >>> Cc: Luca Barbato 
> >>> Signed-off-by: Matt Ranostay 
> >>> ---
> >>> Changes from v1:
> >>> * correct i2c_polling_remove() operations
> >>> * fixed delay calcuation in buffer_queue()
> >>> * add include linux/slab.h
> >>> 
> >>> Changes from v2:
> >>> * fix build error due to typo in include of slab.h
> >>> 
> >>>  drivers/media/i2c/Kconfig   |   8 +
> >>>  drivers/media/i2c/Makefile  |   1 +
> >>>  drivers/media/i2c/i2c-polling.c | 469 +
> >> 
> >> Just looking at the driver name I believe a rename is needed. i2c-polling
> >> is a very generic name and would mislead many people into thinking about
> >> an I2C subsystem core feature instead of a video driver. "video-i2c" is
> >> one option, I'm open to other ideas.
> >> 
> >>>  3 files changed, 478 insertions(+)
> >>>  create mode 100644 drivers/media/i2c/i2c-polling.c

[snip]

> >>> diff --git a/drivers/media/i2c/i2c-polling.c
> >>> b/drivers/media/i2c/i2c-polling.c new file mode 100644
> >>> index ..46a4eecde2d2
> >>> --- /dev/null
> >>> +++ b/drivers/media/i2c/i2c-polling.c

[snip]

> >>> +static void buffer_queue(struct vb2_buffer *vb)
> >>> +{
> >>> + struct i2c_polling_data *data = vb2_get_drv_priv(vb->vb2_queue);
> >>> + unsigned int delay = 1000 / data->chip->max_fps;
> >>> + int delta;
> >>> +
> >>> + mutex_lock(>lock);
> >>> +
> >>> + delta = jiffies - data->last_update;
> >>> +
> >>> + if (delta < msecs_to_jiffies(delay)) {
> >>> + int tmp = (delay - jiffies_to_msecs(delta)) * 1000;
> >>> +
> >>> + usleep_range(tmp, tmp + 1000);
> >>> + }
> >>> + data->last_update = jiffies;
> >>> +
> >>> + mutex_unlock(>lock);
> >>> +
> >>> + vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
> >>> +}
> >>> +
> >>> +static void buffer_finish(struct vb2_buffer *vb)
> >>> +{
> >>> + struct i2c_polling_data *data = vb2_get_drv_priv(vb->vb2_queue);
> >>> + void *vbuf = vb2_plane_vaddr(vb, 0);
> >>> + int size = vb2_plane_size(vb, 0);
> >>> + int ret;
> >>> +
> >>> + mutex_lock(>lock);
> >>> +
> >>> + ret = data->chip->xfer(data, vbuf);
> >>> + if (ret < 0)
> >>> + vb->state = VB2_BUF_STATE_ERROR;
> >> 
> >> That's not nice, the status should be set through vb2_buffer_done().
> >> 
> >> You can't transfer data in the buffer_queue handler is that function
> >> can't sleep. Instead, I'm wondering whether it would make sense to
> >> perform transfers in a workqueue, to making timings less dependent on
> >> userspace.
> > 
> > About the workqueue how would one signal to that the buffer is written
> > to buffer_queue/buffer_finish?
> 
> Testing the workqueue way and it isn't fine enough... need to use some
> form of the high resolution timers. Need to figure the best way to do
> that with signaling back to the queue functions.. would completion
> queues make sense?

How about a kthread, as done in the vivid driver ?

> >>> + mutex_unlock(>lock);
> >>> +
> >>> + vb->timestamp = ktime_get_ns();
> >>> + vb2_set_plane_payload(vb, 0, ret ? 0 : size);
> >>> +}

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 6/7] mediactl: libv4l2subdev: add support for comparing mbus formats

[Jacek Anaszewski]

On 11/24/2016 03:36 PM, Sakari Ailus wrote:

Hi Jacek,

On Wed, Oct 12, 2016 at 04:35:21PM +0200, Jacek Anaszewski wrote:

This patch adds a function for checking whether two mbus formats
are compatible.


Compatible doesn't in general case mean the same as... the same.

On parallel busses a 10-bit source can be connected to an 8-bit sink-for
instance.

How about moving this to the plugin, and if someone else needs it, then we
move it out later?


This is a good idea, as I am checking not all fields of
v4l2_mbus_framefmt, but only those which matter during Exynos4 media
devuce pipeline format negotiation.

--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 4/7] mediactl: Add media_device creation helpers

[Jacek Anaszewski]

On 11/24/2016 03:32 PM, Sakari Ailus wrote:
[...]

+   sprintf(device_dir_path, "/sys/class/video4linux/%s/device/", p + 1);
+
+   device_dir = opendir(device_dir_path);
+   if (device_dir == NULL)
+   return NULL;
+
+   while ((entry = readdir(device_dir))) {
+   if (strncmp(entry->d_name, "media", 4))


Why 4? And isn't entry->d_name nul-terminated, so you could use strcmp()?


Media devices, as other devices, have numerical postfix, which is
not of our interest.


Right. But still 5 would be the right number as we should also check the
last "a".


Of course, this needs to be fixed, thanks.

--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 7/7] Add a libv4l plugin for Exynos4 camera

[Sakari Ailus]

Hi Jacek,

Thank you for your continued work on the Exynos plugin patchset!

I think we're pretty close to being able to merge the set, if you could
still bear with me awhile. :-)

On Wed, Oct 12, 2016 at 04:35:22PM +0200, Jacek Anaszewski wrote:
...
> diff --git a/lib/libv4l-exynos4-camera/Makefile.am 
> b/lib/libv4l-exynos4-camera/Makefile.am
> new file mode 100644
> index 000..c38b7f6
> --- /dev/null
> +++ b/lib/libv4l-exynos4-camera/Makefile.am
> @@ -0,0 +1,19 @@
> +if WITH_V4L_PLUGINS
> +libv4l2plugin_LTLIBRARIES = libv4l-exynos4-camera.la
> +endif
> +
> +media-bus-format-names.h: ../../include/linux/media-bus-format.h
> + sed -e '/#define MEDIA_BUS_FMT/ ! d; s/.*FMT_//; /FIXED/ d; s/\t.*//; 
> s/.*/{ \"&\", MEDIA_BUS_FMT_& },/;' \
> + < $< > $@
> +
> +media-bus-format-codes.h: ../../include/linux/media-bus-format.h
> + sed -e '/#define MEDIA_BUS_FMT/ ! d; s/.*#define //; /FIXED/ d; 
> s/\t.*//; s/.*/ &,/;' \
> + < $< > $@
> +
> +BUILT_SOURCES = media-bus-format-names.h media-bus-format-codes.h
> +CLEANFILES = $(BUILT_SOURCES)

It'd be nice to be able to use the same generated headers that now are under
utils/media-ctl, instead of copying the sed script here in verbatim. If the
script is changed or fixed in some way, the other location probably will
remain unchanged...

I wonder if there's a proper way to generate build time headers such as
these.

Another less good alternative would be to put these into a separate Makefile
and include that Makefile where the headers are needed. But I don't like
that much either, it's a hack.

> +
> +nodist_libv4l_exynos4_camera_la_SOURCES = $(BUILT_SOURCES)
> +libv4l_exynos4_camera_la_SOURCES = libv4l-exynos4-camera.c 
> ../../utils/media-ctl/libmediactl.c ../../utils/media-ctl/libv4l2subdev.c 
> ../../utils/media-ctl/mediatext.c
> +libv4l_exynos4_camera_la_CFLAGS = -fvisibility=hidden -std=gnu99
> +libv4l_exynos4_camera_la_LDFLAGS = -avoid-version -module -shared 
> -export-dynamic -lpthread
> diff --git a/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c 
> b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
> new file mode 100644
> index 000..c219fe5
> --- /dev/null
> +++ b/lib/libv4l-exynos4-camera/libv4l-exynos4-camera.c
> @@ -0,0 +1,1325 @@
> +/*
> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
> + *  http://www.samsung.com
> + *
> + * Author: Jacek Anaszewski 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published 
> by
> + * the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "../../utils/media-ctl/mediactl.h"
> +#include "../../utils/media-ctl/mediatext.h"
> +#include "../../utils/media-ctl/v4l2subdev.h"
> +#include "libv4l-plugin.h"
> +
> +#ifdef DEBUG
> +#define V4L2_EXYNOS4_DBG(format, ARG...)\
> + printf("[%s:%d] [%s] " format " \n", __FILE__, __LINE__, __func__, 
> ##ARG)
> +#else
> +#define V4L2_EXYNOS4_DBG(format, ARG...)
> +#endif
> +
> +#define V4L2_EXYNOS4_ERR(format, ARG...)\
> + fprintf(stderr, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
> +
> +#define V4L2_EXYNOS4_LOG(format, ARG...)\
> + fprintf(stdout, "Libv4l Exynos4 camera plugin: "format "\n", ##ARG)
> +
> +#define VIDIOC_CTRL(type)\
> + ((type) == VIDIOC_S_CTRL ? "VIDIOC_S_CTRL" :\
> +"VIDIOC_G_CTRL")
> +
> +#if HAVE_VISIBILITY
> +#define PLUGIN_PUBLIC __attribute__ ((visibility("default")))
> +#else
> +#define PLUGIN_PUBLIC
> +#endif
> +
> +#define SYS_IOCTL(fd, cmd, arg) \
> + syscall(SYS_ioctl, (int)(fd), (unsigned long)(cmd), (void *)(arg))
> +
> +#define SIMPLE_CONVERT_IOCTL(fd, cmd, arg, __struc) ({  \
> + int __ret;  \
> + struct __struc *req = arg;  \
> + uint32_t type = req->type;  \
> + req->type = convert_type(type); \
> + __ret = SYS_IOCTL(fd, cmd, arg);\
> + req->type = type;   \
> + __ret;  \
> + })
> +
> +#ifndef min
> +#define min(a, b) (((a) < (b)) ? (a) : (b))
> +#endif
> +
> +#ifndef max
> +#define max(a, b) (((a) > (b)) ? (a) : (b))
> +#endif
> +
> +#define EXYNOS4_FIMC_DRV "exynos4-fimc"
> +#define EXYNOS4_FIMC_LITE_DRV"exynos-fimc-lit"
> +#define EXYNOS4_FIMC_IS_ISP_DRV  "exynos4-fimc-is"
> 

Re: [PATCH v4l-utils v7 6/7] mediactl: libv4l2subdev: add support for comparing mbus formats

[Sakari Ailus]

Hi Jacek,

On Wed, Oct 12, 2016 at 04:35:21PM +0200, Jacek Anaszewski wrote:
> This patch adds a function for checking whether two mbus formats
> are compatible.

Compatible doesn't in general case mean the same as... the same.

On parallel busses a 10-bit source can be connected to an 8-bit sink-for
instance.

How about moving this to the plugin, and if someone else needs it, then we
move it out later?

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 4/7] mediactl: Add media_device creation helpers

[Sakari Ailus]

Hi Jacek,

On Thu, Nov 24, 2016 at 02:50:39PM +0100, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> Thanks for the review.
> 
> On 11/24/2016 01:17 PM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >Thanks for the patchset.
> >
> >On Wed, Oct 12, 2016 at 04:35:19PM +0200, Jacek Anaszewski wrote:
> >>Add helper functions that allow for easy instantiation of media_device
> >>object basing on whether the media device contains v4l2 subdev with
> >>given file descriptor.
> >
> >Doesn't this work with video nodes as well? That's what you seem to be using
> >it for later on. And I think that's actually more useful.
> 
> Exactly, thanks for spotting this.
> 
> s/v4l2 subdev/video device opened/
> 
> >
> >The existing implementation uses udev to look up devices. Could you use
> >libudev device enumeration API to find the media devices, and fall back to
> >sysfs if udev doesn't work? There seems to be a reasonable-looking example
> >here:
> >
> >
> 
> I'll check that, thanks.
> 
> >>
> >>Signed-off-by: Jacek Anaszewski 
> >>Acked-by: Kyungmin Park 
> >>---
> >> utils/media-ctl/libmediactl.c | 131 
> >> +-
> >> utils/media-ctl/mediactl.h|  27 +
> >> 2 files changed, 156 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
> >>index 155b65f..d347a40 100644
> >>--- a/utils/media-ctl/libmediactl.c
> >>+++ b/utils/media-ctl/libmediactl.c
> >>@@ -27,6 +27,7 @@
> >> #include 
> >>
> >> #include 
> >>+#include 
> >> #include 
> >> #include 
> >> #include 
> >>@@ -440,8 +441,9 @@ static int media_get_devname_udev(struct udev *udev,
> >>return -EINVAL;
> >>
> >>devnum = makedev(entity->info.v4l.major, entity->info.v4l.minor);
> >>-   media_dbg(entity->media, "looking up device: %u:%u\n",
> >>- major(devnum), minor(devnum));
> >>+   if (entity->media)
> >>+   media_dbg(entity->media, "looking up device: %u:%u\n",
> >>+ major(devnum), minor(devnum));
> >>device = udev_device_new_from_devnum(udev, 'c', devnum);
> >>if (device) {
> >>p = udev_device_get_devnode(device);
> >>@@ -523,6 +525,7 @@ static int media_get_devname_sysfs(struct media_entity 
> >>*entity)
> >>return 0;
> >> }
> >>
> >>+
> >
> >Unrelated change.
> >
> >> static int media_enum_entities(struct media_device *media)
> >> {
> >>struct media_entity *entity;
> >>@@ -707,6 +710,92 @@ struct media_device *media_device_new(const char 
> >>*devnode)
> >>return media;
> >> }
> >>
> >>+struct media_device *media_device_new_by_subdev_fd(int fd, struct 
> >>media_entity **fd_entity)
> >>+{
> >>+   char video_devname[32], device_dir_path[256], media_dev_path[256], 
> >>media_major_minor[10];
> >>+   struct media_device *media = NULL;
> >>+   struct dirent *entry;
> >>+   struct media_entity tmp_entity;
> >>+   DIR *device_dir;
> >>+   struct udev *udev;
> >>+   char *p;
> >>+   int ret, i;
> >>+
> >>+   if (fd_entity == NULL)
> >>+   return NULL;
> >>+
> >>+   ret = media_get_devname_by_fd(fd, video_devname);
> >>+   if (ret < 0)
> >>+   return NULL;
> >>+
> >>+   p = strrchr(video_devname, '/');
> >>+   if (p == NULL)
> >>+   return NULL;
> >>+
> >>+   ret = media_udev_open();
> >>+   if (ret < 0)
> >>+   return NULL;
> >>+
> >>+   sprintf(device_dir_path, "/sys/class/video4linux/%s/device/", p + 1);
> >>+
> >>+   device_dir = opendir(device_dir_path);
> >>+   if (device_dir == NULL)
> >>+   return NULL;
> >>+
> >>+   while ((entry = readdir(device_dir))) {
> >>+   if (strncmp(entry->d_name, "media", 4))
> >
> >Why 4? And isn't entry->d_name nul-terminated, so you could use strcmp()?
> 
> Media devices, as other devices, have numerical postfix, which is
> not of our interest.

Right. But still 5 would be the right number as we should also check the
last "a".

> 
> >>+   continue;
> >>+
> >>+   sprintf(media_dev_path, "%s%s/dev", device_dir_path, 
> >>entry->d_name);
> >>+
> >>+   fd = open(media_dev_path, O_RDONLY);
> >>+   if (fd < 0)
> >>+   continue;
> >>+
> >>+   ret = read(fd, media_major_minor, sizeof(media_major_minor));
> >>+   if (ret < 0)
> >>+   continue;
> >>+
> >>+   sscanf(media_major_minor, "%d:%d", _entity.info.dev.major, 
> >>_entity.info.dev.minor);
> >
> >This would be better split on two lines.
> 
> OK.
> 
> >>+
> >>+   /* Try to get the device name via udev */
> >>+   if (media_get_devname_udev(udev, _entity)) {
> >>+   /* Fall back to get the device name via sysfs */
> >>+   if (media_get_devname_sysfs(_entity))
> >>+   continue;
> >>+   }
> >>+
> >>+   media = 

Re: [PATCH v4l-utils v7 3/7] mediactl: Add media_entity_get_backlinks()

[Sakari Ailus]

Hi Jacek,

On Thu, Nov 24, 2016 at 03:00:46PM +0100, Jacek Anaszewski wrote:
> Hi Sakari,
> 
> Thanks for the review.

It's taken way too long. :-( My apologies for that.

> On 11/24/2016 01:40 PM, Sakari Ailus wrote:
> >Hi Jacek,
> >
> >On Wed, Oct 12, 2016 at 04:35:18PM +0200, Jacek Anaszewski wrote:
> >>Add a new graph helper useful for discovering video pipeline.
> >>
> >>Signed-off-by: Jacek Anaszewski 
> >>Acked-by: Kyungmin Park 
> >>---
> >> utils/media-ctl/libmediactl.c | 21 +
> >> utils/media-ctl/mediactl.h| 15 +++
> >> 2 files changed, 36 insertions(+)
> >>
> >>diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
> >>index 91ed003..155b65f 100644
> >>--- a/utils/media-ctl/libmediactl.c
> >>+++ b/utils/media-ctl/libmediactl.c
> >>@@ -36,6 +36,7 @@
> >> #include 
> >>
> >> #include 
> >>+#include 
> >
> >Is there something that needs this one in the patch?
> 
> MAJOR and MINOR macros.

Ok.

> 
> >
> >> #include 
> >>
> >> #include "mediactl.h"
> >>@@ -172,6 +173,26 @@ const struct media_entity_desc 
> >>*media_entity_get_info(struct media_entity *entit
> >>return >info;
> >> }
> >>
> >>+int media_entity_get_backlinks(struct media_entity *entity,
> >>+   struct media_link **backlinks,
> >>+   unsigned int *num_backlinks)
> >>+{
> >>+   unsigned int num_bklinks = 0;
> >>+   int i;
> >>+
> >>+   if (entity == NULL || backlinks == NULL || num_backlinks == NULL)
> >>+   return -EINVAL;
> >>+
> >
> >If you have an interface that accesses a memory buffer of unknown size, you
> >need to verify that the user has provided a buffer large enough.
> >
> >How about using the num_backlinks argument to provide the maximum size to
> >the function, and passing the actual number to the user, the latter of which
> >you already do?
> 
> Sounds reasonable.
> 
> >Alternatively, an iterator style API could be nice as well. Up to you.
> 
> It would probably need an addition of some generic infrastructure.
> I suppose that there is no such a feature in v4l-utils?

You basically need to store a value for the framework to tell which entity
is being worked on. So nothing too fancy.

I guess Laurent would prefer the current interface but I let him answer
that.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 1/7] mediactl: Add support for v4l2-ctrl-binding config

[Sakari Ailus]

Hi Jacek,

On Wed, Oct 12, 2016 at 04:35:16PM +0200, Jacek Anaszewski wrote:
> Make struct v4l2_subdev capable of aggregating v4l2-ctrl-bindings -
> media device configuration entries. Added are also functions for
> validating support for the control on given media entity and checking
> whether a v4l2-ctrl-binding has been defined for a media entity.

I still don't think this belongs here.

I think I told you about the generic pipeline configuration library I worked
on years ago; unfortunately it was left on prototype stage. Still, what I
realised was that something very similar is needed in that library ---
associating information to the representation of the media entities (or the
V4L2 sub-devices) in user space that has got nothing to do with the devices
themselves.

We could have e.g. a list of key--value pairs where the key is a pointer
provided by the user (i.e. application, another library) that could be
associated with the value. That would avoid having explicit information on
that in the struct media_entity itself.

An quicker alternative would be to manage a list of controls e.g. in the
plugin itself and store the media entity where they're implemented in that
list, with the control value.

Cc Laurent as well.

> 
> Signed-off-by: Jacek Anaszewski 
> Acked-by: Kyungmin Park 
> ---
>  utils/media-ctl/libv4l2subdev.c | 32 
>  utils/media-ctl/v4l2subdev.h| 19 +++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
> index c3439d7..4f8ee7f 100644
> --- a/utils/media-ctl/libv4l2subdev.c
> +++ b/utils/media-ctl/libv4l2subdev.c
> @@ -50,7 +50,15 @@ int v4l2_subdev_create(struct media_entity *entity)
>  
>   entity->sd->fd = -1;
>  
> + entity->sd->v4l2_ctrl_bindings = malloc(sizeof(__u32));
> + if (entity->sd->v4l2_ctrl_bindings == NULL)
> + goto err_v4l2_ctrl_bindings_alloc;
> +
>   return 0;
> +
> +err_v4l2_ctrl_bindings_alloc:
> + free(entity->sd);
> + return -ENOMEM;
>  }
>  
>  int v4l2_subdev_create_opened(struct media_entity *entity, int fd)
> @@ -102,6 +110,7 @@ void v4l2_subdev_close(struct media_entity *entity)
>   if (entity->sd->fd_owner)
>   close(entity->sd->fd);
>  
> + free(entity->sd->v4l2_ctrl_bindings);
>   free(entity->sd);
>  }
>  
> @@ -884,3 +893,26 @@ const enum v4l2_mbus_pixelcode 
> *v4l2_subdev_pixelcode_list(unsigned int *length)
>  
>   return mbus_codes;
>  }
> +
> +int v4l2_subdev_supports_v4l2_ctrl(struct media_device *media,
> +struct media_entity *entity,
> +__u32 ctrl_id)
> +{
> + struct v4l2_queryctrl queryctrl = {};
> + int ret;
> +
> + ret = v4l2_subdev_open(entity);
> + if (ret < 0)
> + return ret;
> +
> + queryctrl.id = ctrl_id;
> +
> + ret = ioctl(entity->sd->fd, VIDIOC_QUERYCTRL, );
> + if (ret < 0)
> + return ret;
> +
> + media_dbg(media, "Validated control \"%s\" (0x%8.8x) on entity %s\n",
> +   queryctrl.name, queryctrl.id, entity->info.name);
> +
> + return 0;
> +}
> diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h
> index 011fab1..4dee6b1 100644
> --- a/utils/media-ctl/v4l2subdev.h
> +++ b/utils/media-ctl/v4l2subdev.h
> @@ -26,10 +26,14 @@
>  
>  struct media_device;
>  struct media_entity;
> +struct media_device;
>  
>  struct v4l2_subdev {
>   int fd;
>   unsigned int fd_owner:1;
> +
> + __u32 *v4l2_ctrl_bindings;
> + unsigned int num_v4l2_ctrl_bindings;
>  };
>  
>  /**
> @@ -314,4 +318,19 @@ enum v4l2_field v4l2_subdev_string_to_field(const char 
> *string);
>  const enum v4l2_mbus_pixelcode *v4l2_subdev_pixelcode_list(
>   unsigned int *length);
>  
> +/**
> + * @brief Check if sub-device supports given v4l2 control
> + * @param media - media device.
> + * @param entity - media entity.
> + * @param ctrl_id - id of the v4l2 control to check.
> + *
> + * Verify if the sub-device associated with given media entity
> + * supports v4l2-control with given ctrl_id.
> + *
> + * @return 1 if the control is supported, 0 otherwise.
> + */
> +int v4l2_subdev_supports_v4l2_ctrl(struct media_device *device,
> +struct media_entity *entity,
> +__u32 ctrl_id);
> +
>  #endif

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 2/7] mediatext: Add library

[Jacek Anaszewski]

Hi Sakari,

On 11/24/2016 02:01 PM, Sakari Ailus wrote:

Hi Jacek,

On Wed, Oct 12, 2016 at 04:35:17PM +0200, Jacek Anaszewski wrote:

libmediatext is a helper library for converting configurations (Media
controller links, V4L2 controls and V4L2 sub-device media bus formats and
selections) from text-based form into IOCTLs.

libmediatext depends on libv4l2subdev and libmediactl.


I'm not very happy with the plugin using the mediatext library as it was in
2013. I've heavily changed (and extended) the library since to cover
additional use cases such as the request API:



This version is not meaningfully extensible in forward-compatible way. In
order to resolve the matter quickly without making merging the further
developed library difficult, I propose merging the code to the plugin
itself. We could later check if it could be refactored.

How about that?


It works for me.

Thanks,
Jacek Anaszewski


Signed-off-by: Sakari Ailus 
Signed-off-by: Teemu Tuominen 
Signed-off-by: Jacek Anaszewski 
---
 utils/media-ctl/Makefile.am|  10 +-
 utils/media-ctl/libmediatext.pc.in |  10 ++
 utils/media-ctl/mediatext-test.c   |  64 
 utils/media-ctl/mediatext.c| 312 +
 utils/media-ctl/mediatext.h|  52 +++
 5 files changed, 446 insertions(+), 2 deletions(-)
 create mode 100644 utils/media-ctl/libmediatext.pc.in
 create mode 100644 utils/media-ctl/mediatext-test.c
 create mode 100644 utils/media-ctl/mediatext.c
 create mode 100644 utils/media-ctl/mediatext.h

diff --git a/utils/media-ctl/Makefile.am b/utils/media-ctl/Makefile.am
index ee7dcc9..2f12357 100644
--- a/utils/media-ctl/Makefile.am
+++ b/utils/media-ctl/Makefile.am
@@ -1,4 +1,4 @@
-noinst_LTLIBRARIES = libmediactl.la libv4l2subdev.la
+noinst_LTLIBRARIES = libmediactl.la libv4l2subdev.la libmediatext.la

 libmediactl_la_SOURCES = libmediactl.c mediactl-priv.h
 libmediactl_la_CFLAGS = -static $(LIBUDEV_CFLAGS)
@@ -21,9 +21,15 @@ libv4l2subdev_la_LIBADD = libmediactl.la
 libv4l2subdev_la_CFLAGS = -static
 libv4l2subdev_la_LDFLAGS = -static

+libmediatext_la_SOURCES = mediatext.c
+libmediatext_la_CFLAGS = -static $(LIBUDEV_CFLAGS)
+libmediatext_la_LDFLAGS = -static $(LIBUDEV_LIBS)
+
 mediactl_includedir=$(includedir)/mediactl
 noinst_HEADERS = mediactl.h v4l2subdev.h

-bin_PROGRAMS = media-ctl
+bin_PROGRAMS = media-ctl mediatext-test
 media_ctl_SOURCES = media-ctl.c options.c options.h tools.h
 media_ctl_LDADD = libmediactl.la libv4l2subdev.la
+mediatext_test_SOURCES = mediatext-test.c
+mediatext_test_LDADD = libmediatext.la libmediactl.la libv4l2subdev.la
diff --git a/utils/media-ctl/libmediatext.pc.in 
b/utils/media-ctl/libmediatext.pc.in
new file mode 100644
index 000..6aa6353
--- /dev/null
+++ b/utils/media-ctl/libmediatext.pc.in
@@ -0,0 +1,10 @@
+prefix=@prefix@
+exec_prefix=@exec_prefix@
+libdir=@libdir@
+includedir=@includedir@
+
+Name: libmediatext
+Description: Media controller and V4L2 text-based configuration library
+Version: @PACKAGE_VERSION@
+Cflags: -I${includedir}
+Libs: -L${libdir} -lmediatext
diff --git a/utils/media-ctl/mediatext-test.c b/utils/media-ctl/mediatext-test.c
new file mode 100644
index 000..b8b9282
--- /dev/null
+++ b/utils/media-ctl/mediatext-test.c
@@ -0,0 +1,64 @@
+/*
+ * libmediatext test program
+ *
+ * Copyright (C) 2013 Intel Corporation
+ *
+ * Contact: Sakari Ailus 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; either version 2.1 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see .
+ */
+
+#include 
+#include 
+#include 
+
+#include "mediactl.h"
+#include "mediatext.h"
+
+int main(int argc, char *argv[])
+{
+   struct media_device *device;
+   int rval;
+
+   if (argc != 3) {
+   fprintf(stderr, "usage: %s  \n\n", 
argv[0]);
+   fprintf(stderr, "\tstring := [ v4l2-ctrl |?v4l2-mbus | link-reset | 
link-conf]\n\n");
+   fprintf(stderr, "\tv4l2-ctrl := \"entity\" ctrl_type ctrl_id 
ctrl_value\n");
+   fprintf(stderr, "\tctrl_type := [ int | int64 | bitmask ]\n");
+   fprintf(stderr, "\tctrl_value := [ %%d | %%PRId64 | bitmask_value 
]\n");
+   fprintf(stderr, "\tbitmask_value := b\n\n");
+   fprintf(stderr, "\tv4l2-mbus := \n");
+

Re: [PATCH v4l-utils v7 3/7] mediactl: Add media_entity_get_backlinks()

[Jacek Anaszewski]

Hi Sakari,

Thanks for the review.

On 11/24/2016 01:40 PM, Sakari Ailus wrote:

Hi Jacek,

On Wed, Oct 12, 2016 at 04:35:18PM +0200, Jacek Anaszewski wrote:

Add a new graph helper useful for discovering video pipeline.

Signed-off-by: Jacek Anaszewski 
Acked-by: Kyungmin Park 
---
 utils/media-ctl/libmediactl.c | 21 +
 utils/media-ctl/mediactl.h| 15 +++
 2 files changed, 36 insertions(+)

diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
index 91ed003..155b65f 100644
--- a/utils/media-ctl/libmediactl.c
+++ b/utils/media-ctl/libmediactl.c
@@ -36,6 +36,7 @@
 #include 

 #include 
+#include 


Is there something that needs this one in the patch?


MAJOR and MINOR macros.




 #include 

 #include "mediactl.h"
@@ -172,6 +173,26 @@ const struct media_entity_desc 
*media_entity_get_info(struct media_entity *entit
return >info;
 }

+int media_entity_get_backlinks(struct media_entity *entity,
+   struct media_link **backlinks,
+   unsigned int *num_backlinks)
+{
+   unsigned int num_bklinks = 0;
+   int i;
+
+   if (entity == NULL || backlinks == NULL || num_backlinks == NULL)
+   return -EINVAL;
+


If you have an interface that accesses a memory buffer of unknown size, you
need to verify that the user has provided a buffer large enough.

How about using the num_backlinks argument to provide the maximum size to
the function, and passing the actual number to the user, the latter of which
you already do?


Sounds reasonable.


Alternatively, an iterator style API could be nice as well. Up to you.


It would probably need an addition of some generic infrastructure.
I suppose that there is no such a feature in v4l-utils?



I wonder what Laurent thinks.


+   for (i = 0; i < entity->num_links; ++i)
+   if ((entity->links[i].flags & MEDIA_LNK_FL_ENABLED) &&
+   (entity->links[i].sink->entity == entity))
+   backlinks[num_bklinks++] = >links[i];
+
+   *num_backlinks = num_bklinks;
+
+   return 0;
+}
+
 /* 
-
  * Open/close
  */
diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
index 336cbf9..b1f33cd 100644
--- a/utils/media-ctl/mediactl.h
+++ b/utils/media-ctl/mediactl.h
@@ -434,6 +434,20 @@ int media_parse_setup_link(struct media_device *media,
 int media_parse_setup_links(struct media_device *media, const char *p);

 /**
+ * @brief Get entity's enabled backlinks
+ * @param entity - media entity.
+ * @param backlinks - array of pointers to matching backlinks.
+ * @param num_backlinks - number of matching backlinks.
+ *
+ * Get links that are connected to the entity sink pads.
+ *
+ * @return 0 on success, or a negative error code on failure.
+ */
+int media_entity_get_backlinks(struct media_entity *entity,
+   struct media_link **backlinks,
+   unsigned int *num_backlinks);
+
+/**
  * @brief Get v4l2_subdev for the entity
  * @param entity - media entity
  *
@@ -443,4 +457,5 @@ int media_parse_setup_links(struct media_device *media, 
const char *p);
  */
 struct v4l2_subdev *media_entity_get_v4l2_subdev(struct media_entity *entity);

+


Unrelated change.


 #endif





--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 4/7] mediactl: Add media_device creation helpers

[Jacek Anaszewski]

Hi Sakari,

Thanks for the review.

On 11/24/2016 01:17 PM, Sakari Ailus wrote:

Hi Jacek,

Thanks for the patchset.

On Wed, Oct 12, 2016 at 04:35:19PM +0200, Jacek Anaszewski wrote:

Add helper functions that allow for easy instantiation of media_device
object basing on whether the media device contains v4l2 subdev with
given file descriptor.


Doesn't this work with video nodes as well? That's what you seem to be using
it for later on. And I think that's actually more useful.


Exactly, thanks for spotting this.

s/v4l2 subdev/video device opened/



The existing implementation uses udev to look up devices. Could you use
libudev device enumeration API to find the media devices, and fall back to
sysfs if udev doesn't work? There seems to be a reasonable-looking example
here:




I'll check that, thanks.



Signed-off-by: Jacek Anaszewski 
Acked-by: Kyungmin Park 
---
 utils/media-ctl/libmediactl.c | 131 +-
 utils/media-ctl/mediactl.h|  27 +
 2 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
index 155b65f..d347a40 100644
--- a/utils/media-ctl/libmediactl.c
+++ b/utils/media-ctl/libmediactl.c
@@ -27,6 +27,7 @@
 #include 

 #include 
+#include 
 #include 
 #include 
 #include 
@@ -440,8 +441,9 @@ static int media_get_devname_udev(struct udev *udev,
return -EINVAL;

devnum = makedev(entity->info.v4l.major, entity->info.v4l.minor);
-   media_dbg(entity->media, "looking up device: %u:%u\n",
- major(devnum), minor(devnum));
+   if (entity->media)
+   media_dbg(entity->media, "looking up device: %u:%u\n",
+ major(devnum), minor(devnum));
device = udev_device_new_from_devnum(udev, 'c', devnum);
if (device) {
p = udev_device_get_devnode(device);
@@ -523,6 +525,7 @@ static int media_get_devname_sysfs(struct media_entity 
*entity)
return 0;
 }

+


Unrelated change.


 static int media_enum_entities(struct media_device *media)
 {
struct media_entity *entity;
@@ -707,6 +710,92 @@ struct media_device *media_device_new(const char *devnode)
return media;
 }

+struct media_device *media_device_new_by_subdev_fd(int fd, struct media_entity 
**fd_entity)
+{
+   char video_devname[32], device_dir_path[256], media_dev_path[256], 
media_major_minor[10];
+   struct media_device *media = NULL;
+   struct dirent *entry;
+   struct media_entity tmp_entity;
+   DIR *device_dir;
+   struct udev *udev;
+   char *p;
+   int ret, i;
+
+   if (fd_entity == NULL)
+   return NULL;
+
+   ret = media_get_devname_by_fd(fd, video_devname);
+   if (ret < 0)
+   return NULL;
+
+   p = strrchr(video_devname, '/');
+   if (p == NULL)
+   return NULL;
+
+   ret = media_udev_open();
+   if (ret < 0)
+   return NULL;
+
+   sprintf(device_dir_path, "/sys/class/video4linux/%s/device/", p + 1);
+
+   device_dir = opendir(device_dir_path);
+   if (device_dir == NULL)
+   return NULL;
+
+   while ((entry = readdir(device_dir))) {
+   if (strncmp(entry->d_name, "media", 4))


Why 4? And isn't entry->d_name nul-terminated, so you could use strcmp()?


Media devices, as other devices, have numerical postfix, which is
not of our interest.


+   continue;
+
+   sprintf(media_dev_path, "%s%s/dev", device_dir_path, 
entry->d_name);
+
+   fd = open(media_dev_path, O_RDONLY);
+   if (fd < 0)
+   continue;
+
+   ret = read(fd, media_major_minor, sizeof(media_major_minor));
+   if (ret < 0)
+   continue;
+
+   sscanf(media_major_minor, "%d:%d", _entity.info.dev.major, 
_entity.info.dev.minor);


This would be better split on two lines.


OK.


+
+   /* Try to get the device name via udev */
+   if (media_get_devname_udev(udev, _entity)) {
+   /* Fall back to get the device name via sysfs */
+   if (media_get_devname_sysfs(_entity))
+   continue;
+   }
+
+   media = media_device_new(tmp_entity.devname);
+   if (media == NULL)
+   continue;
+
+   ret = media_device_enumerate(media);
+   if (ret < 0) {
+   media_dbg(media, "Failed to enumerate %s (%d)\n",
+ tmp_entity.devname, ret);
+   media_device_unref(media);
+   media = NULL;
+   continue;
+   }
+
+  

Re: ir-keytable: infinite loops, segfaults

[Sean Young]

On Thu, Nov 24, 2016 at 11:12:57PM +1100, Vincent McIntyre wrote:
> On Wed, Nov 23, 2016 at 10:34:19PM +, Sean Young wrote:
> > > Not sure why Driver is (null), dvb_usb_cxusb is loaded.
> > 
> > That's a mistake, I've fixed that now.
> 
> Ah. I see the added module_name struct members.
> 
> > > I tried -t and it generated events constantly, before I could press
> > > any keys.
> > > # ir-keytable -s rc1 -t
> > > Testing events. Please, press CTRL-C to abort.
> > > 1479903007.535509: event type EV_MSC(0x04): scancode = 0x00
> > > 1479903007.535509: event type EV_SYN(0x00).
> > > 1479903007.635521: event type EV_MSC(0x04): scancode = 0x00
> > 
> > That's also been fixed.
> > 
> 
> yep, works nicely.
> 
> Things are looking much better!
> As shown below I am able to clear a keytable and put in a fresh one.
> Having a bit of trouble with key remapping.
> I guess we still have to work out the protocol in use.
> 
> Test details:
> # ir-keytable -v
> Found device /sys/class/rc/rc0/
> Found device /sys/class/rc/rc1/
> Found device /sys/class/rc/rc2/
> Input sysfs node is /sys/class/rc/rc0/input8/
> Event sysfs node is /sys/class/rc/rc0/input8/event5/
> Parsing uevent /sys/class/rc/rc0/input8/event5/uevent
> /sys/class/rc/rc0/input8/event5/uevent uevent MAJOR=13
> /sys/class/rc/rc0/input8/event5/uevent uevent MINOR=69
> /sys/class/rc/rc0/input8/event5/uevent uevent DEVNAME=input/event5
> Parsing uevent /sys/class/rc/rc0/uevent
> /sys/class/rc/rc0/uevent uevent NAME=rc-imon-mce
> /sys/class/rc/rc0/uevent uevent DRV_NAME=imon
> input device is /dev/input/event5
> /sys/class/rc/rc0/protocols protocol rc-6 (enabled)
> Found /sys/class/rc/rc0/ (/dev/input/event5) with:
>   Driver imon, table rc-imon-mce
>   Supported protocols: rc-6 
>   Enabled protocols: rc-6 
>   Name: iMON Remote (15c2:ffdc)
>   bus: 3, vendor/product: 15c2:ffdc, version: 0x
> Input sysfs node is /sys/class/rc/rc1/input18/
> Event sysfs node is /sys/class/rc/rc1/input18/event15/
> Parsing uevent /sys/class/rc/rc1/input18/event15/uevent
> /sys/class/rc/rc1/input18/event15/uevent uevent MAJOR=13
> /sys/class/rc/rc1/input18/event15/uevent uevent MINOR=79
> /sys/class/rc/rc1/input18/event15/uevent uevent DEVNAME=input/event15
> Parsing uevent /sys/class/rc/rc1/uevent
> /sys/class/rc/rc1/uevent uevent NAME=rc-dvico-mce
> /sys/class/rc/rc1/uevent uevent DRV_NAME=dvb_usb_cxusb
> input device is /dev/input/event15
> /sys/class/rc/rc1/protocols protocol unknown (disabled)
> Found /sys/class/rc/rc1/ (/dev/input/event15) with:
>   Driver dvb_usb_cxusb, table rc-dvico-mce
>   Supported protocols: unknown 
>   Enabled protocols: 
>   Name: IR-receiver inside an USB DVB re
>   bus: 3, vendor/product: 0fe9:db78, version: 0x827b
> Input sysfs node is /sys/class/rc/rc2/input19/
> Event sysfs node is /sys/class/rc/rc2/input19/event16/
> Parsing uevent /sys/class/rc/rc2/input19/event16/uevent
> /sys/class/rc/rc2/input19/event16/uevent uevent MAJOR=13
> /sys/class/rc/rc2/input19/event16/uevent uevent MINOR=80
> /sys/class/rc/rc2/input19/event16/uevent uevent DEVNAME=input/event16
> Parsing uevent /sys/class/rc/rc2/uevent
> /sys/class/rc/rc2/uevent uevent NAME=rc-empty
> /sys/class/rc/rc2/uevent uevent DRV_NAME=dvb_usb_af9035
> input device is /dev/input/event16
> /sys/class/rc/rc2/protocols protocol nec (disabled)
> Found /sys/class/rc/rc2/ (/dev/input/event16) with:
>   Driver dvb_usb_af9035, table rc-empty
>   Supported protocols: nec 
>   Enabled protocols: 
>   Name: Leadtek WinFast DTV Dongle Dual
>   bus: 3, vendor/product: 0413:6a05, version: 0x0200
>   Repeat delay = 500 ms, repeat period = 125 ms
>   Repeat delay = 500 ms, repeat period = 125 ms
>   Repeat delay = 500 ms, repeat period = 125 ms
> 
> # ir-keytable -r -v -s rc1
> Found device /sys/class/rc/rc0/
> Found device /sys/class/rc/rc1/
> Found device /sys/class/rc/rc2/
> Input sysfs node is /sys/class/rc/rc1/input18/
> Event sysfs node is /sys/class/rc/rc1/input18/event15/
> Parsing uevent /sys/class/rc/rc1/input18/event15/uevent
> /sys/class/rc/rc1/input18/event15/uevent uevent MAJOR=13
> /sys/class/rc/rc1/input18/event15/uevent uevent MINOR=79
> /sys/class/rc/rc1/input18/event15/uevent uevent DEVNAME=input/event15
> Parsing uevent /sys/class/rc/rc1/uevent
> /sys/class/rc/rc1/uevent uevent NAME=rc-dvico-mce
> /sys/class/rc/rc1/uevent uevent DRV_NAME=dvb_usb_cxusb
> input device is /dev/input/event15
> /sys/class/rc/rc1/protocols protocol unknown (disabled)
> Opening /dev/input/event15
> Input Protocol version: 0x00010001
> Enabled protocols: 
> scancode 0xfe01 = KEY_RECORD (0xa7)
> scancode 0xfe02 = KEY_TV (0x179)
> scancode 0xfe03 = KEY_0 (0x0b)
> scancode 0xfe05 = KEY_VOLUMEDOWN (0x72)
> scancode 0xfe07 = KEY_4 (0x05)
> scancode 0xfe09 = KEY_CHANNELDOWN (0x193)
> scancode 0xfe0a = KEY_EPG (0x16d)
> scancode 0xfe0b = KEY_1 (0x02)
> scancode 0xfe0d = KEY_STOP (0x80)
> scancode 0xfe0e = KEY_MP3 (0x187)
> scancode 0xfe0f = 

Re: [PATCH v3 4/9] media: venus: vdec: add video decoder files

[Stanimir Varbanov]

Hi Nicolas,

On 11/23/2016 10:24 PM, Nicolas Dufresne wrote:
> Le lundi 21 novembre 2016 à 18:09 +0200, Stanimir Varbanov a écrit :
 Meanwhile I have found bigger obstacle - I cannot run multiple
>> instances
 simultaneously. By m2m design it can execute only one job (m2m
>> context)
 at a time per m2m device. Can you confirm that my observation is
>> correct?
>>>  
>>> The m2m framework assumes a single HW instance, yes. Do you have
>> multiple
>>> HW decoders? I might not understand what you mean...
>>>  
>>
>> I mean that I can start and execute up to 16 decoder sessions
>> simultaneously. Its a firmware responsibility how those sessions are
>> scheduled and how the hardware is shared between them. Of course
>> depending on the resolution the firmware can refuse to start the
>> session
>> because the hardware will be overloaded and will not be able to
>> satisfy
>> the bitrate requirements.
> 
> This is similar to S5P-MFC driver, which you may have notice not use
> m2m framework.

Thanks for the note.

I have started to look into m2m because Hans asked me to reuse the ioctl
helpers that it provides.

I have no problem with usage of the m2m API if they help me to reduce
code size and doesn't impact performance.


-- 
regards,
Stan
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 2/7] mediatext: Add library

[Sakari Ailus]

Hi Jacek,

On Wed, Oct 12, 2016 at 04:35:17PM +0200, Jacek Anaszewski wrote:
> libmediatext is a helper library for converting configurations (Media
> controller links, V4L2 controls and V4L2 sub-device media bus formats and
> selections) from text-based form into IOCTLs.
> 
> libmediatext depends on libv4l2subdev and libmediactl.

I'm not very happy with the plugin using the mediatext library as it was in
2013. I've heavily changed (and extended) the library since to cover
additional use cases such as the request API:



This version is not meaningfully extensible in forward-compatible way. In
order to resolve the matter quickly without making merging the further
developed library difficult, I propose merging the code to the plugin
itself. We could later check if it could be refactored.

How about that?

> 
> Signed-off-by: Sakari Ailus 
> Signed-off-by: Teemu Tuominen 
> Signed-off-by: Jacek Anaszewski 
> ---
>  utils/media-ctl/Makefile.am|  10 +-
>  utils/media-ctl/libmediatext.pc.in |  10 ++
>  utils/media-ctl/mediatext-test.c   |  64 
>  utils/media-ctl/mediatext.c| 312 
> +
>  utils/media-ctl/mediatext.h|  52 +++
>  5 files changed, 446 insertions(+), 2 deletions(-)
>  create mode 100644 utils/media-ctl/libmediatext.pc.in
>  create mode 100644 utils/media-ctl/mediatext-test.c
>  create mode 100644 utils/media-ctl/mediatext.c
>  create mode 100644 utils/media-ctl/mediatext.h
> 
> diff --git a/utils/media-ctl/Makefile.am b/utils/media-ctl/Makefile.am
> index ee7dcc9..2f12357 100644
> --- a/utils/media-ctl/Makefile.am
> +++ b/utils/media-ctl/Makefile.am
> @@ -1,4 +1,4 @@
> -noinst_LTLIBRARIES = libmediactl.la libv4l2subdev.la
> +noinst_LTLIBRARIES = libmediactl.la libv4l2subdev.la libmediatext.la
>  
>  libmediactl_la_SOURCES = libmediactl.c mediactl-priv.h
>  libmediactl_la_CFLAGS = -static $(LIBUDEV_CFLAGS)
> @@ -21,9 +21,15 @@ libv4l2subdev_la_LIBADD = libmediactl.la
>  libv4l2subdev_la_CFLAGS = -static
>  libv4l2subdev_la_LDFLAGS = -static
>  
> +libmediatext_la_SOURCES = mediatext.c
> +libmediatext_la_CFLAGS = -static $(LIBUDEV_CFLAGS)
> +libmediatext_la_LDFLAGS = -static $(LIBUDEV_LIBS)
> +
>  mediactl_includedir=$(includedir)/mediactl
>  noinst_HEADERS = mediactl.h v4l2subdev.h
>  
> -bin_PROGRAMS = media-ctl
> +bin_PROGRAMS = media-ctl mediatext-test
>  media_ctl_SOURCES = media-ctl.c options.c options.h tools.h
>  media_ctl_LDADD = libmediactl.la libv4l2subdev.la
> +mediatext_test_SOURCES = mediatext-test.c
> +mediatext_test_LDADD = libmediatext.la libmediactl.la libv4l2subdev.la
> diff --git a/utils/media-ctl/libmediatext.pc.in 
> b/utils/media-ctl/libmediatext.pc.in
> new file mode 100644
> index 000..6aa6353
> --- /dev/null
> +++ b/utils/media-ctl/libmediatext.pc.in
> @@ -0,0 +1,10 @@
> +prefix=@prefix@
> +exec_prefix=@exec_prefix@
> +libdir=@libdir@
> +includedir=@includedir@
> +
> +Name: libmediatext
> +Description: Media controller and V4L2 text-based configuration library
> +Version: @PACKAGE_VERSION@
> +Cflags: -I${includedir}
> +Libs: -L${libdir} -lmediatext
> diff --git a/utils/media-ctl/mediatext-test.c 
> b/utils/media-ctl/mediatext-test.c
> new file mode 100644
> index 000..b8b9282
> --- /dev/null
> +++ b/utils/media-ctl/mediatext-test.c
> @@ -0,0 +1,64 @@
> +/*
> + * libmediatext test program
> + *
> + * Copyright (C) 2013 Intel Corporation
> + *
> + * Contact: Sakari Ailus 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; either version 2.1 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program. If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "mediactl.h"
> +#include "mediatext.h"
> +
> +int main(int argc, char *argv[])
> +{
> + struct media_device *device;
> + int rval;
> +
> + if (argc != 3) {
> + fprintf(stderr, "usage: %s  \n\n", 
> argv[0]);
> + fprintf(stderr, "\tstring := [ v4l2-ctrl |?v4l2-mbus | 
> link-reset | link-conf]\n\n");
> + fprintf(stderr, "\tv4l2-ctrl := \"entity\" ctrl_type ctrl_id 
> ctrl_value\n");
> + fprintf(stderr, "\tctrl_type := [ int | int64 | bitmask ]\n");
> + fprintf(stderr, "\tctrl_value := [ %%d | %%PRId64 | 
> bitmask_value ]\n");
> + 

Re: [PATCH v4l-utils v7 3/7] mediactl: Add media_entity_get_backlinks()

[Sakari Ailus]

Hi Jacek,

On Wed, Oct 12, 2016 at 04:35:18PM +0200, Jacek Anaszewski wrote:
> Add a new graph helper useful for discovering video pipeline.
> 
> Signed-off-by: Jacek Anaszewski 
> Acked-by: Kyungmin Park 
> ---
>  utils/media-ctl/libmediactl.c | 21 +
>  utils/media-ctl/mediactl.h| 15 +++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
> index 91ed003..155b65f 100644
> --- a/utils/media-ctl/libmediactl.c
> +++ b/utils/media-ctl/libmediactl.c
> @@ -36,6 +36,7 @@
>  #include 
>  
>  #include 
> +#include 

Is there something that needs this one in the patch?

>  #include 
>  
>  #include "mediactl.h"
> @@ -172,6 +173,26 @@ const struct media_entity_desc 
> *media_entity_get_info(struct media_entity *entit
>   return >info;
>  }
>  
> +int media_entity_get_backlinks(struct media_entity *entity,
> + struct media_link **backlinks,
> + unsigned int *num_backlinks)
> +{
> + unsigned int num_bklinks = 0;
> + int i;
> +
> + if (entity == NULL || backlinks == NULL || num_backlinks == NULL)
> + return -EINVAL;
> +

If you have an interface that accesses a memory buffer of unknown size, you
need to verify that the user has provided a buffer large enough.

How about using the num_backlinks argument to provide the maximum size to
the function, and passing the actual number to the user, the latter of which
you already do?

Alternatively, an iterator style API could be nice as well. Up to you.

I wonder what Laurent thinks.

> + for (i = 0; i < entity->num_links; ++i)
> + if ((entity->links[i].flags & MEDIA_LNK_FL_ENABLED) &&
> + (entity->links[i].sink->entity == entity))
> + backlinks[num_bklinks++] = >links[i];
> +
> + *num_backlinks = num_bklinks;
> +
> + return 0;
> +}
> +
>  /* 
> -
>   * Open/close
>   */
> diff --git a/utils/media-ctl/mediactl.h b/utils/media-ctl/mediactl.h
> index 336cbf9..b1f33cd 100644
> --- a/utils/media-ctl/mediactl.h
> +++ b/utils/media-ctl/mediactl.h
> @@ -434,6 +434,20 @@ int media_parse_setup_link(struct media_device *media,
>  int media_parse_setup_links(struct media_device *media, const char *p);
>  
>  /**
> + * @brief Get entity's enabled backlinks
> + * @param entity - media entity.
> + * @param backlinks - array of pointers to matching backlinks.
> + * @param num_backlinks - number of matching backlinks.
> + *
> + * Get links that are connected to the entity sink pads.
> + *
> + * @return 0 on success, or a negative error code on failure.
> + */
> +int media_entity_get_backlinks(struct media_entity *entity,
> + struct media_link **backlinks,
> + unsigned int *num_backlinks);
> +
> +/**
>   * @brief Get v4l2_subdev for the entity
>   * @param entity - media entity
>   *
> @@ -443,4 +457,5 @@ int media_parse_setup_links(struct media_device *media, 
> const char *p);
>   */
>  struct v4l2_subdev *media_entity_get_v4l2_subdev(struct media_entity 
> *entity);
>  
> +

Unrelated change.

>  #endif

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 5/7] mediactl: libv4l2subdev: Add colorspace logging

[Sakari Ailus]

Hi Jacek,

On Wed, Oct 12, 2016 at 04:35:20PM +0200, Jacek Anaszewski wrote:
> Add a function for obtaining colorspace name by id.
> 
> Signed-off-by: Jacek Anaszewski 
> Acked-by: Kyungmin Park 
> ---
>  utils/media-ctl/libv4l2subdev.c | 32 
>  utils/media-ctl/v4l2subdev.h| 10 ++
>  2 files changed, 42 insertions(+)
> 
> diff --git a/utils/media-ctl/libv4l2subdev.c b/utils/media-ctl/libv4l2subdev.c
> index 4f8ee7f..31393bb 100644
> --- a/utils/media-ctl/libv4l2subdev.c
> +++ b/utils/media-ctl/libv4l2subdev.c
> @@ -33,6 +33,7 @@
>  #include 
>  
>  #include 
> +#include 
>  
>  #include "mediactl.h"
>  #include "mediactl-priv.h"
> @@ -831,6 +832,37 @@ const char *v4l2_subdev_pixelcode_to_string(enum 
> v4l2_mbus_pixelcode code)
>   return "unknown";
>  }
>  
> +static struct {

static const

> + const char *name;
> + enum v4l2_colorspace cs;
> +} colorspaces[] = {
> +{ "DEFAULT", V4L2_COLORSPACE_DEFAULT },
> +{ "SMPTE170M", V4L2_COLORSPACE_SMPTE170M },
> +{ "SMPTE240M", V4L2_COLORSPACE_SMPTE240M },
> +{ "REC709", V4L2_COLORSPACE_REC709 },
> +{ "BT878", V4L2_COLORSPACE_BT878 },
> +{ "470_SYSTEM_M", V4L2_COLORSPACE_470_SYSTEM_M },
> +{ "470_SYSTEM_BG", V4L2_COLORSPACE_470_SYSTEM_BG },
> +{ "JPEG", V4L2_COLORSPACE_JPEG },
> +{ "SRGB", V4L2_COLORSPACE_SRGB },
> +{ "ADOBERGB", V4L2_COLORSPACE_ADOBERGB },
> +{ "BT2020", V4L2_COLORSPACE_BT2020 },
> +{ "RAW", V4L2_COLORSPACE_RAW },
> +{ "DCI_P3", V4L2_COLORSPACE_DCI_P3 },
> +};
> +
> +const char *v4l2_subdev_colorspace_to_string(enum v4l2_colorspace cs)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(colorspaces); ++i) {
> + if (colorspaces[i].cs == cs)
> + return colorspaces[i].name;
> + }
> +
> + return "unknown";
> +}
> +
>  enum v4l2_mbus_pixelcode v4l2_subdev_string_to_pixelcode(const char *string)
>  {
>   unsigned int i;
> diff --git a/utils/media-ctl/v4l2subdev.h b/utils/media-ctl/v4l2subdev.h
> index 4dee6b1..cf1250d 100644
> --- a/utils/media-ctl/v4l2subdev.h
> +++ b/utils/media-ctl/v4l2subdev.h
> @@ -278,6 +278,16 @@ int v4l2_subdev_parse_setup_formats(struct media_device 
> *media, const char *p);
>  const char *v4l2_subdev_pixelcode_to_string(enum v4l2_mbus_pixelcode code);
>  
>  /**
> + * @brief Convert colorspace to string.
> + * @param code - input string
> + *
> + * Convert colorspace @a to a human-readable string.

@a code ?

With these,

Acked-by: Sakari Ailus 

> + *
> + * @return A pointer to a string on success, NULL on failure.
> + */
> +const char *v4l2_subdev_colorspace_to_string(enum v4l2_colorspace cs);
> +
> +/**
>   * @brief Parse string to media bus pixel code.
>   * @param string - nul terminalted string, textual media bus pixel code
>   *

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 0/7] Add a plugin for Exynos4 camera

[Jacek Anaszewski]

On 11/24/2016 10:10 AM, Mauro Carvalho Chehab wrote:

Em Thu, 24 Nov 2016 09:10:28 +0100
Jacek Anaszewski  escreveu:


On 11/23/2016 10:51 PM, Mauro Carvalho Chehab wrote:

Em Thu, 03 Nov 2016 13:13:12 +0100
Jacek Anaszewski  escreveu:


Hi Hans,

On 11/03/2016 12:51 PM, Hans Verkuil wrote:

Hi all,

Is there anything that blocks me from merging this?

This plugin work has been ongoing for years and unless there are serious
objections I propose that this is merged.

Jacek, is there anything missing that would prevent merging this?


There were issues raised by Sakari during last review, related to
the way how v4l2 control bindings are defined. That discussion wasn't
finished, so I stayed by my approach.


Are those things something that could be fixed later without
breaking binary apps? If not, then perhaps it is time to merge
this.


They are related to the format of configuration file being introduced
by this patch set, so there are no binary apps depending on it.


Ok. Yet, changing the configuration file format could be a pain
for the users. The best would be to add a format that would be
extensible enough to be able to add support for other needs in
the future.


Few days ago I had a discussion with Sakari on IRC, and he pointed
me to his patch set adding an extended version of his mediatext
library RFC [0], the first version of which I included to my patch set,
and extended a bit. A decision should be made if I should adapt my
API to the mediatext RFC. RFC is not a stable ground though.

[0] https://www.spinics.net/lists/linux-media/msg103242.html


If I understood well, you're proposing to use this format for
the configuration file:

link-conf "s5p-mipi-csis.0":1 -> "FIMC.0":0 [1]
ctrl-to-subdev-conf 0x0098091f -> "fimc.0.capture"
ctrl-to-subdev-conf 0x00980902 -> "S5C73M3"
ctrl-to-subdev-conf 0x00980922 -> "fimc.0.capture"
ctrl-to-subdev-conf 0x009a0914 -> "S5C73M3"

What if, instead, you use something like:

[group]
key = value

type of configuration file?

This way, if one wants to extend it, all it is needed is to add
other groups or more key types.

The users with an older configuration won't suffer if newer
groups and newer key types were added.

E. g something like:

[link]
source = "s5p-mipi-csis.0":1
sink = "FIMC.0":0 [1]

[ctrl-to-subdev]
ctrl_id = 0x0098091f
subdev= "fimc.0.capture"

[ctrl-to-subdev]
ctrl_id = 0x00980902
subdev = "S5C73M3"

...


This way, Sakari's patches could use the same format to add mediatext
entries. E. g. something like (the actual content actually depends on
what is here - I'm just wild guessing from the tree diagram at the
RFC e-mail):

[mediatext]
shell_script = foo
devnode = /dev/video*
devnode = /dev/mediaX
devnode = /dev/v4l-subdev*

without needing to change the format of the file, and keeping it
backward compatible.


First version of this patch set used similar configuration format
but Sakari asked me to switch to the format accepted by media-ctl,
which the first version of mediatext adhered to.

Sakari has pledged to review this patch set, so we will be able
to decide on how to proceed no sooner than that.

Best regards,
Jacek Anaszewski



Regards,
Mauro



Other than that - I've tested it
and it works fine both with GStreamer and my test app.

Best regards,
Jacek Anaszewski



On 12/10/16 16:35, Jacek Anaszewski wrote:

This is a seventh version of the patch series adding a plugin for the
Exynos4 camera. Last version [0] of the patch set was posted in
January.

The plugin doesn't link against libmediactl, but has its sources
compiled in. Currently utils are built after the plugins, but
libv4l-exynos4-camera plugin depends on the utils. In order to link
the plugin against libmediactl the build system would have to be
modified.


Changes from v6:


- close v4l2 sub-devices on media device release
- moved non-generic code from libmediactl to the plugin
- resigned from adding libmedia_ioctl library and moved all its
  code to the plugin, since it depended on pipeline representation,
  which was not generic for all possible media device topologies
- used media_get_info()->name instead of adding media_entity_get_name
- renamed media_get_backlinks_by_entity() to
media_entity_get_backlinks(()
- moved pipeline from struct media_device to the plugin
- changed the way of associating video device file descriptor with
media device
- switched to using auto-generated media-bus-format-names.h header file
- renamed v4l2-ctrl-redir config entry name to v4l2-ctrl-binding


Changes from v5:


- fixed and tested use cases with S5K6A3 sensor and FIMC-IS-ISP
- added conversion "colorspace id to string"


Changes from v4:


- removed some redundant functions for traversing media device graph
  and 

Re: [PATCH v4l-utils v7 4/7] mediactl: Add media_device creation helpers

[Sakari Ailus]

Hi Jacek,

Thanks for the patchset.

On Wed, Oct 12, 2016 at 04:35:19PM +0200, Jacek Anaszewski wrote:
> Add helper functions that allow for easy instantiation of media_device
> object basing on whether the media device contains v4l2 subdev with
> given file descriptor.

Doesn't this work with video nodes as well? That's what you seem to be using
it for later on. And I think that's actually more useful.

The existing implementation uses udev to look up devices. Could you use
libudev device enumeration API to find the media devices, and fall back to
sysfs if udev doesn't work? There seems to be a reasonable-looking example
here:



> 
> Signed-off-by: Jacek Anaszewski 
> Acked-by: Kyungmin Park 
> ---
>  utils/media-ctl/libmediactl.c | 131 
> +-
>  utils/media-ctl/mediactl.h|  27 +
>  2 files changed, 156 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/media-ctl/libmediactl.c b/utils/media-ctl/libmediactl.c
> index 155b65f..d347a40 100644
> --- a/utils/media-ctl/libmediactl.c
> +++ b/utils/media-ctl/libmediactl.c
> @@ -27,6 +27,7 @@
>  #include 
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -440,8 +441,9 @@ static int media_get_devname_udev(struct udev *udev,
>   return -EINVAL;
>  
>   devnum = makedev(entity->info.v4l.major, entity->info.v4l.minor);
> - media_dbg(entity->media, "looking up device: %u:%u\n",
> -   major(devnum), minor(devnum));
> + if (entity->media)
> + media_dbg(entity->media, "looking up device: %u:%u\n",
> +   major(devnum), minor(devnum));
>   device = udev_device_new_from_devnum(udev, 'c', devnum);
>   if (device) {
>   p = udev_device_get_devnode(device);
> @@ -523,6 +525,7 @@ static int media_get_devname_sysfs(struct media_entity 
> *entity)
>   return 0;
>  }
>  
> +

Unrelated change.

>  static int media_enum_entities(struct media_device *media)
>  {
>   struct media_entity *entity;
> @@ -707,6 +710,92 @@ struct media_device *media_device_new(const char 
> *devnode)
>   return media;
>  }
>  
> +struct media_device *media_device_new_by_subdev_fd(int fd, struct 
> media_entity **fd_entity)
> +{
> + char video_devname[32], device_dir_path[256], media_dev_path[256], 
> media_major_minor[10];
> + struct media_device *media = NULL;
> + struct dirent *entry;
> + struct media_entity tmp_entity;
> + DIR *device_dir;
> + struct udev *udev;
> + char *p;
> + int ret, i;
> +
> + if (fd_entity == NULL)
> + return NULL;
> +
> + ret = media_get_devname_by_fd(fd, video_devname);
> + if (ret < 0)
> + return NULL;
> +
> + p = strrchr(video_devname, '/');
> + if (p == NULL)
> + return NULL;
> +
> + ret = media_udev_open();
> + if (ret < 0)
> + return NULL;
> +
> + sprintf(device_dir_path, "/sys/class/video4linux/%s/device/", p + 1);
> +
> + device_dir = opendir(device_dir_path);
> + if (device_dir == NULL)
> + return NULL;
> +
> + while ((entry = readdir(device_dir))) {
> + if (strncmp(entry->d_name, "media", 4))

Why 4? And isn't entry->d_name nul-terminated, so you could use strcmp()?

> + continue;
> +
> + sprintf(media_dev_path, "%s%s/dev", device_dir_path, 
> entry->d_name);
> +
> + fd = open(media_dev_path, O_RDONLY);
> + if (fd < 0)
> + continue;
> +
> + ret = read(fd, media_major_minor, sizeof(media_major_minor));
> + if (ret < 0)
> + continue;
> +
> + sscanf(media_major_minor, "%d:%d", _entity.info.dev.major, 
> _entity.info.dev.minor);

This would be better split on two lines.

> +
> + /* Try to get the device name via udev */
> + if (media_get_devname_udev(udev, _entity)) {
> + /* Fall back to get the device name via sysfs */
> + if (media_get_devname_sysfs(_entity))
> + continue;
> + }
> +
> + media = media_device_new(tmp_entity.devname);
> + if (media == NULL)
> + continue;
> +
> + ret = media_device_enumerate(media);
> + if (ret < 0) {
> + media_dbg(media, "Failed to enumerate %s (%d)\n",
> +   tmp_entity.devname, ret);
> + media_device_unref(media);
> + media = NULL;
> + continue;
> + }
> +
> + /* Get the entity associated with given fd */
> + for (i = 0; i < media->entities_count; i++) {
> + struct media_entity *entity 

Re: [PATCH v2 2/3] [media] ivtv: use pr_foo() instead of calling printk() directly

[Hans Verkuil]

On 11/24/16 12:52, Mauro Carvalho Chehab wrote:

pr_foo() provides a convenient way for printk's, enforcing
that they'll all prepend the error message with the driver's
name.

Use it inside ivtv.

Signed-off-by: Mauro Carvalho Chehab 


Acked-by: Hans Verkuil 

Thanks!

Hans


---
 drivers/media/pci/ivtv/ivtv-alsa-main.c | 11 +--
 drivers/media/pci/ivtv/ivtv-driver.c| 12 ++--
 drivers/media/pci/ivtv/ivtvfb.c |  6 +++---
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-alsa-main.c 
b/drivers/media/pci/ivtv/ivtv-alsa-main.c
index 67ab73ef2bca..f7a036844f02 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-main.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-main.c
@@ -34,10 +34,11 @@
 int ivtv_alsa_debug;
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;

-#define IVTV_DEBUG_ALSA_INFO(fmt, arg...) \
+#define IVTV_DEBUG_ALSA_INFO(__fmt, __arg...) \
do { \
if (ivtv_alsa_debug & 2) \
-   pr_info("%s: " fmt, "ivtv-alsa", ## arg); \
+   printk(KERN_INFO pr_fmt("%s: alsa:" __fmt),   \
+  __func__, ##__arg);  \
} while (0)

 module_param_named(debug, ivtv_alsa_debug, int, 0644);
@@ -226,8 +227,7 @@ static int ivtv_alsa_load(struct ivtv *itv)

s = >streams[IVTV_ENC_STREAM_TYPE_PCM];
if (s->vdev.v4l2_dev == NULL) {
-   IVTV_DEBUG_ALSA_INFO("%s: PCM stream for card is disabled - 
skipping\n",
-__func__);
+   IVTV_DEBUG_ALSA_INFO("PCM stream for card is disabled - 
skipping\n");
return 0;
}

@@ -241,8 +241,7 @@ static int ivtv_alsa_load(struct ivtv *itv)
IVTV_ALSA_ERR("%s: failed to create struct snd_ivtv_card\n",
  __func__);
} else {
-   IVTV_DEBUG_ALSA_INFO("%s: created ivtv ALSA interface instance 
\n",
-__func__);
+   IVTV_DEBUG_ALSA_INFO("created ivtv ALSA interface instance\n");
}
return 0;
 }
diff --git a/drivers/media/pci/ivtv/ivtv-driver.c 
b/drivers/media/pci/ivtv/ivtv-driver.c
index 0a3b80a4bd69..ab2ae53618e8 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.c
+++ b/drivers/media/pci/ivtv/ivtv-driver.c
@@ -1452,7 +1452,7 @@ static void ivtv_remove(struct pci_dev *pdev)
for (i = 0; i < IVTV_VBI_FRAMES; i++)
kfree(itv->vbi.sliced_mpeg_data[i]);

-   printk(KERN_INFO "ivtv: Removed %s\n", itv->card_name);
+   pr_info("Removed %s\n", itv->card_name);

v4l2_device_unregister(>v4l2_dev);
kfree(itv);
@@ -1468,25 +1468,25 @@ static struct pci_driver ivtv_pci_driver = {

 static int __init module_start(void)
 {
-   printk(KERN_INFO "ivtv: Start initialization, version %s\n", 
IVTV_VERSION);
+   pr_info("Start initialization, version %s\n", IVTV_VERSION);

/* Validate parameters */
if (ivtv_first_minor < 0 || ivtv_first_minor >= IVTV_MAX_CARDS) {
-   printk(KERN_ERR "ivtv: Exiting, ivtv_first_minor must be between 0 
and %d\n",
+   pr_err("Exiting, ivtv_first_minor must be between 0 and %d\n",
 IVTV_MAX_CARDS - 1);
return -1;
}

if (ivtv_debug < 0 || ivtv_debug > 2047) {
ivtv_debug = 0;
-   printk(KERN_INFO "ivtv: Debug value must be >= 0 and <= 
2047\n");
+   pr_info("Debug value must be >= 0 and <= 2047\n");
}

if (pci_register_driver(_pci_driver)) {
-   printk(KERN_ERR "ivtv: Error detecting PCI card\n");
+   pr_err("Error detecting PCI card\n");
return -ENODEV;
}
-   printk(KERN_INFO "ivtv: End initialization\n");
+   pr_info("End initialization\n");
return 0;
 }

diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index b59b60d605eb..621b2f613d81 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -1270,7 +1270,7 @@ static int __init ivtvfb_init(void)


if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
-   printk(KERN_ERR "ivtvfb:  ivtvfb_card_id parameter is out of range 
(valid range: -1 - %d)\n",
+   pr_err("ivtvfb_card_id parameter is out of range (valid range: -1 - 
%d)\n",
 IVTV_MAX_CARDS - 1);
return -EINVAL;
}
@@ -1279,7 +1279,7 @@ static int __init ivtvfb_init(void)
err = driver_for_each_device(drv, NULL, , 
ivtvfb_callback_init);
(void)err;  /* suppress compiler warning */
if (!registered) {
-   printk(KERN_ERR "ivtvfb:  no cards found\n");
+   pr_err("no cards found\n");
return -ENODEV;
}
return 0;
@@ -1290,7 +1290,7 @@ static void 

Re: ir-keytable: infinite loops, segfaults

[Vincent McIntyre]

On Wed, Nov 23, 2016 at 10:34:19PM +, Sean Young wrote:
> > Not sure why Driver is (null), dvb_usb_cxusb is loaded.
> 
> That's a mistake, I've fixed that now.

Ah. I see the added module_name struct members.

> > I tried -t and it generated events constantly, before I could press
> > any keys.
> > # ir-keytable -s rc1 -t
> > Testing events. Please, press CTRL-C to abort.
> > 1479903007.535509: event type EV_MSC(0x04): scancode = 0x00
> > 1479903007.535509: event type EV_SYN(0x00).
> > 1479903007.635521: event type EV_MSC(0x04): scancode = 0x00
> 
> That's also been fixed.
> 

yep, works nicely.

Things are looking much better!
As shown below I am able to clear a keytable and put in a fresh one.
Having a bit of trouble with key remapping.
I guess we still have to work out the protocol in use.

Test details:
# ir-keytable -v
Found device /sys/class/rc/rc0/
Found device /sys/class/rc/rc1/
Found device /sys/class/rc/rc2/
Input sysfs node is /sys/class/rc/rc0/input8/
Event sysfs node is /sys/class/rc/rc0/input8/event5/
Parsing uevent /sys/class/rc/rc0/input8/event5/uevent
/sys/class/rc/rc0/input8/event5/uevent uevent MAJOR=13
/sys/class/rc/rc0/input8/event5/uevent uevent MINOR=69
/sys/class/rc/rc0/input8/event5/uevent uevent DEVNAME=input/event5
Parsing uevent /sys/class/rc/rc0/uevent
/sys/class/rc/rc0/uevent uevent NAME=rc-imon-mce
/sys/class/rc/rc0/uevent uevent DRV_NAME=imon
input device is /dev/input/event5
/sys/class/rc/rc0/protocols protocol rc-6 (enabled)
Found /sys/class/rc/rc0/ (/dev/input/event5) with:
Driver imon, table rc-imon-mce
Supported protocols: rc-6 
Enabled protocols: rc-6 
Name: iMON Remote (15c2:ffdc)
bus: 3, vendor/product: 15c2:ffdc, version: 0x
Input sysfs node is /sys/class/rc/rc1/input18/
Event sysfs node is /sys/class/rc/rc1/input18/event15/
Parsing uevent /sys/class/rc/rc1/input18/event15/uevent
/sys/class/rc/rc1/input18/event15/uevent uevent MAJOR=13
/sys/class/rc/rc1/input18/event15/uevent uevent MINOR=79
/sys/class/rc/rc1/input18/event15/uevent uevent DEVNAME=input/event15
Parsing uevent /sys/class/rc/rc1/uevent
/sys/class/rc/rc1/uevent uevent NAME=rc-dvico-mce
/sys/class/rc/rc1/uevent uevent DRV_NAME=dvb_usb_cxusb
input device is /dev/input/event15
/sys/class/rc/rc1/protocols protocol unknown (disabled)
Found /sys/class/rc/rc1/ (/dev/input/event15) with:
Driver dvb_usb_cxusb, table rc-dvico-mce
Supported protocols: unknown 
Enabled protocols: 
Name: IR-receiver inside an USB DVB re
bus: 3, vendor/product: 0fe9:db78, version: 0x827b
Input sysfs node is /sys/class/rc/rc2/input19/
Event sysfs node is /sys/class/rc/rc2/input19/event16/
Parsing uevent /sys/class/rc/rc2/input19/event16/uevent
/sys/class/rc/rc2/input19/event16/uevent uevent MAJOR=13
/sys/class/rc/rc2/input19/event16/uevent uevent MINOR=80
/sys/class/rc/rc2/input19/event16/uevent uevent DEVNAME=input/event16
Parsing uevent /sys/class/rc/rc2/uevent
/sys/class/rc/rc2/uevent uevent NAME=rc-empty
/sys/class/rc/rc2/uevent uevent DRV_NAME=dvb_usb_af9035
input device is /dev/input/event16
/sys/class/rc/rc2/protocols protocol nec (disabled)
Found /sys/class/rc/rc2/ (/dev/input/event16) with:
Driver dvb_usb_af9035, table rc-empty
Supported protocols: nec 
Enabled protocols: 
Name: Leadtek WinFast DTV Dongle Dual
bus: 3, vendor/product: 0413:6a05, version: 0x0200
Repeat delay = 500 ms, repeat period = 125 ms
Repeat delay = 500 ms, repeat period = 125 ms
Repeat delay = 500 ms, repeat period = 125 ms

# ir-keytable -r -v -s rc1
Found device /sys/class/rc/rc0/
Found device /sys/class/rc/rc1/
Found device /sys/class/rc/rc2/
Input sysfs node is /sys/class/rc/rc1/input18/
Event sysfs node is /sys/class/rc/rc1/input18/event15/
Parsing uevent /sys/class/rc/rc1/input18/event15/uevent
/sys/class/rc/rc1/input18/event15/uevent uevent MAJOR=13
/sys/class/rc/rc1/input18/event15/uevent uevent MINOR=79
/sys/class/rc/rc1/input18/event15/uevent uevent DEVNAME=input/event15
Parsing uevent /sys/class/rc/rc1/uevent
/sys/class/rc/rc1/uevent uevent NAME=rc-dvico-mce
/sys/class/rc/rc1/uevent uevent DRV_NAME=dvb_usb_cxusb
input device is /dev/input/event15
/sys/class/rc/rc1/protocols protocol unknown (disabled)
Opening /dev/input/event15
Input Protocol version: 0x00010001
Enabled protocols: 
scancode 0xfe01 = KEY_RECORD (0xa7)
scancode 0xfe02 = KEY_TV (0x179)
scancode 0xfe03 = KEY_0 (0x0b)
scancode 0xfe05 = KEY_VOLUMEDOWN (0x72)
scancode 0xfe07 = KEY_4 (0x05)
scancode 0xfe09 = KEY_CHANNELDOWN (0x193)
scancode 0xfe0a = KEY_EPG (0x16d)
scancode 0xfe0b = KEY_1 (0x02)
scancode 0xfe0d = KEY_STOP (0x80)
scancode 0xfe0e = KEY_MP3 (0x187)
scancode 0xfe0f = KEY_PREVIOUSSONG (0xa5)
scancode 0xfe11 = KEY_CHANNELUP (0x192)
scancode 0xfe12 = KEY_NEXTSONG (0xa3)
scancode 0xfe13 = KEY_ANGLE (0x173)
scancode 0xfe15 = KEY_VOLUMEUP (0x73)
scancode 0xfe16 = KEY_SETUP (0x8d)
scancode 0xfe17 = KEY_2 (0x03)
scancode 0xfe19 = 

[PATCH v2 3/3] [media] ivtv: mark DVB "borrowed" ioctls as deprecated

[Mauro Carvalho Chehab]

changeset da8ec560e3b4 ("[media] ivtv: implement new decoder command
ioctls") implemented proper support for mpeg audio and video control
at V4L2 API. Since then, the usage of the the DVB APIs is deprecated.

However, we never actually marked it as deprecated nor provided any
way to disable it. Let's do it now.

This patch prepares for the removal of this bad usage on a couple
of Kernel versions.

Fixes: da8ec560e3b4 ("[media] ivtv: implement new decoder command ioctls")
Acked-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/ivtv/Kconfig   | 13 ++
 drivers/media/pci/ivtv/ivtv-driver.h |  2 --
 drivers/media/pci/ivtv/ivtv-ioctl.c  | 49 
 3 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index 6e5867c57305..c72cbbd2d40c 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -28,6 +28,19 @@ config VIDEO_IVTV
  To compile this driver as a module, choose M here: the
  module will be called ivtv.
 
+config VIDEO_IVTV_DEPRECATED_IOCTLS
+   bool "enable the DVB ioctls abuse on ivtv driver"
+   depends on VIDEO_IVTV
+   default n
+   ---help---
+ Enable the usage of the a DVB set of ioctls that were abused by
+ IVTV driver for a while.
+
+ Those ioctls were not needed for a long time, as IVTV implements
+ the proper V4L2 ioctls since kernel 3.3.
+
+ If unsure, say N.
+
 config VIDEO_IVTV_ALSA
tristate "Conexant cx23415/cx23416 ALSA interface for PCM audio capture"
depends on VIDEO_IVTV && SND
diff --git a/drivers/media/pci/ivtv/ivtv-driver.h 
b/drivers/media/pci/ivtv/ivtv-driver.h
index b2b0fa27b1a7..ab1fb48d5629 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.h
+++ b/drivers/media/pci/ivtv/ivtv-driver.h
@@ -42,8 +42,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.c 
b/drivers/media/pci/ivtv/ivtv-ioctl.c
index 2dc4b20f3ac0..f956188f7f19 100644
--- a/drivers/media/pci/ivtv/ivtv-ioctl.c
+++ b/drivers/media/pci/ivtv/ivtv-ioctl.c
@@ -35,7 +35,10 @@
 #include 
 #include 
 #include 
+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
 #include 
+#include 
+#endif
 
 u16 ivtv_service2vbi(int type)
 {
@@ -1620,13 +1623,23 @@ static int ivtv_try_decoder_cmd(struct file *file, void 
*fh, struct v4l2_decoder
return ivtv_video_command(itv, id, dec, true);
 }
 
+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
+static __inline__ void warn_deprecated_ioctl(const char *name)
+{
+   pr_warn_once("warning: the %s ioctl is deprecated. Don't use it, as it 
will be removed soon\n",
+name);
+}
+#endif
+
 static int ivtv_decoder_ioctls(struct file *filp, unsigned int cmd, void *arg)
 {
struct ivtv_open_id *id = fh2id(filp->private_data);
struct ivtv *itv = id->itv;
+   struct ivtv_stream *s = >streams[id->type];
+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
int nonblocking = filp->f_flags & O_NONBLOCK;
-   struct ivtv_stream *s = >streams[id->type];
unsigned long iarg = (unsigned long)arg;
+#endif
 
switch (cmd) {
case IVTV_IOC_DMA_FRAME: {
@@ -1658,12 +1671,12 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
if (!(itv->v4l2_cap & V4L2_CAP_VIDEO_OUTPUT))
return -EINVAL;
return ivtv_passthrough_mode(itv, *(int *)arg != 0);
-
+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
case VIDEO_GET_PTS: {
s64 *pts = arg;
s64 frame;
 
-   IVTV_DEBUG_IOCTL("VIDEO_GET_PTS\n");
+   warn_deprecated_ioctl("VIDEO_GET_PTS");
if (s->type < IVTV_DEC_STREAM_TYPE_MPG) {
*pts = s->dma_pts;
break;
@@ -1677,7 +1690,7 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
s64 *frame = arg;
s64 pts;
 
-   IVTV_DEBUG_IOCTL("VIDEO_GET_FRAME_COUNT\n");
+   warn_deprecated_ioctl("VIDEO_GET_FRAME_COUNT");
if (s->type < IVTV_DEC_STREAM_TYPE_MPG) {
*frame = 0;
break;
@@ -1690,7 +1703,7 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
case VIDEO_PLAY: {
struct v4l2_decoder_cmd dc;
 
-   IVTV_DEBUG_IOCTL("VIDEO_PLAY\n");
+   warn_deprecated_ioctl("VIDEO_PLAY");
memset(, 0, sizeof(dc));
dc.cmd = V4L2_DEC_CMD_START;
return ivtv_video_command(itv, id, , 0);
@@ -1699,7 +1712,7 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
case VIDEO_STOP: {
struct 

[PATCH v2 1/3] [media] ivtv: prepare to convert to pr_foo()

[Mauro Carvalho Chehab]

Move the pr_fmt() macro to ivtv_driver.h and ensure that it
will be the first file to be included on all ivtv files.

While here, put the includes inside ivtv-driver.h on
alphabetic order.

Acked-by: Hans Verkuil 
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/ivtv/ivtv-alsa-main.c  | 15 +++-
 drivers/media/pci/ivtv/ivtv-alsa-mixer.c | 13 --
 drivers/media/pci/ivtv/ivtv-alsa-pcm.c   | 15 +---
 drivers/media/pci/ivtv/ivtv-driver.h | 41 +---
 drivers/media/pci/ivtv/ivtv-mailbox.c|  4 ++--
 drivers/media/pci/ivtv/ivtvfb.c  | 19 ++-
 6 files changed, 44 insertions(+), 63 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-alsa-main.c 
b/drivers/media/pci/ivtv/ivtv-alsa-main.c
index 374f45f81ab3..67ab73ef2bca 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-main.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-main.c
@@ -22,24 +22,15 @@
  *  02111-1307  USA
  */
 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-
-#include 
-#include 
-
 #include "ivtv-driver.h"
 #include "ivtv-version.h"
 #include "ivtv-alsa.h"
 #include "ivtv-alsa-mixer.h"
 #include "ivtv-alsa-pcm.h"
 
+#include 
+#include 
+
 int ivtv_alsa_debug;
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;
 
diff --git a/drivers/media/pci/ivtv/ivtv-alsa-mixer.c 
b/drivers/media/pci/ivtv/ivtv-alsa-mixer.c
index 79b24bde4a39..a5a92c856d8c 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-mixer.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-mixer.c
@@ -20,21 +20,16 @@
  *  02111-1307  USA
  */
 
-#include 
-#include 
-#include 
-#include 
+#include "ivtv-alsa.h"
+#include "ivtv-alsa-mixer.h"
+#include "ivtv-driver.h"
+
 #include 
 
-#include 
-
 #include 
 #include 
 #include 
 
-#include "ivtv-alsa.h"
-#include "ivtv-driver.h"
-
 /*
  * Note the cx25840-core volume scale is funny, due to the alignment of the
  * scale with another chip's range:
diff --git a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c 
b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
index a26f9800eca3..912a85f64e0e 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
@@ -23,15 +23,6 @@
  *  02111-1307  USA
  */
 
-#include 
-#include 
-#include 
-
-#include 
-
-#include 
-#include 
-
 #include "ivtv-driver.h"
 #include "ivtv-queue.h"
 #include "ivtv-streams.h"
@@ -39,6 +30,12 @@
 #include "ivtv-alsa.h"
 #include "ivtv-alsa-pcm.h"
 
+#include 
+
+#include 
+#include 
+
+
 static unsigned int pcm_debug;
 module_param(pcm_debug, int, 0644);
 MODULE_PARM_DESC(pcm_debug, "enable debug messages for pcm");
diff --git a/drivers/media/pci/ivtv/ivtv-driver.h 
b/drivers/media/pci/ivtv/ivtv-driver.h
index 10cba305dbd2..b2b0fa27b1a7 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.h
+++ b/drivers/media/pci/ivtv/ivtv-driver.h
@@ -22,6 +22,8 @@
 #ifndef IVTV_DRIVER_H
 #define IVTV_DRIVER_H
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 /* Internal header for ivtv project:
  * Driver for the cx23415/6 chip.
  * Author: Kevin Thayer (nufan_wfk at yahoo.com)
@@ -36,38 +38,39 @@
  *using information provided by Jiun-Kuei Jung @ AVerMedia.
  */
 
-#include 
-#include 
+#include 
+#include 
 #include 
-#include 
+#include 
+#include 
+#include 
 #include 
-#include 
-#include 
-#include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
-#include 
+#include 
+#include 
 #include 
+#include 
 #include 
-#include 
-#include 
+#include 
 #include 
-#include 
-#include 
+#include 
+#include 
 
-#include 
-#include 
+#include 
+#include 
+#include 
 #include 
-#include 
 #include 
 #include 
 #include 
-#include 
-#include 
-#include 
-
-#include 
+#include 
 
 /* Memory layout */
 #define IVTV_ENCODER_OFFSET0x
diff --git a/drivers/media/pci/ivtv/ivtv-mailbox.c 
b/drivers/media/pci/ivtv/ivtv-mailbox.c
index e3ce96763785..9a2506a5edbe 100644
--- a/drivers/media/pci/ivtv/ivtv-mailbox.c
+++ b/drivers/media/pci/ivtv/ivtv-mailbox.c
@@ -19,11 +19,11 @@
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#include 
-
 #include "ivtv-driver.h"
 #include "ivtv-mailbox.h"
 
+#include 
+
 /* Firmware mailbox flags*/
 #define IVTV_MBOX_FIRMWARE_DONE 0x0004
 #define IVTV_MBOX_DRIVER_DONE   0x0002
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 612a8402cf4d..b59b60d605eb 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -38,18 +38,6 @@
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#ifdef CONFIG_X86_64
-#include 
-#endif
-
 #include "ivtv-driver.h"
 #include "ivtv-cards.h"
 #include "ivtv-i2c.h"
@@ -57,6 +45,13 @@
 #include "ivtv-mailbox.h"
 #include "ivtv-firmware.h"
 
+#include 
+#include 
+
+#ifdef CONFIG_X86_64

[PATCH v2 2/3] [media] ivtv: use pr_foo() instead of calling printk() directly

[Mauro Carvalho Chehab]

pr_foo() provides a convenient way for printk's, enforcing
that they'll all prepend the error message with the driver's
name.

Use it inside ivtv.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/ivtv/ivtv-alsa-main.c | 11 +--
 drivers/media/pci/ivtv/ivtv-driver.c| 12 ++--
 drivers/media/pci/ivtv/ivtvfb.c |  6 +++---
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-alsa-main.c 
b/drivers/media/pci/ivtv/ivtv-alsa-main.c
index 67ab73ef2bca..f7a036844f02 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-main.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-main.c
@@ -34,10 +34,11 @@
 int ivtv_alsa_debug;
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;
 
-#define IVTV_DEBUG_ALSA_INFO(fmt, arg...) \
+#define IVTV_DEBUG_ALSA_INFO(__fmt, __arg...) \
do { \
if (ivtv_alsa_debug & 2) \
-   pr_info("%s: " fmt, "ivtv-alsa", ## arg); \
+   printk(KERN_INFO pr_fmt("%s: alsa:" __fmt), \
+  __func__, ##__arg);  \
} while (0)
 
 module_param_named(debug, ivtv_alsa_debug, int, 0644);
@@ -226,8 +227,7 @@ static int ivtv_alsa_load(struct ivtv *itv)
 
s = >streams[IVTV_ENC_STREAM_TYPE_PCM];
if (s->vdev.v4l2_dev == NULL) {
-   IVTV_DEBUG_ALSA_INFO("%s: PCM stream for card is disabled - 
skipping\n",
-__func__);
+   IVTV_DEBUG_ALSA_INFO("PCM stream for card is disabled - 
skipping\n");
return 0;
}
 
@@ -241,8 +241,7 @@ static int ivtv_alsa_load(struct ivtv *itv)
IVTV_ALSA_ERR("%s: failed to create struct snd_ivtv_card\n",
  __func__);
} else {
-   IVTV_DEBUG_ALSA_INFO("%s: created ivtv ALSA interface instance 
\n",
-__func__);
+   IVTV_DEBUG_ALSA_INFO("created ivtv ALSA interface instance\n");
}
return 0;
 }
diff --git a/drivers/media/pci/ivtv/ivtv-driver.c 
b/drivers/media/pci/ivtv/ivtv-driver.c
index 0a3b80a4bd69..ab2ae53618e8 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.c
+++ b/drivers/media/pci/ivtv/ivtv-driver.c
@@ -1452,7 +1452,7 @@ static void ivtv_remove(struct pci_dev *pdev)
for (i = 0; i < IVTV_VBI_FRAMES; i++)
kfree(itv->vbi.sliced_mpeg_data[i]);
 
-   printk(KERN_INFO "ivtv: Removed %s\n", itv->card_name);
+   pr_info("Removed %s\n", itv->card_name);
 
v4l2_device_unregister(>v4l2_dev);
kfree(itv);
@@ -1468,25 +1468,25 @@ static struct pci_driver ivtv_pci_driver = {
 
 static int __init module_start(void)
 {
-   printk(KERN_INFO "ivtv: Start initialization, version %s\n", 
IVTV_VERSION);
+   pr_info("Start initialization, version %s\n", IVTV_VERSION);
 
/* Validate parameters */
if (ivtv_first_minor < 0 || ivtv_first_minor >= IVTV_MAX_CARDS) {
-   printk(KERN_ERR "ivtv: Exiting, ivtv_first_minor must be 
between 0 and %d\n",
+   pr_err("Exiting, ivtv_first_minor must be between 0 and %d\n",
 IVTV_MAX_CARDS - 1);
return -1;
}
 
if (ivtv_debug < 0 || ivtv_debug > 2047) {
ivtv_debug = 0;
-   printk(KERN_INFO "ivtv: Debug value must be >= 0 and <= 
2047\n");
+   pr_info("Debug value must be >= 0 and <= 2047\n");
}
 
if (pci_register_driver(_pci_driver)) {
-   printk(KERN_ERR "ivtv: Error detecting PCI card\n");
+   pr_err("Error detecting PCI card\n");
return -ENODEV;
}
-   printk(KERN_INFO "ivtv: End initialization\n");
+   pr_info("End initialization\n");
return 0;
 }
 
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index b59b60d605eb..621b2f613d81 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -1270,7 +1270,7 @@ static int __init ivtvfb_init(void)
 
 
if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
-   printk(KERN_ERR "ivtvfb:  ivtvfb_card_id parameter is out of 
range (valid range: -1 - %d)\n",
+   pr_err("ivtvfb_card_id parameter is out of range (valid range: 
-1 - %d)\n",
 IVTV_MAX_CARDS - 1);
return -EINVAL;
}
@@ -1279,7 +1279,7 @@ static int __init ivtvfb_init(void)
err = driver_for_each_device(drv, NULL, , 
ivtvfb_callback_init);
(void)err;  /* suppress compiler warning */
if (!registered) {
-   printk(KERN_ERR "ivtvfb:  no cards found\n");
+   pr_err("no cards found\n");
return -ENODEV;
}
return 0;
@@ -1290,7 +1290,7 @@ static void ivtvfb_cleanup(void)
struct device_driver *drv;
int err;
 
-   printk(KERN_INFO "ivtvfb:  Unloading 

Re: [PATCH 3/3] [media] ivtv: mark DVB "borrowed" ioctls as deprecated

[Hans Verkuil]

On 11/24/16 12:01, Mauro Carvalho Chehab wrote:

changeset da8ec560e3b4 ("[media] ivtv: implement new decoder command
ioctls") implemented proper support for mpeg audio and video control
at V4L2 API. Since then, the usage of the the DVB APIs is deprecated.

However, we never actually marked it as deprecated nor provided any
way to disable it. Let's do it now.

This patch prepares for the removal of this bad usage on a couple
of Kernel versions.

Fixes: da8ec560e3b4 ("[media] ivtv: implement new decoder command ioctls")
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/ivtv/Kconfig   | 13 ++
 drivers/media/pci/ivtv/ivtv-driver.h |  2 --
 drivers/media/pci/ivtv/ivtv-ioctl.c  | 49 
 3 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index 6e5867c57305..c72cbbd2d40c 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -28,6 +28,19 @@ config VIDEO_IVTV
  To compile this driver as a module, choose M here: the
  module will be called ivtv.

+config VIDEO_IVTV_DEPRECATED_IOCTLS
+   bool "enable the DVB ioctls abuse on ivtv driver"
+   depends on VIDEO_IVTV
+   default n
+   ---help---
+ Enable the usage of the a DVB set of ioctls that were abused by
+ IVTV driver for a while.
+
+ Those ioctls were not needed for a long time, as IVTV implements
+ the proper V4L2 ioctls since kernel 3.3.
+
+ If unsure, say N.
+
 config VIDEO_IVTV_ALSA
tristate "Conexant cx23415/cx23416 ALSA interface for PCM audio capture"
depends on VIDEO_IVTV && SND
diff --git a/drivers/media/pci/ivtv/ivtv-driver.h 
b/drivers/media/pci/ivtv/ivtv-driver.h
index b2b0fa27b1a7..ab1fb48d5629 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.h
+++ b/drivers/media/pci/ivtv/ivtv-driver.h
@@ -42,8 +42,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.c 
b/drivers/media/pci/ivtv/ivtv-ioctl.c
index 2dc4b20f3ac0..e5e53a09537e 100644
--- a/drivers/media/pci/ivtv/ivtv-ioctl.c
+++ b/drivers/media/pci/ivtv/ivtv-ioctl.c
@@ -35,7 +35,10 @@
 #include 
 #include 
 #include 
+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
 #include 
+#include 
+#endif

 u16 ivtv_service2vbi(int type)
 {
@@ -1620,13 +1623,23 @@ static int ivtv_try_decoder_cmd(struct file *file, void 
*fh, struct v4l2_decoder
return ivtv_video_command(itv, id, dec, true);
 }

+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
+static __inline__ void warn_deprecated_ioctl(char *name)


const char *


+{
+   pr_warn_once("warning: the %s ioctl is deprecated. Don't use it, as it will 
be removed soon\n",
+name);
+}
+#endif
+
 static int ivtv_decoder_ioctls(struct file *filp, unsigned int cmd, void *arg)
 {
struct ivtv_open_id *id = fh2id(filp->private_data);
struct ivtv *itv = id->itv;
+   struct ivtv_stream *s = >streams[id->type];
+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
int nonblocking = filp->f_flags & O_NONBLOCK;
-   struct ivtv_stream *s = >streams[id->type];
unsigned long iarg = (unsigned long)arg;
+#endif

switch (cmd) {
case IVTV_IOC_DMA_FRAME: {
@@ -1658,12 +1671,12 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
if (!(itv->v4l2_cap & V4L2_CAP_VIDEO_OUTPUT))
return -EINVAL;
return ivtv_passthrough_mode(itv, *(int *)arg != 0);
-
+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
case VIDEO_GET_PTS: {
s64 *pts = arg;
s64 frame;

-   IVTV_DEBUG_IOCTL("VIDEO_GET_PTS\n");
+   warn_deprecated_ioctl("VIDEO_GET_PTS");
if (s->type < IVTV_DEC_STREAM_TYPE_MPG) {
*pts = s->dma_pts;
break;
@@ -1677,7 +1690,7 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
s64 *frame = arg;
s64 pts;

-   IVTV_DEBUG_IOCTL("VIDEO_GET_FRAME_COUNT\n");
+   warn_deprecated_ioctl("VIDEO_GET_FRAME_COUNT");
if (s->type < IVTV_DEC_STREAM_TYPE_MPG) {
*frame = 0;
break;
@@ -1690,7 +1703,7 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
case VIDEO_PLAY: {
struct v4l2_decoder_cmd dc;

-   IVTV_DEBUG_IOCTL("VIDEO_PLAY\n");
+   warn_deprecated_ioctl("VIDEO_PLAY");
memset(, 0, sizeof(dc));
dc.cmd = V4L2_DEC_CMD_START;
return ivtv_video_command(itv, id, , 0);
@@ -1699,7 +1712,7 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
case VIDEO_STOP: {
struct 

Re: [PATCH 1/3] [media] ivtv: prepare to convert to pr_foo()

[Hans Verkuil]

On 11/24/16 12:01, Mauro Carvalho Chehab wrote:

Move the pr_fmt() macro to ivtv_driver.h and ensure that it
will be the first file to be included on all ivtv files.

While here, put the includes inside ivtv-driver.h on
alphabetic order.

Signed-off-by: Mauro Carvalho Chehab 


Acked-by: Hans Verkuil 

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] [media] ivtv: use pr_foo() instead of calling printk() directly

[Hans Verkuil]

On 11/24/16 12:01, Mauro Carvalho Chehab wrote:

pr_foo() provides a convenient way for printk's, enforcing
that they'll all prepend the error message with the driver's
name.

Use it inside ivtv.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/ivtv/ivtv-alsa-main.c | 11 +--
 drivers/media/pci/ivtv/ivtv-driver.c| 12 ++--
 drivers/media/pci/ivtv/ivtvfb.c |  6 +++---
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-alsa-main.c 
b/drivers/media/pci/ivtv/ivtv-alsa-main.c
index 67ab73ef2bca..c66a8e991eb2 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-main.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-main.c
@@ -34,10 +34,11 @@
 int ivtv_alsa_debug;
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;

-#define IVTV_DEBUG_ALSA_INFO(fmt, arg...) \
+#define IVTV_DEBUG_ALSA_INFO(__fmt, __arg...) \
do { \
if (ivtv_alsa_debug & 2) \
-   pr_info("%s: " fmt, "ivtv-alsa", ## arg); \
+   printk(KERN_DEBUG pr_fmt("%s: alsa:" __fmt),  \
+  __func__, ##__arg);  \
} while (0)


Please keep using KERN_INFO.



 module_param_named(debug, ivtv_alsa_debug, int, 0644);
@@ -226,8 +227,7 @@ static int ivtv_alsa_load(struct ivtv *itv)

s = >streams[IVTV_ENC_STREAM_TYPE_PCM];
if (s->vdev.v4l2_dev == NULL) {
-   IVTV_DEBUG_ALSA_INFO("%s: PCM stream for card is disabled - 
skipping\n",
-__func__);
+   IVTV_DEBUG_ALSA_INFO("PCM stream for card is disabled - 
skipping\n");
return 0;
}

@@ -241,8 +241,7 @@ static int ivtv_alsa_load(struct ivtv *itv)
IVTV_ALSA_ERR("%s: failed to create struct snd_ivtv_card\n",
  __func__);
} else {
-   IVTV_DEBUG_ALSA_INFO("%s: created ivtv ALSA interface instance 
\n",
-__func__);
+   IVTV_DEBUG_ALSA_INFO("created ivtv ALSA interface instance\n");
}
return 0;
 }
diff --git a/drivers/media/pci/ivtv/ivtv-driver.c 
b/drivers/media/pci/ivtv/ivtv-driver.c
index 0a3b80a4bd69..ab2ae53618e8 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.c
+++ b/drivers/media/pci/ivtv/ivtv-driver.c
@@ -1452,7 +1452,7 @@ static void ivtv_remove(struct pci_dev *pdev)
for (i = 0; i < IVTV_VBI_FRAMES; i++)
kfree(itv->vbi.sliced_mpeg_data[i]);

-   printk(KERN_INFO "ivtv: Removed %s\n", itv->card_name);
+   pr_info("Removed %s\n", itv->card_name);

v4l2_device_unregister(>v4l2_dev);
kfree(itv);
@@ -1468,25 +1468,25 @@ static struct pci_driver ivtv_pci_driver = {

 static int __init module_start(void)
 {
-   printk(KERN_INFO "ivtv: Start initialization, version %s\n", 
IVTV_VERSION);
+   pr_info("Start initialization, version %s\n", IVTV_VERSION);

/* Validate parameters */
if (ivtv_first_minor < 0 || ivtv_first_minor >= IVTV_MAX_CARDS) {
-   printk(KERN_ERR "ivtv: Exiting, ivtv_first_minor must be between 0 
and %d\n",
+   pr_err("Exiting, ivtv_first_minor must be between 0 and %d\n",
 IVTV_MAX_CARDS - 1);
return -1;
}

if (ivtv_debug < 0 || ivtv_debug > 2047) {
ivtv_debug = 0;
-   printk(KERN_INFO "ivtv: Debug value must be >= 0 and <= 
2047\n");
+   pr_info("Debug value must be >= 0 and <= 2047\n");
}

if (pci_register_driver(_pci_driver)) {
-   printk(KERN_ERR "ivtv: Error detecting PCI card\n");
+   pr_err("Error detecting PCI card\n");
return -ENODEV;
}
-   printk(KERN_INFO "ivtv: End initialization\n");
+   pr_info("End initialization\n");
return 0;
 }

diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index b59b60d605eb..d8a152e45ce2 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -1270,7 +1270,7 @@ static int __init ivtvfb_init(void)


if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
-   printk(KERN_ERR "ivtvfb:  ivtvfb_card_id parameter is out of range 
(valid range: -1 - %d)\n",
+   pr_err(" ivtvfb_card_id parameter is out of range (valid range: -1 - 
%d)\n",
 IVTV_MAX_CARDS - 1);
return -EINVAL;
}
@@ -1279,7 +1279,7 @@ static int __init ivtvfb_init(void)
err = driver_for_each_device(drv, NULL, , 
ivtvfb_callback_init);
(void)err;  /* suppress compiler warning */
if (!registered) {
-   printk(KERN_ERR "ivtvfb:  no cards found\n");
+   pr_err(" no cards found\n");


Why add the first space? It's in several pr_foo conversion in this patch.
pr_fmt already adds a space:

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

[PATCH 3/3] [media] ivtv: mark DVB "borrowed" ioctls as deprecated

[Mauro Carvalho Chehab]

changeset da8ec560e3b4 ("[media] ivtv: implement new decoder command
ioctls") implemented proper support for mpeg audio and video control
at V4L2 API. Since then, the usage of the the DVB APIs is deprecated.

However, we never actually marked it as deprecated nor provided any
way to disable it. Let's do it now.

This patch prepares for the removal of this bad usage on a couple
of Kernel versions.

Fixes: da8ec560e3b4 ("[media] ivtv: implement new decoder command ioctls")
Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/ivtv/Kconfig   | 13 ++
 drivers/media/pci/ivtv/ivtv-driver.h |  2 --
 drivers/media/pci/ivtv/ivtv-ioctl.c  | 49 
 3 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index 6e5867c57305..c72cbbd2d40c 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -28,6 +28,19 @@ config VIDEO_IVTV
  To compile this driver as a module, choose M here: the
  module will be called ivtv.
 
+config VIDEO_IVTV_DEPRECATED_IOCTLS
+   bool "enable the DVB ioctls abuse on ivtv driver"
+   depends on VIDEO_IVTV
+   default n
+   ---help---
+ Enable the usage of the a DVB set of ioctls that were abused by
+ IVTV driver for a while.
+
+ Those ioctls were not needed for a long time, as IVTV implements
+ the proper V4L2 ioctls since kernel 3.3.
+
+ If unsure, say N.
+
 config VIDEO_IVTV_ALSA
tristate "Conexant cx23415/cx23416 ALSA interface for PCM audio capture"
depends on VIDEO_IVTV && SND
diff --git a/drivers/media/pci/ivtv/ivtv-driver.h 
b/drivers/media/pci/ivtv/ivtv-driver.h
index b2b0fa27b1a7..ab1fb48d5629 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.h
+++ b/drivers/media/pci/ivtv/ivtv-driver.h
@@ -42,8 +42,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/media/pci/ivtv/ivtv-ioctl.c 
b/drivers/media/pci/ivtv/ivtv-ioctl.c
index 2dc4b20f3ac0..e5e53a09537e 100644
--- a/drivers/media/pci/ivtv/ivtv-ioctl.c
+++ b/drivers/media/pci/ivtv/ivtv-ioctl.c
@@ -35,7 +35,10 @@
 #include 
 #include 
 #include 
+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
 #include 
+#include 
+#endif
 
 u16 ivtv_service2vbi(int type)
 {
@@ -1620,13 +1623,23 @@ static int ivtv_try_decoder_cmd(struct file *file, void 
*fh, struct v4l2_decoder
return ivtv_video_command(itv, id, dec, true);
 }
 
+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
+static __inline__ void warn_deprecated_ioctl(char *name)
+{
+   pr_warn_once("warning: the %s ioctl is deprecated. Don't use it, as it 
will be removed soon\n",
+name);
+}
+#endif
+
 static int ivtv_decoder_ioctls(struct file *filp, unsigned int cmd, void *arg)
 {
struct ivtv_open_id *id = fh2id(filp->private_data);
struct ivtv *itv = id->itv;
+   struct ivtv_stream *s = >streams[id->type];
+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
int nonblocking = filp->f_flags & O_NONBLOCK;
-   struct ivtv_stream *s = >streams[id->type];
unsigned long iarg = (unsigned long)arg;
+#endif
 
switch (cmd) {
case IVTV_IOC_DMA_FRAME: {
@@ -1658,12 +1671,12 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
if (!(itv->v4l2_cap & V4L2_CAP_VIDEO_OUTPUT))
return -EINVAL;
return ivtv_passthrough_mode(itv, *(int *)arg != 0);
-
+#ifdef CONFIG_VIDEO_IVTV_DEPRECATED_IOCTLS
case VIDEO_GET_PTS: {
s64 *pts = arg;
s64 frame;
 
-   IVTV_DEBUG_IOCTL("VIDEO_GET_PTS\n");
+   warn_deprecated_ioctl("VIDEO_GET_PTS");
if (s->type < IVTV_DEC_STREAM_TYPE_MPG) {
*pts = s->dma_pts;
break;
@@ -1677,7 +1690,7 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
s64 *frame = arg;
s64 pts;
 
-   IVTV_DEBUG_IOCTL("VIDEO_GET_FRAME_COUNT\n");
+   warn_deprecated_ioctl("VIDEO_GET_FRAME_COUNT");
if (s->type < IVTV_DEC_STREAM_TYPE_MPG) {
*frame = 0;
break;
@@ -1690,7 +1703,7 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
case VIDEO_PLAY: {
struct v4l2_decoder_cmd dc;
 
-   IVTV_DEBUG_IOCTL("VIDEO_PLAY\n");
+   warn_deprecated_ioctl("VIDEO_PLAY");
memset(, 0, sizeof(dc));
dc.cmd = V4L2_DEC_CMD_START;
return ivtv_video_command(itv, id, , 0);
@@ -1699,7 +1712,7 @@ static int ivtv_decoder_ioctls(struct file *filp, 
unsigned int cmd, void *arg)
case VIDEO_STOP: {
struct v4l2_decoder_cmd dc;
 
-   

[PATCH 2/3] [media] ivtv: use pr_foo() instead of calling printk() directly

[Mauro Carvalho Chehab]

pr_foo() provides a convenient way for printk's, enforcing
that they'll all prepend the error message with the driver's
name.

Use it inside ivtv.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/ivtv/ivtv-alsa-main.c | 11 +--
 drivers/media/pci/ivtv/ivtv-driver.c| 12 ++--
 drivers/media/pci/ivtv/ivtvfb.c |  6 +++---
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-alsa-main.c 
b/drivers/media/pci/ivtv/ivtv-alsa-main.c
index 67ab73ef2bca..c66a8e991eb2 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-main.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-main.c
@@ -34,10 +34,11 @@
 int ivtv_alsa_debug;
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;
 
-#define IVTV_DEBUG_ALSA_INFO(fmt, arg...) \
+#define IVTV_DEBUG_ALSA_INFO(__fmt, __arg...) \
do { \
if (ivtv_alsa_debug & 2) \
-   pr_info("%s: " fmt, "ivtv-alsa", ## arg); \
+   printk(KERN_DEBUG pr_fmt("%s: alsa:" __fmt),\
+  __func__, ##__arg);  \
} while (0)
 
 module_param_named(debug, ivtv_alsa_debug, int, 0644);
@@ -226,8 +227,7 @@ static int ivtv_alsa_load(struct ivtv *itv)
 
s = >streams[IVTV_ENC_STREAM_TYPE_PCM];
if (s->vdev.v4l2_dev == NULL) {
-   IVTV_DEBUG_ALSA_INFO("%s: PCM stream for card is disabled - 
skipping\n",
-__func__);
+   IVTV_DEBUG_ALSA_INFO("PCM stream for card is disabled - 
skipping\n");
return 0;
}
 
@@ -241,8 +241,7 @@ static int ivtv_alsa_load(struct ivtv *itv)
IVTV_ALSA_ERR("%s: failed to create struct snd_ivtv_card\n",
  __func__);
} else {
-   IVTV_DEBUG_ALSA_INFO("%s: created ivtv ALSA interface instance 
\n",
-__func__);
+   IVTV_DEBUG_ALSA_INFO("created ivtv ALSA interface instance\n");
}
return 0;
 }
diff --git a/drivers/media/pci/ivtv/ivtv-driver.c 
b/drivers/media/pci/ivtv/ivtv-driver.c
index 0a3b80a4bd69..ab2ae53618e8 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.c
+++ b/drivers/media/pci/ivtv/ivtv-driver.c
@@ -1452,7 +1452,7 @@ static void ivtv_remove(struct pci_dev *pdev)
for (i = 0; i < IVTV_VBI_FRAMES; i++)
kfree(itv->vbi.sliced_mpeg_data[i]);
 
-   printk(KERN_INFO "ivtv: Removed %s\n", itv->card_name);
+   pr_info("Removed %s\n", itv->card_name);
 
v4l2_device_unregister(>v4l2_dev);
kfree(itv);
@@ -1468,25 +1468,25 @@ static struct pci_driver ivtv_pci_driver = {
 
 static int __init module_start(void)
 {
-   printk(KERN_INFO "ivtv: Start initialization, version %s\n", 
IVTV_VERSION);
+   pr_info("Start initialization, version %s\n", IVTV_VERSION);
 
/* Validate parameters */
if (ivtv_first_minor < 0 || ivtv_first_minor >= IVTV_MAX_CARDS) {
-   printk(KERN_ERR "ivtv: Exiting, ivtv_first_minor must be 
between 0 and %d\n",
+   pr_err("Exiting, ivtv_first_minor must be between 0 and %d\n",
 IVTV_MAX_CARDS - 1);
return -1;
}
 
if (ivtv_debug < 0 || ivtv_debug > 2047) {
ivtv_debug = 0;
-   printk(KERN_INFO "ivtv: Debug value must be >= 0 and <= 
2047\n");
+   pr_info("Debug value must be >= 0 and <= 2047\n");
}
 
if (pci_register_driver(_pci_driver)) {
-   printk(KERN_ERR "ivtv: Error detecting PCI card\n");
+   pr_err("Error detecting PCI card\n");
return -ENODEV;
}
-   printk(KERN_INFO "ivtv: End initialization\n");
+   pr_info("End initialization\n");
return 0;
 }
 
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index b59b60d605eb..d8a152e45ce2 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -1270,7 +1270,7 @@ static int __init ivtvfb_init(void)
 
 
if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
-   printk(KERN_ERR "ivtvfb:  ivtvfb_card_id parameter is out of 
range (valid range: -1 - %d)\n",
+   pr_err(" ivtvfb_card_id parameter is out of range (valid range: 
-1 - %d)\n",
 IVTV_MAX_CARDS - 1);
return -EINVAL;
}
@@ -1279,7 +1279,7 @@ static int __init ivtvfb_init(void)
err = driver_for_each_device(drv, NULL, , 
ivtvfb_callback_init);
(void)err;  /* suppress compiler warning */
if (!registered) {
-   printk(KERN_ERR "ivtvfb:  no cards found\n");
+   pr_err(" no cards found\n");
return -ENODEV;
}
return 0;
@@ -1290,7 +1290,7 @@ static void ivtvfb_cleanup(void)
struct device_driver *drv;
int err;
 
-   printk(KERN_INFO "ivtvfb:  Unloading 

[PATCH 1/3] [media] ivtv: prepare to convert to pr_foo()

[Mauro Carvalho Chehab]

Move the pr_fmt() macro to ivtv_driver.h and ensure that it
will be the first file to be included on all ivtv files.

While here, put the includes inside ivtv-driver.h on
alphabetic order.

Signed-off-by: Mauro Carvalho Chehab 
---
 drivers/media/pci/ivtv/ivtv-alsa-main.c  | 15 +++-
 drivers/media/pci/ivtv/ivtv-alsa-mixer.c | 13 --
 drivers/media/pci/ivtv/ivtv-alsa-pcm.c   | 15 +---
 drivers/media/pci/ivtv/ivtv-driver.h | 41 +---
 drivers/media/pci/ivtv/ivtv-mailbox.c|  4 ++--
 drivers/media/pci/ivtv/ivtvfb.c  | 19 ++-
 6 files changed, 44 insertions(+), 63 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-alsa-main.c 
b/drivers/media/pci/ivtv/ivtv-alsa-main.c
index 374f45f81ab3..67ab73ef2bca 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-main.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-main.c
@@ -22,24 +22,15 @@
  *  02111-1307  USA
  */
 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include 
-
-#include 
-#include 
-
 #include "ivtv-driver.h"
 #include "ivtv-version.h"
 #include "ivtv-alsa.h"
 #include "ivtv-alsa-mixer.h"
 #include "ivtv-alsa-pcm.h"
 
+#include 
+#include 
+
 int ivtv_alsa_debug;
 static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;
 
diff --git a/drivers/media/pci/ivtv/ivtv-alsa-mixer.c 
b/drivers/media/pci/ivtv/ivtv-alsa-mixer.c
index 79b24bde4a39..a5a92c856d8c 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-mixer.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-mixer.c
@@ -20,21 +20,16 @@
  *  02111-1307  USA
  */
 
-#include 
-#include 
-#include 
-#include 
+#include "ivtv-alsa.h"
+#include "ivtv-alsa-mixer.h"
+#include "ivtv-driver.h"
+
 #include 
 
-#include 
-
 #include 
 #include 
 #include 
 
-#include "ivtv-alsa.h"
-#include "ivtv-driver.h"
-
 /*
  * Note the cx25840-core volume scale is funny, due to the alignment of the
  * scale with another chip's range:
diff --git a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c 
b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
index a26f9800eca3..912a85f64e0e 100644
--- a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
+++ b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c
@@ -23,15 +23,6 @@
  *  02111-1307  USA
  */
 
-#include 
-#include 
-#include 
-
-#include 
-
-#include 
-#include 
-
 #include "ivtv-driver.h"
 #include "ivtv-queue.h"
 #include "ivtv-streams.h"
@@ -39,6 +30,12 @@
 #include "ivtv-alsa.h"
 #include "ivtv-alsa-pcm.h"
 
+#include 
+
+#include 
+#include 
+
+
 static unsigned int pcm_debug;
 module_param(pcm_debug, int, 0644);
 MODULE_PARM_DESC(pcm_debug, "enable debug messages for pcm");
diff --git a/drivers/media/pci/ivtv/ivtv-driver.h 
b/drivers/media/pci/ivtv/ivtv-driver.h
index 10cba305dbd2..b2b0fa27b1a7 100644
--- a/drivers/media/pci/ivtv/ivtv-driver.h
+++ b/drivers/media/pci/ivtv/ivtv-driver.h
@@ -22,6 +22,8 @@
 #ifndef IVTV_DRIVER_H
 #define IVTV_DRIVER_H
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 /* Internal header for ivtv project:
  * Driver for the cx23415/6 chip.
  * Author: Kevin Thayer (nufan_wfk at yahoo.com)
@@ -36,38 +38,39 @@
  *using information provided by Jiun-Kuei Jung @ AVerMedia.
  */
 
-#include 
-#include 
+#include 
+#include 
 #include 
-#include 
+#include 
+#include 
+#include 
 #include 
-#include 
-#include 
-#include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
 #include 
-#include 
+#include 
+#include 
 #include 
+#include 
 #include 
-#include 
-#include 
+#include 
 #include 
-#include 
-#include 
+#include 
+#include 
 
-#include 
-#include 
+#include 
+#include 
+#include 
 #include 
-#include 
 #include 
 #include 
 #include 
-#include 
-#include 
-#include 
-
-#include 
+#include 
 
 /* Memory layout */
 #define IVTV_ENCODER_OFFSET0x
diff --git a/drivers/media/pci/ivtv/ivtv-mailbox.c 
b/drivers/media/pci/ivtv/ivtv-mailbox.c
index e3ce96763785..9a2506a5edbe 100644
--- a/drivers/media/pci/ivtv/ivtv-mailbox.c
+++ b/drivers/media/pci/ivtv/ivtv-mailbox.c
@@ -19,11 +19,11 @@
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#include 
-
 #include "ivtv-driver.h"
 #include "ivtv-mailbox.h"
 
+#include 
+
 /* Firmware mailbox flags*/
 #define IVTV_MBOX_FIRMWARE_DONE 0x0004
 #define IVTV_MBOX_DRIVER_DONE   0x0002
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 612a8402cf4d..b59b60d605eb 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -38,18 +38,6 @@
 Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#ifdef CONFIG_X86_64
-#include 
-#endif
-
 #include "ivtv-driver.h"
 #include "ivtv-cards.h"
 #include "ivtv-i2c.h"
@@ -57,6 +45,13 @@
 #include "ivtv-mailbox.h"
 #include "ivtv-firmware.h"
 
+#include 
+#include 
+
+#ifdef CONFIG_X86_64
+#include 
+#endif
+
 /* card parameters */
 static 

Re: Enabling peer to peer device transactions for PCIe devices

[Christian König]

Am 24.11.2016 um 00:25 schrieb Jason Gunthorpe:

There is certainly nothing about the hardware that cares
about ZONE_DEVICE vs System memory.
Well that is clearly not so simple. When your ZONE_DEVICE pages describe 
a PCI BAR and another PCI device initiates a DMA to this address the DMA 
subsystem must be able to check if the interconnection really works.


E.g. it can happen that PCI device A exports it's BAR using ZONE_DEVICE. 
Not PCI device B (a SATA device) can directly read/write to it because 
it is on the same bus segment, but PCI device C (a network card for 
example) can't because it is on a different bus segment and the bridge 
can't handle P2P transactions.


We need to be able to handle such cases and fall back to bouncing 
buffers, but I don't see that in the DMA subsystem right now.


Regards,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4l-utils v7 0/7] Add a plugin for Exynos4 camera

[Mauro Carvalho Chehab]

Em Thu, 24 Nov 2016 09:10:28 +0100
Jacek Anaszewski  escreveu:

> On 11/23/2016 10:51 PM, Mauro Carvalho Chehab wrote:
> > Em Thu, 03 Nov 2016 13:13:12 +0100
> > Jacek Anaszewski  escreveu:
> >  
> >> Hi Hans,
> >>
> >> On 11/03/2016 12:51 PM, Hans Verkuil wrote:  
> >>> Hi all,
> >>>
> >>> Is there anything that blocks me from merging this?
> >>>
> >>> This plugin work has been ongoing for years and unless there are serious
> >>> objections I propose that this is merged.
> >>>
> >>> Jacek, is there anything missing that would prevent merging this?  
> >>
> >> There were issues raised by Sakari during last review, related to
> >> the way how v4l2 control bindings are defined. That discussion wasn't
> >> finished, so I stayed by my approach.  
> >
> > Are those things something that could be fixed later without
> > breaking binary apps? If not, then perhaps it is time to merge
> > this.  
> 
> They are related to the format of configuration file being introduced
> by this patch set, so there are no binary apps depending on it.

Ok. Yet, changing the configuration file format could be a pain
for the users. The best would be to add a format that would be
extensible enough to be able to add support for other needs in
the future.

> Few days ago I had a discussion with Sakari on IRC, and he pointed
> me to his patch set adding an extended version of his mediatext
> library RFC [0], the first version of which I included to my patch set,
> and extended a bit. A decision should be made if I should adapt my
> API to the mediatext RFC. RFC is not a stable ground though.
> 
> [0] https://www.spinics.net/lists/linux-media/msg103242.html

If I understood well, you're proposing to use this format for
the configuration file:

link-conf "s5p-mipi-csis.0":1 -> "FIMC.0":0 [1]
ctrl-to-subdev-conf 0x0098091f -> "fimc.0.capture"
ctrl-to-subdev-conf 0x00980902 -> "S5C73M3"
ctrl-to-subdev-conf 0x00980922 -> "fimc.0.capture"
ctrl-to-subdev-conf 0x009a0914 -> "S5C73M3"

What if, instead, you use something like:

[group]
key = value

type of configuration file?

This way, if one wants to extend it, all it is needed is to add
other groups or more key types.

The users with an older configuration won't suffer if newer
groups and newer key types were added.

E. g something like:

[link]
source = "s5p-mipi-csis.0":1
sink = "FIMC.0":0 [1]

[ctrl-to-subdev]
ctrl_id = 0x0098091f
subdev= "fimc.0.capture"

[ctrl-to-subdev]
ctrl_id = 0x00980902
subdev = "S5C73M3"

...


This way, Sakari's patches could use the same format to add mediatext
entries. E. g. something like (the actual content actually depends on
what is here - I'm just wild guessing from the tree diagram at the
RFC e-mail):

[mediatext]
shell_script = foo
devnode = /dev/video*
devnode = /dev/mediaX
devnode = /dev/v4l-subdev*

without needing to change the format of the file, and keeping it
backward compatible.

Regards,
Mauro
> 
> >> Other than that - I've tested it
> >> and it works fine both with GStreamer and my test app.
> >>
> >> Best regards,
> >> Jacek Anaszewski
> >>
> >>  
> >>> On 12/10/16 16:35, Jacek Anaszewski wrote:  
>  This is a seventh version of the patch series adding a plugin for the
>  Exynos4 camera. Last version [0] of the patch set was posted in
>  January.
> 
>  The plugin doesn't link against libmediactl, but has its sources
>  compiled in. Currently utils are built after the plugins, but
>  libv4l-exynos4-camera plugin depends on the utils. In order to link
>  the plugin against libmediactl the build system would have to be
>  modified.
> 
>  
>  Changes from v6:
>  
> 
>  - close v4l2 sub-devices on media device release
>  - moved non-generic code from libmediactl to the plugin
>  - resigned from adding libmedia_ioctl library and moved all its
>    code to the plugin, since it depended on pipeline representation,
>    which was not generic for all possible media device topologies
>  - used media_get_info()->name instead of adding media_entity_get_name
>  - renamed media_get_backlinks_by_entity() to
>  media_entity_get_backlinks(()
>  - moved pipeline from struct media_device to the plugin
>  - changed the way of associating video device file descriptor with
>  media device
>  - switched to using auto-generated media-bus-format-names.h header file
>  - renamed v4l2-ctrl-redir config entry name to v4l2-ctrl-binding
> 
>  
>  Changes from v5:
>  
> 
>  - fixed and tested use cases with S5K6A3 sensor and FIMC-IS-ISP
>  - added conversion "colorspace id to string"
> 
>  
>  Changes from v4:
>  
> 
>  - removed some redundant 

Re: [PATCH] [media] ti-vpe: get rid of some smatch warnings

[Tomi Valkeinen]

Hi,

On 22/11/16 13:09, Mauro Carvalho Chehab wrote:
> When compiled on i386, it produces several warnings:
> 
>   ./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an 
> lvalue
>   ./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an 
> lvalue
>   ./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an 
> lvalue
>   ./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an 
> lvalue
>   ./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an 
> lvalue
>   ./arch/x86/include/asm/bitops.h:457:22: warning: asm output is not an 
> lvalue
> 
> I suspect that some gcc optimization could be causing the asm code to be
> incorrectly generated. Splitting it into two macro calls fix the issues
> and gets us rid of 6 smatch warnings, with is a good thing. As it should
> not cause any troubles, as we're basically doing the same thing, let's
> apply such change to vpe.c.
> 
> Cc: Benoit Parrot 
> Cc: Hans Verkuil 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/platform/ti-vpe/vpe.c | 18 +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

I think the point of COMPILE_TEST is to improve the quality of the code.
This patch doesn't improve the quality, on the contrary.

If those warnings on a (buggy?) i386 gcc are a problem, I suggest
removing COMPILE_TEST for vpe.

 Tomi



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4l-utils v7 0/7] Add a plugin for Exynos4 camera

[Jacek Anaszewski]

On 11/23/2016 10:51 PM, Mauro Carvalho Chehab wrote:

Em Thu, 03 Nov 2016 13:13:12 +0100
Jacek Anaszewski  escreveu:


Hi Hans,

On 11/03/2016 12:51 PM, Hans Verkuil wrote:

Hi all,

Is there anything that blocks me from merging this?

This plugin work has been ongoing for years and unless there are serious
objections I propose that this is merged.

Jacek, is there anything missing that would prevent merging this?


There were issues raised by Sakari during last review, related to
the way how v4l2 control bindings are defined. That discussion wasn't
finished, so I stayed by my approach.


Are those things something that could be fixed later without
breaking binary apps? If not, then perhaps it is time to merge
this.


They are related to the format of configuration file being introduced
by this patch set, so there are no binary apps depending on it.

Few days ago I had a discussion with Sakari on IRC, and he pointed
me to his patch set adding an extended version of his mediatext
library RFC [0], the first version of which I included to my patch set,
and extended a bit. A decision should be made if I should adapt my
API to the mediatext RFC. RFC is not a stable ground though.

[0] https://www.spinics.net/lists/linux-media/msg103242.html


Other than that - I've tested it
and it works fine both with GStreamer and my test app.

Best regards,
Jacek Anaszewski



On 12/10/16 16:35, Jacek Anaszewski wrote:

This is a seventh version of the patch series adding a plugin for the
Exynos4 camera. Last version [0] of the patch set was posted in
January.

The plugin doesn't link against libmediactl, but has its sources
compiled in. Currently utils are built after the plugins, but
libv4l-exynos4-camera plugin depends on the utils. In order to link
the plugin against libmediactl the build system would have to be
modified.


Changes from v6:


- close v4l2 sub-devices on media device release
- moved non-generic code from libmediactl to the plugin
- resigned from adding libmedia_ioctl library and moved all its
  code to the plugin, since it depended on pipeline representation,
  which was not generic for all possible media device topologies
- used media_get_info()->name instead of adding media_entity_get_name
- renamed media_get_backlinks_by_entity() to
media_entity_get_backlinks(()
- moved pipeline from struct media_device to the plugin
- changed the way of associating video device file descriptor with
media device
- switched to using auto-generated media-bus-format-names.h header file
- renamed v4l2-ctrl-redir config entry name to v4l2-ctrl-binding


Changes from v5:


- fixed and tested use cases with S5K6A3 sensor and FIMC-IS-ISP
- added conversion "colorspace id to string"


Changes from v4:


- removed some redundant functions for traversing media device graph
  and switched over to using existing ones
- avoided accessing struct v4l2_subdev from libmediactl
- applied various improvements


Changes from v3:


- added struct v4l2_subdev and put entity fd and
  information about supported controls to it
- improved functions for negotiating and setting
  pipeline format by using available libv4lsubdev API
- applied minor improvements and cleanups


Changes from v2:


- switched to using mediatext library for parsing
  the media device configuration
- extended libmediactl
- switched to using libmediactl


Changes from v1:


- removed redundant mbus code negotiation
- split the parser, media device helpers and ioctl wrappers
  to the separate modules
- added mechanism for querying extended controls
- applied various fixes and modifications



The plugin was tested on v4.8-rc2 (exynos4-is driver doesn't proble
properly
with current master branch of linux-media.git) with patches fixing
several
issues for Exynos4 camera: [1], [2], [3].

The plugin expects a configuration file:
/var/lib/libv4l/exynos4_capture_conf

Exemplary configuration file for pipeline with sensor
S5C73M3 (rear camera):

==

link-conf "s5p-mipi-csis.0":1 -> "FIMC.0":0 [1]
v4l2-ctrl-binding 0x0098091f -> "fimc.0.capture"
v4l2-ctrl-binding 0x00980902 -> "S5C73M3"
v4l2-ctrl-binding 0x00980922 -> "fimc.0.capture"
v4l2-ctrl-binding 0x009a0914 -> "S5C73M3"

==

With this settings the plugin can be tested on the exynos4412-trats2
board
using following gstreamer pipeline:

gst-launch-1.0 v4l2src device=/dev/video1
extra-controls="c,rotate=90,color_effects=2" !
video/x-raw,width=960,height=720 ! fbdevsink

Exemplary configuration file for pipeline with sensor
S5K6A3 (front camera):

==

link-conf "s5p-mipi-csis.1":1 -> "FIMC-LITE.1":0 [1]
link-conf "FIMC-LITE.1":2 -> "FIMC-IS-ISP":0 [1]
link-conf "FIMC-IS-ISP":1 -> "FIMC.0":1 [1]

Re: [PATCH v3] media: i2c-polling: add i2c-polling driver

[Matt Ranostay]

On Wed, Nov 23, 2016 at 10:31 PM, Matt Ranostay
 wrote:
> On Wed, Nov 23, 2016 at 8:30 AM, Laurent Pinchart
>  wrote:
>> Hi Matt,
>>
>> Thank you for the patch.
>>
>> On Tuesday 22 Nov 2016 17:18:40 Matt Ranostay wrote:
>>> There are several thermal sensors that only have a low-speed bus
>>> interface but output valid video data. This patchset enables support
>>> for the AMG88xx "Grid-Eye" sensor family.
>>>
>>> Cc: Attila Kinali 
>>> Cc: Marek Vasut 
>>> Cc: Luca Barbato 
>>> Signed-off-by: Matt Ranostay 
>>> ---
>>> Changes from v1:
>>> * correct i2c_polling_remove() operations
>>> * fixed delay calcuation in buffer_queue()
>>> * add include linux/slab.h
>>>
>>> Changes from v2:
>>> * fix build error due to typo in include of slab.h
>>>
>>>  drivers/media/i2c/Kconfig   |   8 +
>>>  drivers/media/i2c/Makefile  |   1 +
>>>  drivers/media/i2c/i2c-polling.c | 469 +
>>
>> Just looking at the driver name I believe a rename is needed. i2c-polling is 
>> a
>> very generic name and would mislead many people into thinking about an I2C
>> subsystem core feature instead of a video driver. "video-i2c" is one option,
>> I'm open to other ideas.
>>
>>>  3 files changed, 478 insertions(+)
>>>  create mode 100644 drivers/media/i2c/i2c-polling.c
>>>
>>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>>> index b31fa6fae009..d840e7be0036 100644
>>> --- a/drivers/media/i2c/Kconfig
>>> +++ b/drivers/media/i2c/Kconfig
>>> @@ -768,6 +768,14 @@ config VIDEO_M52790
>>>
>>>To compile this driver as a module, choose M here: the
>>>module will be called m52790.
>>> +
>>> +config VIDEO_I2C_POLLING
>>> + tristate "I2C polling video support"
>>> + depends on VIDEO_V4L2 && I2C
>>> + select VIDEOBUF2_VMALLOC
>>> + ---help---
>>> +   Enable the I2C polling video support which supports the following:
>>> +* Panasonic AMG88xx Grid-Eye Sensors
>>>  endmenu
>>>
>>>  menu "Sensors used on soc_camera driver"
>>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>>> index 92773b2e6225..8182ec9f66b9 100644
>>> --- a/drivers/media/i2c/Makefile
>>> +++ b/drivers/media/i2c/Makefile
>>> @@ -79,6 +79,7 @@ obj-$(CONFIG_VIDEO_LM3646)  += lm3646.o
>>>  obj-$(CONFIG_VIDEO_SMIAPP_PLL)   += smiapp-pll.o
>>>  obj-$(CONFIG_VIDEO_AK881X)   += ak881x.o
>>>  obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
>>> +obj-$(CONFIG_VIDEO_I2C_POLLING)  += i2c-polling.o
>>>  obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o
>>>  obj-$(CONFIG_VIDEO_OV2659)   += ov2659.o
>>>  obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
>>> diff --git a/drivers/media/i2c/i2c-polling.c
>>> b/drivers/media/i2c/i2c-polling.c new file mode 100644
>>> index ..46a4eecde2d2
>>> --- /dev/null
>>> +++ b/drivers/media/i2c/i2c-polling.c
>>> @@ -0,0 +1,469 @@
>>> +/*
>>> + * i2c_polling.c - Support for polling I2C video devices
>>> + *
>>> + * Copyright (C) 2016 Matt Ranostay 
>>> + *
>>> + * Based on the orginal work drivers/media/parport/bw-qcam.c
>>> + *
>>> + * 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.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * Supported:
>>> + * - Panasonic AMG88xx Grid-Eye Sensors
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>
>> I don't think you implement device tree support, is linux/of.h needed ? Or
>> maybe you could implement device tree support ;-)
>>
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>
>> You don't implement any control, you can drop support for control events as
>> well.
>>
>>> +#include 
>>
>> Please sort includes alphabetically, it makes it easier to locate duplicates.
>>
>>> +#define I2C_POLLING_DRIVER   "i2c-polling"
>>> +
>>> +struct i2c_polling_chip;
>>> +
>>> +struct i2c_polling_data {
>>> + struct i2c_client *client;
>>> + const struct i2c_polling_chip *chip;
>>> + struct mutex lock;
>>> + struct mutex queue_lock;
>>> + unsigned int last_update;
>>> +
>>> + struct v4l2_device v4l2_dev;
>>> + struct video_device vdev;
>>> + struct vb2_queue vb_vidq;
>>> +};
>>> +
>>> +static struct v4l2_fmtdesc amg88xx_format = {
>>> + .description = "12-bit Greyscale",
>>> + .pixelformat = V4L2_PIX_FMT_Y12,
>>> +};
>>> +
>>> +static