cron job: media_tree daily build: WARNINGS

2017-09-07 Thread Hans Verkuil
This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Fri Sep  8 05:00:22 CEST 2017
media-tree git hash:1efdf1776e2253b77413c997bed862410e4b6aaf
media_build git hash:   bbd9f669f0da6705fe44aff89281c0d6e7bfd73e
v4l-utils git hash: 3296adfa7fa169111bf37c041c0ca70ac8506054
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.12.0-164

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

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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


Re: [PATCH v3 01/15] [media] v4l: Document explicit synchronization behaviour

2017-09-07 Thread Nicolas Dufresne
Le jeudi 07 septembre 2017 à 15:42 -0300, Gustavo Padovan a écrit :
> From: Gustavo Padovan 
> 
> Add section to VIDIOC_QBUF about it
> 
> v2:
>   - mention that fences are files (Hans)
>   - rework for the new API
> 
> Signed-off-by: Gustavo Padovan 
> ---
>  Documentation/media/uapi/v4l/vidioc-qbuf.rst | 31 
> 
>  1 file changed, 31 insertions(+)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-qbuf.rst 
> b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
> index 1f3612637200..fae0b1431672 100644
> --- a/Documentation/media/uapi/v4l/vidioc-qbuf.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
> @@ -117,6 +117,37 @@ immediately with an ``EAGAIN`` error code when no buffer 
> is available.
>  The struct :c:type:`v4l2_buffer` structure is specified in
>  :ref:`buffer`.
>  
> +Explicit Synchronization
> +
> +
> +Explicit Synchronization allows us to control the synchronization of
> +shared buffers from userspace by passing fences to the kernel and/or
> +receiving them from it. Fences passed to the kernel are named in-fences and
> +the kernel should wait them to signal before using the buffer, i.e., queueing
> +it to the driver. On the other side, the kernel can create out-fences for the
> +buffers it queues to the drivers, out-fences signal when the driver is
> +finished with buffer, that is the buffer is ready. The fence are represented
> +by file and passed as file descriptor to userspace.

I think the API works to deliver the fence FD userspace, though for the
userspace I maintain (GStreamer) it's often the case that the buffer is
unusable without the associated timestamp.

Let's consider the capture to display case (V4L2 -> DRM). As soon as
you add audio capture to the loop, GStreamer will need to start dealing
with synchronization. We can't just blindly give that buffer to the
display, we need to make sure this buffer makes it on time, in a way
that it is synchronized with the audio. To deal with synchronization,
we need to be able to correlate the video image capture time with the
audio capture time.

The problem is that this timestamp is only delivered when DQBUF
succeed, which is after the fence has unblocked. This makes the fences
completely unusable for that purpose. In general, to achieve very low
latency and still have synchronization, we'll probably need the
timestamp to be delivered somehow before the image transfer have
complete. Obviously, this is only possible if we have timestamp with
flag V4L2_BUF_FLAG_TSTAMP_SRC_SOE.

On another note, for m2m drivers that behave as color converters and
scalers, with ordered queues, userspace knows the timestamp already, so
 using the proposed API and passing the buffer immediately with it's
fence is really going to help.

For encoded (compressed) data, similar issues will be found with the
bytesused member of struct v4l2_buffer. We'll need to know the size of
the encoded data before we can pass it to another driver. I'm not sure
how relevant fences are for this type of data.

> +
> +The in-fences are communicated to the kernel at the ``VIDIOC_QBUF`` ioctl
> +using the ``V4L2_BUF_FLAG_IN_FENCE`` buffer
> +flags and the `fence_fd` field. If an in-fence needs to be passed to the 
> kernel,
> +`fence_fd` should be set to the fence file descriptor number and the
> +``V4L2_BUF_FLAG_IN_FENCE`` should be set as well. Failure to set both will
> +cause ``VIDIOC_QBUF`` to return with error.
> +
> +To get a out-fence back from V4L2 the ``V4L2_BUF_FLAG_OUT_FENCE`` flag should
> +be set to notify it that the next queued buffer should have a fence attached 
> to
> +it. That means the out-fence may not be associated with the buffer in the
> +current ``VIDIOC_QBUF`` ioctl call because the ordering in which videobuf2 
> core
> +queues the buffers to the drivers can't be guaranteed. To become aware of the
> +of the next queued buffer and the out-fence attached to it the

"of the" is repeated twice.

> +``V4L2_EVENT_BUF_QUEUED`` event should be used. It will trigger an event
> +for every buffer queued to the V4L2 driver.
> +
> +At streamoff the out-fences will either signal normally if the drivers wait
> +for the operations on the buffers to finish or signal with error if the
> +driver cancel the pending operations.
>  
>  Return Value
>  


Re: [PATCH v2 00/10] media: rc: gpio-ir-recv: driver update

2017-09-07 Thread Andi Shyti
Hi Ladislav,

> Serie was rebased on top of current linux.git, but something
> happened there and my userspace decoder no longer works: driver
> reports completely bogus timing such as (rc-5):
> ^427, _1342, ^945, _183, ^1128, _671, ^1586, _91, ^1189, _1525,
> ^1738, _1433, ^915, _1159, ^1464, _1525, ^213, _1067, ^793, _0
> (^ used for pulse and _ for space)
> As it has nothing to do with my changes, I'm sending it anyway
> for review, which I do not expect to happen until merge window
> ends.

This means that your patch is anyway untested.

In any case I don't see much use if patch 1/10 as it doesn't
simplify much, but the rest (from 2 to 10) looks good to me.

Once it's tested you can add

Acked-by: Andi Shyti 

Andi

P.S. I don't see in this V2 the changelog from V1. Next time,
please add the changelog.


[PATCH v2 10/10] media: rc: gpio-ir-recv: use gpiolib API

2017-09-07 Thread Ladislav Michl
Gpiolib API is preferred way to access gpios.

Signed-off-by: Ladislav Michl 
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/Kconfig|  1 +
 drivers/media/rc/gpio-ir-recv.c | 59 +++--
 2 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index d9ce8ff55d0c..6bfe983ff295 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -393,6 +393,7 @@ config RC_LOOPBACK
 config IR_GPIO_CIR
tristate "GPIO IR remote control"
depends on RC_CORE
+   depends on (OF && GPIOLIB) || COMPILE_TEST
---help---
   Say Y if you want to use GPIO based IR Receiver.
 
diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 5c47661281e3..5bb0851eacce 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -26,36 +26,25 @@
 
 struct gpio_rc_dev {
struct rc_dev *rcdev;
-   int gpio_nr;
-   bool active_low;
+   struct gpio_desc *gpiod;
+   int irq;
 };
 
 static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
 {
+   int val;
struct gpio_rc_dev *gpio_dev = dev_id;
-   int gval;
-   int rc = 0;
 
-   gval = gpio_get_value(gpio_dev->gpio_nr);
+   val = gpiod_get_value(gpio_dev->gpiod);
+   if (val >= 0)
+   ir_raw_event_store_edge(gpio_dev->rcdev, val == 1);
 
-   if (gval < 0)
-   goto err_get_value;
-
-   if (gpio_dev->active_low)
-   gval = !gval;
-
-   rc = ir_raw_event_store_edge(gpio_dev->rcdev, gval == 1);
-   if (rc < 0)
-   goto err_get_value;
-
-err_get_value:
return IRQ_HANDLED;
 }
 
 static int gpio_ir_recv_probe(struct platform_device *pdev)
 {
int rc;
-   enum of_gpio_flags flags;
struct rc_dev *rcdev;
struct gpio_rc_dev *gpio_dev;
struct device *dev = >dev;
@@ -68,15 +57,17 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
if (!gpio_dev)
return -ENOMEM;
 
-   rc = of_get_gpio_flags(np, 0, );
-   if (rc < 0) {
+   gpio_dev->gpiod = devm_gpiod_get(dev, NULL, GPIOD_IN);
+   if (IS_ERR(gpio_dev->gpiod)) {
+   rc = PTR_ERR(gpio_dev->gpiod);
+   /* Just try again if this happens */
if (rc != -EPROBE_DEFER)
-   dev_err(dev, "Failed to get gpio flags (%d)\n", rc);
+   dev_err(dev, "error getting gpio (%d)\n", rc);
return rc;
}
-
-   gpio_dev->gpio_nr = rc;
-   gpio_dev->active_low = (flags & OF_GPIO_ACTIVE_LOW);
+   gpio_dev->irq = gpiod_to_irq(gpio_dev->gpiod);
+   if (gpio_dev->irq < 0)
+   return gpio_dev->irq;
 
rcdev = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
if (!rcdev)
@@ -101,11 +92,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
gpio_dev->rcdev = rcdev;
 
-   rc = devm_gpio_request_one(dev, gpio_dev->gpio_nr, GPIOF_DIR_IN,
-   "gpio-ir-recv");
-   if (rc < 0)
-   return rc;
-
rc = devm_rc_register_device(dev, rcdev);
if (rc < 0) {
dev_err(dev, "failed to register rc device (%d)\n", rc);
@@ -114,8 +100,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, gpio_dev);
 
-   return devm_request_irq(dev, gpio_to_irq(gpio_dev->gpio_nr),
-   gpio_ir_recv_irq,
+   return devm_request_irq(dev, gpio_dev->irq, gpio_ir_recv_irq,
IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
"gpio-ir-recv-irq", gpio_dev);
 }
@@ -123,26 +108,24 @@ static int gpio_ir_recv_probe(struct platform_device 
*pdev)
 #ifdef CONFIG_PM
 static int gpio_ir_recv_suspend(struct device *dev)
 {
-   struct platform_device *pdev = to_platform_device(dev);
-   struct gpio_rc_dev *gpio_dev = platform_get_drvdata(pdev);
+   struct gpio_rc_dev *gpio_dev = dev_get_drvdata(dev);
 
if (device_may_wakeup(dev))
-   enable_irq_wake(gpio_to_irq(gpio_dev->gpio_nr));
+   enable_irq_wake(gpio_dev->irq);
else
-   disable_irq(gpio_to_irq(gpio_dev->gpio_nr));
+   disable_irq(gpio_dev->irq);
 
return 0;
 }
 
 static int gpio_ir_recv_resume(struct device *dev)
 {
-   struct platform_device *pdev = to_platform_device(dev);
-   struct gpio_rc_dev *gpio_dev = platform_get_drvdata(pdev);
+   struct gpio_rc_dev *gpio_dev = dev_get_drvdata(dev);
 
if (device_may_wakeup(dev))
-   disable_irq_wake(gpio_to_irq(gpio_dev->gpio_nr));
+   disable_irq_wake(gpio_dev->irq);
else
-   enable_irq(gpio_to_irq(gpio_dev->gpio_nr));
+   enable_irq(gpio_dev->irq);
 

[PATCH v2 09/10] media: rc: gpio-ir-recv: remove gpio_ir_recv_platform_data

2017-09-07 Thread Ladislav Michl
gpio_ir_recv_platform_data are not used anywhere in kernel tree,
so remove it.

Signed-off-by: Ladislav Michl 
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c  | 98 +++-
 include/linux/platform_data/media/gpio-ir-recv.h | 23 --
 2 files changed, 29 insertions(+), 92 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index db193ad4b819..5c47661281e3 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #define GPIO_IR_DEVICE_NAME"gpio_ir_recv"
 
@@ -31,45 +30,6 @@ struct gpio_rc_dev {
bool active_low;
 };
 
-#ifdef CONFIG_OF
-/*
- * Translate OpenFirmware node properties into platform_data
- */
-static int gpio_ir_recv_get_devtree_pdata(struct device *dev,
- struct gpio_ir_recv_platform_data *pdata)
-{
-   struct device_node *np = dev->of_node;
-   enum of_gpio_flags flags;
-   int gpio;
-
-   gpio = of_get_gpio_flags(np, 0, );
-   if (gpio < 0) {
-   if (gpio != -EPROBE_DEFER)
-   dev_err(dev, "Failed to get gpio flags (%d)\n", gpio);
-   return gpio;
-   }
-
-   pdata->gpio_nr = gpio;
-   pdata->active_low = (flags & OF_GPIO_ACTIVE_LOW);
-   /* probe() takes care of map_name == NULL or allowed_protos == 0 */
-   pdata->map_name = of_get_property(np, "linux,rc-map-name", NULL);
-   pdata->allowed_protos = 0;
-
-   return 0;
-}
-
-static const struct of_device_id gpio_ir_recv_of_match[] = {
-   { .compatible = "gpio-ir-receiver", },
-   { },
-};
-MODULE_DEVICE_TABLE(of, gpio_ir_recv_of_match);
-
-#else /* !CONFIG_OF */
-
-#define gpio_ir_recv_get_devtree_pdata(dev, pdata) (-ENOSYS)
-
-#endif
-
 static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
 {
struct gpio_rc_dev *gpio_dev = dev_id;
@@ -94,33 +54,30 @@ static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
 
 static int gpio_ir_recv_probe(struct platform_device *pdev)
 {
-   struct device *dev = >dev;
-   struct gpio_rc_dev *gpio_dev;
-   struct rc_dev *rcdev;
-   const struct gpio_ir_recv_platform_data *pdata = dev->platform_data;
int rc;
+   enum of_gpio_flags flags;
+   struct rc_dev *rcdev;
+   struct gpio_rc_dev *gpio_dev;
+   struct device *dev = >dev;
+   struct device_node *np = dev->of_node;
 
-   if (pdev->dev.of_node) {
-   struct gpio_ir_recv_platform_data *dtpdata =
-   devm_kzalloc(dev, sizeof(*dtpdata), GFP_KERNEL);
-   if (!dtpdata)
-   return -ENOMEM;
-   rc = gpio_ir_recv_get_devtree_pdata(dev, dtpdata);
-   if (rc)
-   return rc;
-   pdata = dtpdata;
-   }
-
-   if (!pdata)
-   return -EINVAL;
-
-   if (pdata->gpio_nr < 0)
-   return -EINVAL;
+   if (!np)
+   return -ENODEV;
 
gpio_dev = devm_kzalloc(dev, sizeof(struct gpio_rc_dev), GFP_KERNEL);
if (!gpio_dev)
return -ENOMEM;
 
+   rc = of_get_gpio_flags(np, 0, );
+   if (rc < 0) {
+   if (rc != -EPROBE_DEFER)
+   dev_err(dev, "Failed to get gpio flags (%d)\n", rc);
+   return rc;
+   }
+
+   gpio_dev->gpio_nr = rc;
+   gpio_dev->active_low = (flags & OF_GPIO_ACTIVE_LOW);
+
rcdev = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
if (!rcdev)
return -ENOMEM;
@@ -137,17 +94,14 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
rcdev->min_timeout = 1;
rcdev->timeout = IR_DEFAULT_TIMEOUT;
rcdev->max_timeout = 10 * IR_DEFAULT_TIMEOUT;
-   if (pdata->allowed_protos)
-   rcdev->allowed_protocols = pdata->allowed_protos;
-   else
-   rcdev->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
-   rcdev->map_name = pdata->map_name ?: RC_MAP_EMPTY;
+   rcdev->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
+   rcdev->map_name = of_get_property(np, "linux,rc-map-name", NULL);
+   if (!rcdev->map_name)
+   rcdev->map_name = RC_MAP_EMPTY;
 
gpio_dev->rcdev = rcdev;
-   gpio_dev->gpio_nr = pdata->gpio_nr;
-   gpio_dev->active_low = pdata->active_low;
 
-   rc = devm_gpio_request_one(dev, pdata->gpio_nr, GPIOF_DIR_IN,
+   rc = devm_gpio_request_one(dev, gpio_dev->gpio_nr, GPIOF_DIR_IN,
"gpio-ir-recv");
if (rc < 0)
return rc;
@@ -160,7 +114,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, gpio_dev);
 
-   return devm_request_irq(dev, gpio_to_irq(pdata->gpio_nr),
+   return devm_request_irq(dev, gpio_to_irq(gpio_dev->gpio_nr),

[PATCH v2 08/10] media: rc: gpio-ir-recv: use KBUILD_MODNAME

2017-09-07 Thread Ladislav Michl
There already is standard macro providing driver name, use it.

Signed-off-by: Ladislav Michl 
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 1d115f8531ba..db193ad4b819 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 
-#define GPIO_IR_DRIVER_NAME"gpio-rc-recv"
 #define GPIO_IR_DEVICE_NAME"gpio_ir_recv"
 
 struct gpio_rc_dev {
@@ -134,7 +133,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
rcdev->input_id.product = 0x0001;
rcdev->input_id.version = 0x0100;
rcdev->dev.parent = dev;
-   rcdev->driver_name = GPIO_IR_DRIVER_NAME;
+   rcdev->driver_name = KBUILD_MODNAME;
rcdev->min_timeout = 1;
rcdev->timeout = IR_DEFAULT_TIMEOUT;
rcdev->max_timeout = 10 * IR_DEFAULT_TIMEOUT;
@@ -203,7 +202,7 @@ static const struct dev_pm_ops gpio_ir_recv_pm_ops = {
 static struct platform_driver gpio_ir_recv_driver = {
.probe  = gpio_ir_recv_probe,
.driver = {
-   .name   = GPIO_IR_DRIVER_NAME,
+   .name   = KBUILD_MODNAME,
.of_match_table = of_match_ptr(gpio_ir_recv_of_match),
 #ifdef CONFIG_PM
.pm = _ir_recv_pm_ops,
-- 
2.11.0



[PATCH v2 07/10] media: rc: gpio-ir-recv: use devm_request_irq

2017-09-07 Thread Ladislav Michl
Use of devm_request_irq simplifies error unwinding and as
free_irq was the last user of driver remove function,
remove it too.

Signed-off-by: Ladislav Michl 
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 22 +++---
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 110276d49495..1d115f8531ba 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -161,25 +161,10 @@ static int gpio_ir_recv_probe(struct platform_device 
*pdev)
 
platform_set_drvdata(pdev, gpio_dev);
 
-   rc = request_irq(gpio_to_irq(pdata->gpio_nr),
+   return devm_request_irq(dev, gpio_to_irq(pdata->gpio_nr),
gpio_ir_recv_irq,
-   IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
-   "gpio-ir-recv-irq", gpio_dev);
-   if (rc < 0)
-   goto err_request_irq;
-
-   return 0;
-
-err_request_irq:
-   return rc;
-}
-
-static int gpio_ir_recv_remove(struct platform_device *pdev)
-{
-   struct gpio_rc_dev *gpio_dev = platform_get_drvdata(pdev);
-
-   free_irq(gpio_to_irq(gpio_dev->gpio_nr), gpio_dev);
-   return 0;
+   IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
+   "gpio-ir-recv-irq", gpio_dev);
 }
 
 #ifdef CONFIG_PM
@@ -217,7 +202,6 @@ static const struct dev_pm_ops gpio_ir_recv_pm_ops = {
 
 static struct platform_driver gpio_ir_recv_driver = {
.probe  = gpio_ir_recv_probe,
-   .remove = gpio_ir_recv_remove,
.driver = {
.name   = GPIO_IR_DRIVER_NAME,
.of_match_table = of_match_ptr(gpio_ir_recv_of_match),
-- 
2.11.0



[PATCH v2 06/10] media: rc: gpio-ir-recv: do not allow threaded interrupt handler

2017-09-07 Thread Ladislav Michl
Requesting any context irq is not actually great idea since threaded
interrupt handler is run at too unpredictable time which turns
timing information wrong. Fix it by requesting regular interrupt.

Signed-off-by: Ladislav Michl 
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 733e4ed35078..110276d49495 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -161,7 +161,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, gpio_dev);
 
-   rc = request_any_context_irq(gpio_to_irq(pdata->gpio_nr),
+   rc = request_irq(gpio_to_irq(pdata->gpio_nr),
gpio_ir_recv_irq,
IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
"gpio-ir-recv-irq", gpio_dev);
-- 
2.11.0



[PATCH v2 01/10] media: rc: gpio-ir-recv: use helper vaiable to acess device info

2017-09-07 Thread Ladislav Michl
Using explicit struct device variable makes code a bit more readable.

Signed-off-by: Ladislav Michl 
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 7248b3662285..741a68c192ce 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -95,18 +95,18 @@ static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id)
 
 static int gpio_ir_recv_probe(struct platform_device *pdev)
 {
+   struct device *dev = >dev;
struct gpio_rc_dev *gpio_dev;
struct rc_dev *rcdev;
-   const struct gpio_ir_recv_platform_data *pdata =
-   pdev->dev.platform_data;
+   const struct gpio_ir_recv_platform_data *pdata = dev->platform_data;
int rc;
 
if (pdev->dev.of_node) {
struct gpio_ir_recv_platform_data *dtpdata =
-   devm_kzalloc(>dev, sizeof(*dtpdata), GFP_KERNEL);
+   devm_kzalloc(dev, sizeof(*dtpdata), GFP_KERNEL);
if (!dtpdata)
return -ENOMEM;
-   rc = gpio_ir_recv_get_devtree_pdata(>dev, dtpdata);
+   rc = gpio_ir_recv_get_devtree_pdata(dev, dtpdata);
if (rc)
return rc;
pdata = dtpdata;
@@ -135,7 +135,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
rcdev->input_id.vendor = 0x0001;
rcdev->input_id.product = 0x0001;
rcdev->input_id.version = 0x0100;
-   rcdev->dev.parent = >dev;
+   rcdev->dev.parent = dev;
rcdev->driver_name = GPIO_IR_DRIVER_NAME;
rcdev->min_timeout = 1;
rcdev->timeout = IR_DEFAULT_TIMEOUT;
@@ -159,7 +159,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
rc = rc_register_device(rcdev);
if (rc < 0) {
-   dev_err(>dev, "failed to register rc device\n");
+   dev_err(dev, "failed to register rc device (%d)\n", rc);
goto err_register_rc_device;
}
 
-- 
2.11.0



[PATCH v2 05/10] media: rc: gpio-ir-recv: use devm_rc_register_device

2017-09-07 Thread Ladislav Michl
Use of devm_rc_register_device simplifies error unwinding.

Signed-off-by: Ladislav Michl 
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 5ce97b3612d6..733e4ed35078 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -153,10 +153,10 @@ static int gpio_ir_recv_probe(struct platform_device 
*pdev)
if (rc < 0)
return rc;
 
-   rc = rc_register_device(rcdev);
+   rc = devm_rc_register_device(dev, rcdev);
if (rc < 0) {
dev_err(dev, "failed to register rc device (%d)\n", rc);
-   goto err_register_rc_device;
+   return rc;
}
 
platform_set_drvdata(pdev, gpio_dev);
@@ -171,9 +171,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
return 0;
 
 err_request_irq:
-   rc_unregister_device(rcdev);
-   rcdev = NULL;
-err_register_rc_device:
return rc;
 }
 
@@ -182,7 +179,6 @@ static int gpio_ir_recv_remove(struct platform_device *pdev)
struct gpio_rc_dev *gpio_dev = platform_get_drvdata(pdev);
 
free_irq(gpio_to_irq(gpio_dev->gpio_nr), gpio_dev);
-   rc_unregister_device(gpio_dev->rcdev);
return 0;
 }
 
-- 
2.11.0



[PATCH v2 04/10] media: rc: gpio-ir-recv: use devm_gpio_request_one

2017-09-07 Thread Ladislav Michl
Use of devm_gpio_request_one simplifies error unwinding.

Signed-off-by: Ladislav Michl 
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index ee191f26efb4..5ce97b3612d6 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -148,12 +148,10 @@ static int gpio_ir_recv_probe(struct platform_device 
*pdev)
gpio_dev->gpio_nr = pdata->gpio_nr;
gpio_dev->active_low = pdata->active_low;
 
-   rc = gpio_request(pdata->gpio_nr, "gpio-ir-recv");
+   rc = devm_gpio_request_one(dev, pdata->gpio_nr, GPIOF_DIR_IN,
+   "gpio-ir-recv");
if (rc < 0)
return rc;
-   rc  = gpio_direction_input(pdata->gpio_nr);
-   if (rc < 0)
-   goto err_gpio_direction_input;
 
rc = rc_register_device(rcdev);
if (rc < 0) {
@@ -176,8 +174,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
rc_unregister_device(rcdev);
rcdev = NULL;
 err_register_rc_device:
-err_gpio_direction_input:
-   gpio_free(pdata->gpio_nr);
return rc;
 }
 
@@ -187,7 +183,6 @@ static int gpio_ir_recv_remove(struct platform_device *pdev)
 
free_irq(gpio_to_irq(gpio_dev->gpio_nr), gpio_dev);
rc_unregister_device(gpio_dev->rcdev);
-   gpio_free(gpio_dev->gpio_nr);
return 0;
 }
 
-- 
2.11.0



[PATCH v2 03/10] media: rc: gpio-ir-recv: use devm_rc_allocate_device

2017-09-07 Thread Ladislav Michl
Use of devm_rc_allocate_device simplifies error unwinding.

Signed-off-by: Ladislav Michl 
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 8f5e3a84a95e..ee191f26efb4 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -122,7 +122,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
if (!gpio_dev)
return -ENOMEM;
 
-   rcdev = rc_allocate_device(RC_DRIVER_IR_RAW);
+   rcdev = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW);
if (!rcdev)
return -ENOMEM;
 
@@ -150,7 +150,7 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 
rc = gpio_request(pdata->gpio_nr, "gpio-ir-recv");
if (rc < 0)
-   goto err_gpio_request;
+   return rc;
rc  = gpio_direction_input(pdata->gpio_nr);
if (rc < 0)
goto err_gpio_direction_input;
@@ -178,8 +178,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
 err_register_rc_device:
 err_gpio_direction_input:
gpio_free(pdata->gpio_nr);
-err_gpio_request:
-   rc_free_device(rcdev);
return rc;
 }
 
-- 
2.11.0



[PATCH v2 00/10] media: rc: gpio-ir-recv: driver update

2017-09-07 Thread Ladislav Michl
This patch serie brings driver closer to recently used APIs
and removes no longer used gpio_ir_recv_platform_data
support.

It was done as an excercise before writing similar driver using
FIQ and hw timers as this one gives too imprecise timing.

Serie was rebased on top of current linux.git, but something
happened there and my userspace decoder no longer works: driver
reports completely bogus timing such as (rc-5):
^427, _1342, ^945, _183, ^1128, _671, ^1586, _91, ^1189, _1525,
^1738, _1433, ^915, _1159, ^1464, _1525, ^213, _1067, ^793, _0
(^ used for pulse and _ for space)
As it has nothing to do with my changes, I'm sending it anyway
for review, which I do not expect to happen until merge window
ends.

Ladislav Michl (10):
  media: rc: gpio-ir-recv: use helper vaiable to acess device info
  media: rc: gpio-ir-recv: use devm_kzalloc
  media: rc: gpio-ir-recv: use devm_rc_allocate_device
  media: rc: gpio-ir-recv: use devm_gpio_request_one
  media: rc: gpio-ir-recv: use devm_rc_register_device
  media: rc: gpio-ir-recv: do not allow threaded interrupt handler
  media: rc: gpio-ir-recv: use devm_request_irq
  media: rc: gpio-ir-recv: use KBUILD_MODNAME
  media: rc: gpio-ir-recv: remove gpio_ir_recv_platform_data
  media: rc: gpio-ir-recv: use gpiolib API

 drivers/media/rc/Kconfig |   1 +
 drivers/media/rc/gpio-ir-recv.c  | 194 ++-
 include/linux/platform_data/media/gpio-ir-recv.h |  23 ---
 3 files changed, 53 insertions(+), 165 deletions(-)
 delete mode 100644 include/linux/platform_data/media/gpio-ir-recv.h

-- 
2.11.0



[PATCH v2 02/10] media: rc: gpio-ir-recv: use devm_kzalloc

2017-09-07 Thread Ladislav Michl
Use of devm_kzalloc simplifies error unwinding.

Signed-off-by: Ladislav Michl 
---
 Changes:
 -v2: rebased to current linux.git

 drivers/media/rc/gpio-ir-recv.c | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c
index 741a68c192ce..8f5e3a84a95e 100644
--- a/drivers/media/rc/gpio-ir-recv.c
+++ b/drivers/media/rc/gpio-ir-recv.c
@@ -118,15 +118,13 @@ static int gpio_ir_recv_probe(struct platform_device 
*pdev)
if (pdata->gpio_nr < 0)
return -EINVAL;
 
-   gpio_dev = kzalloc(sizeof(struct gpio_rc_dev), GFP_KERNEL);
+   gpio_dev = devm_kzalloc(dev, sizeof(struct gpio_rc_dev), GFP_KERNEL);
if (!gpio_dev)
return -ENOMEM;
 
rcdev = rc_allocate_device(RC_DRIVER_IR_RAW);
-   if (!rcdev) {
-   rc = -ENOMEM;
-   goto err_allocate_device;
-   }
+   if (!rcdev)
+   return -ENOMEM;
 
rcdev->priv = gpio_dev;
rcdev->device_name = GPIO_IR_DEVICE_NAME;
@@ -182,8 +180,6 @@ static int gpio_ir_recv_probe(struct platform_device *pdev)
gpio_free(pdata->gpio_nr);
 err_gpio_request:
rc_free_device(rcdev);
-err_allocate_device:
-   kfree(gpio_dev);
return rc;
 }
 
@@ -194,7 +190,6 @@ static int gpio_ir_recv_remove(struct platform_device *pdev)
free_irq(gpio_to_irq(gpio_dev->gpio_nr), gpio_dev);
rc_unregister_device(gpio_dev->rcdev);
gpio_free(gpio_dev->gpio_nr);
-   kfree(gpio_dev);
return 0;
 }
 
-- 
2.11.0



Re: [PATCH v3 14/15] fs/files: export close_fd() symbol

2017-09-07 Thread Gustavo Padovan
On Fri, 2017-09-08 at 00:09 +0200, Hans Verkuil wrote:
> On 09/07/2017 08:42 PM, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > Rename __close_fd() to close_fd() and export it to be able close
> > files
> > in modules using file descriptors.
> > 
> > The usecase that motivates this change happens in V4L2 where we
> > send
> > events to userspace with a fd that has file installed in it. But if
> > for
> > some reason we have to cancel the video stream we need to close the
> > files
> > that haven't been shared with userspace yet. Thus the export of
> > close_fd() becomes necessary.
> > 
> > fd_install() happens when we call an ioctl to queue a buffer, but
> > we only
> > share the fd with userspace later, and that may happen in a kernel
> > thread
> > instead.
> 
> This isn't the way to do this.
> 
> You should only create the out fence file descriptor when userspace
> dequeues
> (i.e. calls VIDIOC_DQEVENT) the BUF_QUEUED event. That's when you
> give it to
> userspace and at that moment closing the fd is the responsibility of
> userspace.
> There is no point creating it earlier anyway since userspace can't
> get to it
> until it dequeues the event.
> 
> It does mean some more work in the V4L2 core since you need to hook
> into the
> DQEVENT code in order to do this.

Right, that makes a lot more sense. I'll change the implementation so
it can reflecting that. Thanks.

Gustavo


Re: [PATCH v3 14/15] fs/files: export close_fd() symbol

2017-09-07 Thread Hans Verkuil
On 09/07/2017 08:42 PM, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> Rename __close_fd() to close_fd() and export it to be able close files
> in modules using file descriptors.
> 
> The usecase that motivates this change happens in V4L2 where we send
> events to userspace with a fd that has file installed in it. But if for
> some reason we have to cancel the video stream we need to close the files
> that haven't been shared with userspace yet. Thus the export of
> close_fd() becomes necessary.
> 
> fd_install() happens when we call an ioctl to queue a buffer, but we only
> share the fd with userspace later, and that may happen in a kernel thread
> instead.

This isn't the way to do this.

You should only create the out fence file descriptor when userspace dequeues
(i.e. calls VIDIOC_DQEVENT) the BUF_QUEUED event. That's when you give it to
userspace and at that moment closing the fd is the responsibility of userspace.
There is no point creating it earlier anyway since userspace can't get to it
until it dequeues the event.

It does mean some more work in the V4L2 core since you need to hook into the
DQEVENT code in order to do this.

Regards,

Hans

> 
> Cc: Alexander Viro 
> Cc: linux-fsde...@vger.kernel.org
> Cc: Riley Andrews 
> Signed-off-by: Gustavo Padovan 
> ---
> This is more like a question, I don't know how unhappy people will be with
> this proposal to expose close_fd(). I'm all ears for more interesting
> ways of doing it!
> ---
>  drivers/android/binder.c | 2 +-
>  fs/file.c| 5 +++--
>  fs/open.c| 2 +-
>  include/linux/fdtable.h  | 2 +-
>  4 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index f7665c31feca..5a9bc73012df 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -440,7 +440,7 @@ static long task_close_fd(struct binder_proc *proc, 
> unsigned int fd)
>   if (proc->files == NULL)
>   return -ESRCH;
>  
> - retval = __close_fd(proc->files, fd);
> + retval = close_fd(proc->files, fd);
>   /* can't restart close syscall because file table entry was cleared */
>   if (unlikely(retval == -ERESTARTSYS ||
>retval == -ERESTARTNOINTR ||
> diff --git a/fs/file.c b/fs/file.c
> index 1fc7fbbb4510..111d387ac190 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -618,7 +618,7 @@ EXPORT_SYMBOL(fd_install);
>  /*
>   * The same warnings as for __alloc_fd()/__fd_install() apply here...
>   */
> -int __close_fd(struct files_struct *files, unsigned fd)
> +int close_fd(struct files_struct *files, unsigned fd)
>  {
>   struct file *file;
>   struct fdtable *fdt;
> @@ -640,6 +640,7 @@ int __close_fd(struct files_struct *files, unsigned fd)
>   spin_unlock(>file_lock);
>   return -EBADF;
>  }
> +EXPORT_SYMBOL(close_fd);
>  
>  void do_close_on_exec(struct files_struct *files)
>  {
> @@ -856,7 +857,7 @@ int replace_fd(unsigned fd, struct file *file, unsigned 
> flags)
>   struct files_struct *files = current->files;
>  
>   if (!file)
> - return __close_fd(files, fd);
> + return close_fd(files, fd);
>  
>   if (fd >= rlimit(RLIMIT_NOFILE))
>   return -EBADF;
> diff --git a/fs/open.c b/fs/open.c
> index 35bb784763a4..30907d967443 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -1152,7 +1152,7 @@ EXPORT_SYMBOL(filp_close);
>   */
>  SYSCALL_DEFINE1(close, unsigned int, fd)
>  {
> - int retval = __close_fd(current->files, fd);
> + int retval = close_fd(current->files, fd);
>  
>   /* can't restart close syscall because file table entry was cleared */
>   if (unlikely(retval == -ERESTARTSYS ||
> diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
> index 6e84b2cae6ad..511fd38d5e4b 100644
> --- a/include/linux/fdtable.h
> +++ b/include/linux/fdtable.h
> @@ -115,7 +115,7 @@ extern int __alloc_fd(struct files_struct *files,
> unsigned start, unsigned end, unsigned flags);
>  extern void __fd_install(struct files_struct *files,
> unsigned int fd, struct file *file);
> -extern int __close_fd(struct files_struct *files,
> +extern int close_fd(struct files_struct *files,
> unsigned int fd);
>  
>  extern struct kmem_cache *files_cachep;
> 



Re: [PATCH v3 14/15] fs/files: export close_fd() symbol

2017-09-07 Thread Al Viro
On Thu, Sep 07, 2017 at 06:22:45PM -0300, Gustavo Padovan wrote:

> Sorry for my lack of knowledge here and thank you for the explanation,
> things are a lot clear to me. For some reasons I were trying to delay
> the sharing of the fd to a event later. I can delay the install of it
> but that my require __fd_install() to be available and exportedi as it
> may happen in a thread, but I believe you wouldn't be okay with that either,
> is that so?

Only if it has been given a reference to descriptor table to start with.
Which reference should've been acquired by the target process itself.

Why bother, anyway?  You need to handle the case when the stream has
ended just after you'd copied the value to userland; at that point you
obviously can't go hunting for all references to struct file in question,
so you have to guaratee that methods will start giving an error from
that point on.  What's the problem with just leaving it installed?

Both userland and kernel must cope with that sort of thing anyway, so
what does removing it from descriptor table and not reporting it buy
you?  AFAICS, it's an extra layer of complexity for no good reason -
you are not getting it offset by simplifications anywhere else...


Re: [PATCH v3 14/15] fs/files: export close_fd() symbol

2017-09-07 Thread Gustavo Padovan
2017-09-07 Al Viro :

> On Thu, Sep 07, 2017 at 03:42:25PM -0300, Gustavo Padovan wrote:
> > From: Gustavo Padovan 
> > 
> > Rename __close_fd() to close_fd() and export it to be able close files
> > in modules using file descriptors.
> > 
> > The usecase that motivates this change happens in V4L2 where we send
> > events to userspace with a fd that has file installed in it. But if for
> > some reason we have to cancel the video stream we need to close the files
> > that haven't been shared with userspace yet. Thus the export of
> > close_fd() becomes necessary.
> > 
> > fd_install() happens when we call an ioctl to queue a buffer, but we only
> > share the fd with userspace later, and that may happen in a kernel thread
> > instead.
> 
> NAK.  As soon as the reference is in descriptor table, you *can't* do anything
> to it.  This "sharing" part is complete BS - being _told_ that descriptor is
> there does not matter at all.  That descriptor might be hit with dup2() as
> soon as fd_install() has happened.  Or be closed, or any number of other 
> things.
> 
> You can not take it back.  Once fd_install() is done, it's fucking done, 
> period.
> If V4L2 requires removing it from descriptor table, it's a shitty API and 
> needs
> to be fixed.

Sorry for my lack of knowledge here and thank you for the explanation,
things are a lot clear to me. For some reasons I were trying to delay
the sharing of the fd to a event later. I can delay the install of it
but that my require __fd_install() to be available and exportedi as it
may happen in a thread, but I believe you wouldn't be okay with that either,
is that so?

Gustavo


Re: [PATCH v3 14/15] fs/files: export close_fd() symbol

2017-09-07 Thread Al Viro
On Thu, Sep 07, 2017 at 03:42:25PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> Rename __close_fd() to close_fd() and export it to be able close files
> in modules using file descriptors.
> 
> The usecase that motivates this change happens in V4L2 where we send
> events to userspace with a fd that has file installed in it. But if for
> some reason we have to cancel the video stream we need to close the files
> that haven't been shared with userspace yet. Thus the export of
> close_fd() becomes necessary.
> 
> fd_install() happens when we call an ioctl to queue a buffer, but we only
> share the fd with userspace later, and that may happen in a kernel thread
> instead.

NAK.  As soon as the reference is in descriptor table, you *can't* do anything
to it.  This "sharing" part is complete BS - being _told_ that descriptor is
there does not matter at all.  That descriptor might be hit with dup2() as
soon as fd_install() has happened.  Or be closed, or any number of other things.

You can not take it back.  Once fd_install() is done, it's fucking done, period.
If V4L2 requires removing it from descriptor table, it's a shitty API and needs
to be fixed.

Again, NAK.


[PATCH] [media] blackfin: Delete an error message for a failed memory allocation in ppi_create_instance()

2017-09-07 Thread SF Markus Elfring
From: Markus Elfring 
Date: Thu, 7 Sep 2017 22:14:43 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/platform/blackfin/ppi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/platform/blackfin/ppi.c 
b/drivers/media/platform/blackfin/ppi.c
index 37169054b828..478eb2f7d723 100644
--- a/drivers/media/platform/blackfin/ppi.c
+++ b/drivers/media/platform/blackfin/ppi.c
@@ -338,7 +338,6 @@ struct ppi_if *ppi_create_instance(struct platform_device 
*pdev,
ppi = kzalloc(sizeof(*ppi), GFP_KERNEL);
if (!ppi) {
peripheral_free_list(info->pin_req);
-   dev_err(>dev, "unable to allocate memory for ppi 
handle\n");
return NULL;
}
ppi->ops = _ops;
-- 
2.14.1



Re: [PATCH v3 2/3] leds: as3645a: Add LED flash class driver

2017-09-07 Thread Pavel Machek
On Thu 2017-09-07 17:52:05, Sakari Ailus wrote:
> On Thu, Sep 07, 2017 at 02:50:27PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > On Mon, Aug 28, 2017 at 01:04:51PM +0200, Pavel Machek wrote:
> > > > On Wed 2017-08-23 11:10:59, Sakari Ailus wrote:
> > > > > Add a LED flash class driver for the as3654a flash controller. A V4L2 
> > > > > flash
> > > > > driver for it already exists (drivers/media/i2c/as3645a.c), and this 
> > > > > driver
> > > > > is based on that.
> > > > 
> > > > We do not want to have two drivers for same hardware... how is that
> > > > supposed to work?
> > > 
> > > No, we don't. The intent is to remove the other driver later on, as it's
> > > implemented as a V4L2 sub-device driver. It also lacks DT support.
> > 
> > Could we perhaps remove the driver with the same patch that merges
> > this?
> 
> This patch is already merged, but I can submit another patch removing the
> other driver.

Yes, that would be nice.

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


signature.asc
Description: Digital signature


Re: Donation

2017-09-07 Thread Mavis Wanczyk Foundation
Greetings To You,

My Name is Mavis wanczyk , the winner of the Power ball jackpot of $ $758.7 
million  in the AUGUST 24, 2017, My jackpot was a gift from God to me hence my 
Entire family/foundation has AGREED to do this. My foundation is donating 
$500,000.00USD to you. please contac maviswanczyk...@gmail.com for full details 
and please accept this token as a gift from me and my family.

Read more: 
http://money.cnn.com/2017/08/23/news/powerball-700-million-jackpot/index.html

 Best Regards,
 Mavis Wanczyk


Re: [PATCH v3 14/15] fs/files: export close_fd() symbol

2017-09-07 Thread Eric Biggers
On Thu, Sep 07, 2017 at 03:42:25PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan 
> 
> Rename __close_fd() to close_fd() and export it to be able close files
> in modules using file descriptors.
> 
> The usecase that motivates this change happens in V4L2 where we send
> events to userspace with a fd that has file installed in it. But if for
> some reason we have to cancel the video stream we need to close the files
> that haven't been shared with userspace yet. Thus the export of
> close_fd() becomes necessary.
> 
> fd_install() happens when we call an ioctl to queue a buffer, but we only
> share the fd with userspace later, and that may happen in a kernel thread
> instead.

What do you mean?  A file descriptor is shared with userspace as soon as it's
installed in the fdtable by fd_install().  As soon as it's there, another thread
can use it (or close it, duplicate it, etc.), even before the syscall that
installed it returns...

Eric


[PATCH v3 00/15] V4L2 Explicit Synchronization support

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

Hi,

Refer to the documentation on the first patch for the details. The previous
iteration is here: 
https://www.mail-archive.com/linux-media@vger.kernel.org/msg118077.html

The 2nd patch proposes an userspace API for fences, then on patch 3 we
prepare to the addition of in-fences in patch 4, by introducing the
infrastructure on vb2 to wait on an in-fence signal before queueing the
buffer in the driver.

Patch 5 fix uvc v4l2 event handling and patch 6 configure q->dev for
vivid drivers to enable to subscribe and dequeue events on it.

Patches 7-9 enables support to notify BUF_QUEUED events, the event send
to userspace the out-fence fd and the index of the buffer that was queued.

Patches 10-11 add support to mark queues as ordered. Finally patches 12
and 13 add more fence infrastructure to support out-fences, patch 13 exposes
close_fd() and patch 14 adds support to out-fences. 

It only works for ordered queues for now, see open question at the end
of the letter.

Test tool can be found at:
https://git.collabora.com/cgit/user/padovan/v4l2-test.git/

Main Changes


* out-fences: change in behavior: the out-fence fd now comes out of the
BUF_QUEUED event along with the buffer id.

All other changes are recorded on the patches' commit messages.

Open Questions
--

* non-ordered devices, like m2m: I've been thinking a lot about those
  and one possibility is to have a way to tell userspace that the queue
  is not ordered and then associate the fence with the current buffer in
  QBUF instead of the next one to be queued. Of course, there won't be
  any ordering between the fences. But it may be enough for userspace to
  take advantage of Explicit Synchronization in such cases. Any
  thoughts?

* OUTPUT devices and in-fence. If I understood OUTPUT devices correctly
  it is desirable to queue the buffers to the driver in the same order
  we received them from userspace. If that is correct, shouldn't we add
  some mechanism to prevent buffer whose fence signaled to jump ahead of
  others?

Gustavo Padovan (14):
  [media] v4l: Document explicit synchronization behaviour
  [media] vb2: add explicit fence user API
  [media] vb2: check earlier if stream can be started
  [media] vb2: add in-fence support to QBUF
  [media] uvc: enable subscriptions to other events
  [media] vivid: assign the specific device to the vb2_queue->dev
  [media] v4l: add V4L2_EVENT_BUF_QUEUED event
  [media] vb2: add .buffer_queued() to notify queueing in the driver
  [media] v4l: add support to BUF_QUEUED event
  [media] vb2: add 'ordered' property to queues
  [media] vivid: mark vivid queues as ordered
  [media] vb2: add infrastructure to support out-fences
  fs/files: export close_fd() symbol
  [media] vb2: add out-fence support to QBUF

Javier Martinez Canillas (1):
  [media] vb2: add videobuf2 dma-buf fence helpers

 Documentation/media/uapi/v4l/buffer.rst |  19 ++
 Documentation/media/uapi/v4l/vidioc-dqevent.rst |  23 +++
 Documentation/media/uapi/v4l/vidioc-qbuf.rst|  31 
 Documentation/media/videodev2.h.rst.exceptions  |   1 +
 drivers/android/binder.c|   2 +-
 drivers/media/platform/vivid/vivid-core.c   |  15 +-
 drivers/media/usb/cpia2/cpia2_v4l.c |   2 +-
 drivers/media/usb/uvc/uvc_v4l2.c|   2 +-
 drivers/media/v4l2-core/Kconfig |   1 +
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c   |   4 +-
 drivers/media/v4l2-core/v4l2-ctrls.c|   6 +-
 drivers/media/v4l2-core/videobuf2-core.c| 221 ++--
 drivers/media/v4l2-core/videobuf2-v4l2.c|  63 ++-
 fs/file.c   |   5 +-
 fs/open.c   |   2 +-
 include/linux/fdtable.h |   2 +-
 include/media/videobuf2-core.h  |  63 ++-
 include/media/videobuf2-fence.h |  49 ++
 include/uapi/linux/videodev2.h  |  15 +-
 19 files changed, 489 insertions(+), 37 deletions(-)
 create mode 100644 include/media/videobuf2-fence.h

-- 
2.13.5



[PATCH v3 02/15] [media] vb2: add explicit fence user API

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

Turn the reserved2 field into fence_fd that we will use to send
an in-fence to the kernel and return an out-fence from the kernel to
userspace.

Two new flags were added, V4L2_BUF_FLAG_IN_FENCE, that should be used
when sending a fence to the kernel to be waited on, and
V4L2_BUF_FLAG_OUT_FENCE, to ask the kernel to give back an out-fence.

v2: add documentation

Signed-off-by: Gustavo Padovan 
---
 Documentation/media/uapi/v4l/buffer.rst   | 19 +++
 drivers/media/usb/cpia2/cpia2_v4l.c   |  2 +-
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c |  4 ++--
 drivers/media/v4l2-core/videobuf2-v4l2.c  |  2 +-
 include/uapi/linux/videodev2.h|  4 +++-
 5 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/Documentation/media/uapi/v4l/buffer.rst 
b/Documentation/media/uapi/v4l/buffer.rst
index ae6ee73f151c..664507ad06c6 100644
--- a/Documentation/media/uapi/v4l/buffer.rst
+++ b/Documentation/media/uapi/v4l/buffer.rst
@@ -648,6 +648,25 @@ Buffer Flags
   - Start Of Exposure. The buffer timestamp has been taken when the
exposure of the frame has begun. This is only valid for the
``V4L2_BUF_TYPE_VIDEO_CAPTURE`` buffer type.
+* .. _`V4L2-BUF-FLAG-IN-FENCE`:
+
+  - ``V4L2_BUF_FLAG_IN_FENCE``
+  - 0x0020
+  - Ask V4L2 to wait on fence passed in ``fence_fd`` field. The buffer
+   won't be queued to the driver until the fence signals.
+
+* .. _`V4L2-BUF-FLAG-OUT-FENCE`:
+
+  - ``V4L2_BUF_FLAG_OUT_FENCE``
+  - 0x0040
+  - Request a fence for the next buffer to be queued to V4L2 driver.
+   The fence received back through the ``fence_fd`` field  doesn't
+   necessarily relate to the current buffer in the
+   :ref:`VIDIOC_QBUF ` ioctl. Although, most of the time
+   the fence will relate to the current buffer it can't be guaranteed.
+   So to tell userspace which buffer is associated to the out_fence,
+   one should listen for the ``V4L2_EVENT_BUF_QUEUED`` event that
+   provide the id of the buffer when it is queued to the V4L2 driver.
 
 
 
diff --git a/drivers/media/usb/cpia2/cpia2_v4l.c 
b/drivers/media/usb/cpia2/cpia2_v4l.c
index 3dedd83f0b19..6cde686bf44c 100644
--- a/drivers/media/usb/cpia2/cpia2_v4l.c
+++ b/drivers/media/usb/cpia2/cpia2_v4l.c
@@ -948,7 +948,7 @@ static int cpia2_dqbuf(struct file *file, void *fh, struct 
v4l2_buffer *buf)
buf->sequence = cam->buffers[buf->index].seq;
buf->m.offset = cam->buffers[buf->index].data - cam->frame_buffer;
buf->length = cam->frame_size;
-   buf->reserved2 = 0;
+   buf->fence_fd = -1;
buf->reserved = 0;
memset(>timecode, 0, sizeof(buf->timecode));
 
diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 821f2aa299ae..d624fb5df130 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -370,7 +370,7 @@ struct v4l2_buffer32 {
__s32   fd;
} m;
__u32   length;
-   __u32   reserved2;
+   __s32   fence_fd;
__u32   reserved;
 };
 
@@ -533,8 +533,8 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct 
v4l2_buffer32 __user
put_user(kp->timestamp.tv_usec, >timestamp.tv_usec) ||
copy_to_user(>timecode, >timecode, sizeof(struct 
v4l2_timecode)) ||
put_user(kp->sequence, >sequence) ||
-   put_user(kp->reserved2, >reserved2) ||
put_user(kp->reserved, >reserved) ||
+   put_user(kp->fence_fd, >fence_fd) ||
put_user(kp->length, >length))
return -EFAULT;
 
diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c 
b/drivers/media/v4l2-core/videobuf2-v4l2.c
index 0c0669976bdc..110fb45fef6f 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -203,7 +203,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void 
*pb)
b->timestamp = ns_to_timeval(vb->timestamp);
b->timecode = vbuf->timecode;
b->sequence = vbuf->sequence;
-   b->reserved2 = 0;
+   b->fence_fd = -1;
b->reserved = 0;
 
if (q->is_multiplanar) {
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 185d6a0acc06..e5abab9a908c 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -924,7 +924,7 @@ struct v4l2_buffer {
__s32   fd;
} m;
__u32   length;
-   __u32   reserved2;
+   __s32   fence_fd;
__u32   reserved;
 };
 
@@ -961,6 +961,8 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_TSTAMP_SRC_SOE   0x0001
 

[PATCH v3 03/15] [media] vb2: check earlier if stream can be started

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

To support explicit synchronization we need to run all operations that can
fail before we queue the buffer to the driver. With fences the queueing
will be delayed if the fence is not signaled yet and it will be better if
such callback do not fail.

For that we move the vb2_start_streaming() before the queuing for the
buffer may happen.

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/videobuf2-core.c | 20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index cb115ba6a1d2..60f8b582396a 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1399,29 +1399,27 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
index, void *pb)
trace_vb2_qbuf(q, vb);
 
/*
-* If already streaming, give the buffer to driver for processing.
-* If not, the buffer will be given to driver on next streamon.
-*/
-   if (q->start_streaming_called)
-   __enqueue_in_driver(vb);
-
-   /* Fill buffer information for the userspace */
-   if (pb)
-   call_void_bufop(q, fill_user_buffer, vb, pb);
-
-   /*
 * If streamon has been called, and we haven't yet called
 * start_streaming() since not enough buffers were queued, and
 * we now have reached the minimum number of queued buffers,
 * then we can finally call start_streaming().
+*
+* If already streaming, give the buffer to driver for processing.
+* If not, the buffer will be given to driver on next streamon.
 */
if (q->streaming && !q->start_streaming_called &&
q->queued_count >= q->min_buffers_needed) {
ret = vb2_start_streaming(q);
if (ret)
return ret;
+   } else if (q->start_streaming_called) {
+   __enqueue_in_driver(vb);
}
 
+   /* Fill buffer information for the userspace */
+   if (pb)
+   call_void_bufop(q, fill_user_buffer, vb, pb);
+
dprintk(2, "qbuf of buffer %d succeeded\n", vb->index);
return 0;
 }
-- 
2.13.5



[PATCH v3 05/15] [media] uvc: enable subscriptions to other events

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

Call v4l2_ctrl_subscribe_event to subscribe to the BUF_QUEUED event as
well.

Signed-off-by: Gustavo Padovan 
---
 drivers/media/usb/uvc/uvc_v4l2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 3e7e283a44a8..dfa0ccdcf849 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -1240,7 +1240,7 @@ static int uvc_ioctl_subscribe_event(struct v4l2_fh *fh,
case V4L2_EVENT_CTRL:
return v4l2_event_subscribe(fh, sub, 0, _ctrl_sub_ev_ops);
default:
-   return -EINVAL;
+   return v4l2_ctrl_subscribe_event(fh, sub);
}
 }
 
-- 
2.13.5



[PATCH v3 04/15] [media] vb2: add in-fence support to QBUF

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

Receive in-fence from userspace and add support for waiting on them
before queueing the buffer to the driver. Buffers are only queued
to the driver once they are ready. A buffer is ready when its
in-fence signals.

v4:
- Add a comment about dma_fence_add_callback() not returning a
error (Hans)
- Call dma_fence_put(vb->in_fence) if fence signaled (Hans)
- select SYNC_FILE under config VIDEOBUF2_CORE (Hans)
- Move dma_fence_is_signaled() check to __enqueue_in_driver() (Hans)
- Remove list_for_each_entry() in __vb2_core_qbuf() (Hans)
-  Remove if (vb->state != VB2_BUF_STATE_QUEUED) from
vb2_start_streaming() (Hans)
- set IN_FENCE flags on __fill_v4l2_buffer (Hans)
- Queue buffers to the driver as soon as they are ready (Hans)
- call fill_user_buffer() after queuing the buffer (Hans)
- add err: label to clean up fence
- add dma_fence_wait() before calling vb2_start_streaming()

v3: - document fence parameter
- remove ternary if at vb2_qbuf() return (Mauro)
- do not change if conditions behaviour (Mauro)

v2:
- fix vb2_queue_or_prepare_buf() ret check
- remove check for VB2_MEMORY_DMABUF only (Javier)
- check num of ready buffers to start streaming
- when queueing, start from the first ready buffer
- handle queue cancel

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/Kconfig  |   1 +
 drivers/media/v4l2-core/videobuf2-core.c | 103 +++
 drivers/media/v4l2-core/videobuf2-v4l2.c |  27 +++-
 include/media/videobuf2-core.h   |  11 +++-
 4 files changed, 127 insertions(+), 15 deletions(-)

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index a35c33686abf..3f988c407c80 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -83,6 +83,7 @@ config VIDEOBUF_DVB
 # Used by drivers that need Videobuf2 modules
 config VIDEOBUF2_CORE
select DMA_SHARED_BUFFER
+   select SYNC_FILE
tristate
 
 config VIDEOBUF2_MEMOPS
diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 60f8b582396a..b19c1bc4b083 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1222,6 +1222,9 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb->vb2_queue;
 
+   if (vb->in_fence && !dma_fence_is_signaled(vb->in_fence))
+   return;
+
vb->state = VB2_BUF_STATE_ACTIVE;
atomic_inc(>owned_by_drv_count);
 
@@ -1273,6 +1276,20 @@ static int __buf_prepare(struct vb2_buffer *vb, const 
void *pb)
return 0;
 }
 
+static int __get_num_ready_buffers(struct vb2_queue *q)
+{
+   struct vb2_buffer *vb;
+   int ready_count = 0;
+
+   /* count num of buffers ready in front of the queued_list */
+   list_for_each_entry(vb, >queued_list, queued_entry) {
+   if (!vb->in_fence || dma_fence_is_signaled(vb->in_fence))
+   ready_count++;
+   }
+
+   return ready_count;
+}
+
 int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
 {
struct vb2_buffer *vb;
@@ -1361,7 +1378,19 @@ static int vb2_start_streaming(struct vb2_queue *q)
return ret;
 }
 
-int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb)
+static void vb2_qbuf_fence_cb(struct dma_fence *f, struct dma_fence_cb *cb)
+{
+   struct vb2_buffer *vb = container_of(cb, struct vb2_buffer, fence_cb);
+
+   dma_fence_put(vb->in_fence);
+   vb->in_fence = NULL;
+
+   if (vb->vb2_queue->start_streaming_called)
+   __enqueue_in_driver(vb);
+}
+
+int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
+ struct dma_fence *fence)
 {
struct vb2_buffer *vb;
int ret;
@@ -1372,16 +1401,18 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
index, void *pb)
case VB2_BUF_STATE_DEQUEUED:
ret = __buf_prepare(vb, pb);
if (ret)
-   return ret;
+   goto err;
break;
case VB2_BUF_STATE_PREPARED:
break;
case VB2_BUF_STATE_PREPARING:
dprintk(1, "buffer still being prepared\n");
-   return -EINVAL;
+   ret = -EINVAL;
+   goto err;
default:
dprintk(1, "invalid buffer state %d\n", vb->state);
-   return -EINVAL;
+   ret = -EINVAL;
+   goto err;
}
 
/*
@@ -1392,6 +1423,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
index, void *pb)
q->queued_count++;
q->waiting_for_buffers = false;
vb->state = VB2_BUF_STATE_QUEUED;
+   

[PATCH v3 10/15] [media] vb2: add 'ordered' property to queues

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

For explicit synchronization (and soon for HAL3/Request API) we need
the v4l2-driver to guarantee the ordering in which the buffers were queued
by userspace. This is already true for many drivers, but we never needed
to say it.

Signed-off-by: Gustavo Padovan 
---
 include/media/videobuf2-core.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 5ed8d3402474..20099dc22f26 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -508,6 +508,9 @@ struct vb2_buf_ops {
  * @last_buffer_dequeued: used in poll() and DQBUF to immediately return if the
  * last decoded buffer was already dequeued. Set for capture queues
  * when a buffer with the V4L2_BUF_FLAG_LAST is dequeued.
+ * @ordered: if the driver can guarantee that the queue will be ordered or not.
+ * The default is not ordered unless the driver sets this flag. It
+ * is mandatory for using explicit fences.
  * @fileio:file io emulator internal data, used only if emulator is active
  * @threadio:  thread io internal data, used only if thread is active
  */
@@ -560,6 +563,7 @@ struct vb2_queue {
unsigned intis_output:1;
unsigned intcopy_timestamp:1;
unsigned intlast_buffer_dequeued:1;
+   unsigned intordered:1;
 
struct vb2_fileio_data  *fileio;
struct vb2_threadio_data*threadio;
-- 
2.13.5



[PATCH v3 07/15] [media] v4l: add V4L2_EVENT_BUF_QUEUED event

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

Add a new event the userspace can subscribe to receive notifications
when a buffer is queued onto the driver. The event provides the index of
the queued buffer.

v2: - Add missing Documentation (Mauro)

Signed-off-by: Gustavo Padovan 
---
 Documentation/media/uapi/v4l/vidioc-dqevent.rst | 23 +++
 Documentation/media/videodev2.h.rst.exceptions  |  1 +
 include/uapi/linux/videodev2.h  | 11 +++
 3 files changed, 35 insertions(+)

diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst 
b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
index fcd9c933870d..55f9dbdca6ec 100644
--- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
+++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
@@ -78,6 +78,10 @@ call.
   - ``src_change``
   - Event data for event V4L2_EVENT_SOURCE_CHANGE.
 * -
+  - struct :c:type:`v4l2_event_buf_queued`
+  - ``buf_queued``
+  - Event data for event V4L2_EVENT_BUF_QUEUED.
+* -
   - __u8
   - ``data``\ [64]
   - Event data. Defined by the event type. The union should be used to
@@ -337,6 +341,25 @@ call.
each cell in the motion detection grid, then that all cells are
automatically assigned to the default region 0.
 
+.. c:type:: v4l2_event_buf_queued
+
+.. flat-table:: struct v4l2_event_buf_queued
+:header-rows:  0
+:stub-columns: 0
+:widths:   1 1 2
+
+* - __u32
+  - ``index``
+  - The index of the buffer that was queued to the driver.
+* - __s32
+  - ``out_fence_fd``
+  - The out-fence file descriptor of the buffer that was queued to
+   the driver. It will signal when the buffer is ready, or if an
+   error happens.
+
+
+
+.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}|
 
 
 .. tabularcolumns:: |p{6.6cm}|p{2.2cm}|p{8.7cm}|
diff --git a/Documentation/media/videodev2.h.rst.exceptions 
b/Documentation/media/videodev2.h.rst.exceptions
index a5cb0a8686ac..4e014b1d0317 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -462,6 +462,7 @@ replace define V4L2_EVENT_CTRL event-type
 replace define V4L2_EVENT_FRAME_SYNC event-type
 replace define V4L2_EVENT_SOURCE_CHANGE event-type
 replace define V4L2_EVENT_MOTION_DET event-type
+replace define V4L2_EVENT_BUF_QUEUED event-type
 replace define V4L2_EVENT_PRIVATE_START event-type
 
 replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index e5abab9a908c..e2ec0b66f490 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -2158,6 +2158,7 @@ struct v4l2_streamparm {
 #define V4L2_EVENT_FRAME_SYNC  4
 #define V4L2_EVENT_SOURCE_CHANGE   5
 #define V4L2_EVENT_MOTION_DET  6
+#define V4L2_EVENT_BUF_QUEUED  7
 #define V4L2_EVENT_PRIVATE_START   0x0800
 
 /* Payload for V4L2_EVENT_VSYNC */
@@ -2210,6 +2211,15 @@ struct v4l2_event_motion_det {
__u32 region_mask;
 };
 
+/**
+ * struct v4l2_event_buf_queued - buffer queued in the driver event
+ * @index: index of the buffer queued in the driver
+ */
+struct v4l2_event_buf_queued {
+   __u32 index;
+   __s32 out_fence_fd;
+};
+
 struct v4l2_event {
__u32   type;
union {
@@ -2218,6 +2228,7 @@ struct v4l2_event {
struct v4l2_event_frame_syncframe_sync;
struct v4l2_event_src_changesrc_change;
struct v4l2_event_motion_detmotion_det;
+   struct v4l2_event_buf_queuedbuf_queued;
__u8data[64];
} u;
__u32   pending;
-- 
2.13.5



[PATCH v3 06/15] [media] vivid: assign the specific device to the vb2_queue->dev

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

Instead of assigning the global v4l2 device, assign the specific device.
This was causing trouble when using using V4L2 events with vivid
devices. The device's queue should be the same we opened in userspace.

This is needed for the upcoming V4L2_EVENT_BUF_QUEUED support. The current
vivid code isn't wrong, it just needs to be changed so V4L2_EVENT_BUF_QUEUED
can be supported. The change doesn't affect any other behaviour of vivid.

Signed-off-by: Gustavo Padovan 
Acked-by: Hans Verkuil 
---
 drivers/media/platform/vivid/vivid-core.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/vivid/vivid-core.c 
b/drivers/media/platform/vivid/vivid-core.c
index 5f316a5e38db..608bcceed463 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -1070,7 +1070,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
q->lock = >mutex;
-   q->dev = dev->v4l2_dev.dev;
+   q->dev = >vid_cap_dev.dev;
 
ret = vb2_queue_init(q);
if (ret)
@@ -1090,7 +1090,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
q->lock = >mutex;
-   q->dev = dev->v4l2_dev.dev;
+   q->dev = >vid_out_dev.dev;
 
ret = vb2_queue_init(q);
if (ret)
@@ -1110,7 +1110,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
q->lock = >mutex;
-   q->dev = dev->v4l2_dev.dev;
+   q->dev = >vbi_cap_dev.dev;
 
ret = vb2_queue_init(q);
if (ret)
@@ -1130,7 +1130,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 2;
q->lock = >mutex;
-   q->dev = dev->v4l2_dev.dev;
+   q->dev = >vbi_out_dev.dev;
 
ret = vb2_queue_init(q);
if (ret)
@@ -1149,7 +1149,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
q->min_buffers_needed = 8;
q->lock = >mutex;
-   q->dev = dev->v4l2_dev.dev;
+   q->dev = >sdr_cap_dev.dev;
 
ret = vb2_queue_init(q);
if (ret)
-- 
2.13.5



[PATCH v3 09/15] [media] v4l: add support to BUF_QUEUED event

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

Implement the needed pieces to let userspace subscribe for
V4L2_EVENT_BUF_QUEUED events. Videobuf2 will queue the event for the
DQEVENT ioctl.

v3: - Do not call v4l2 event API from vb2 (Mauro)

v2: - Use VIDEO_MAX_FRAME to allocate room for events at
v4l2_event_subscribe() (Hans)

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/v4l2-ctrls.c |  6 +-
 drivers/media/v4l2-core/videobuf2-core.c |  2 ++
 drivers/media/v4l2-core/videobuf2-v4l2.c | 14 ++
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index dd1db678718c..17d4b9e3eec6 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -3438,8 +3438,12 @@ EXPORT_SYMBOL(v4l2_ctrl_log_status);
 int v4l2_ctrl_subscribe_event(struct v4l2_fh *fh,
const struct v4l2_event_subscription *sub)
 {
-   if (sub->type == V4L2_EVENT_CTRL)
+   switch (sub->type) {
+   case V4L2_EVENT_CTRL:
return v4l2_event_subscribe(fh, sub, 0, _ctrl_sub_ev_ops);
+   case V4L2_EVENT_BUF_QUEUED:
+   return v4l2_event_subscribe(fh, sub, VIDEO_MAX_FRAME, NULL);
+   }
return -EINVAL;
 }
 EXPORT_SYMBOL(v4l2_ctrl_subscribe_event);
diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index b19c1bc4b083..bbbae0eed567 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1231,6 +1231,8 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
trace_vb2_buf_queue(q, vb);
 
call_void_vb_qop(vb, buf_queue, vb);
+
+   call_void_bufop(q, buffer_queued, vb);
 }
 
 static int __buf_prepare(struct vb2_buffer *vb, const void *pb)
diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c 
b/drivers/media/v4l2-core/videobuf2-v4l2.c
index 8c322cd1b346..bbfcd054e6f6 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -138,6 +138,19 @@ static void __copy_timestamp(struct vb2_buffer *vb, const 
void *pb)
}
 };
 
+static void __buffer_queued(struct vb2_buffer *vb)
+{
+   struct video_device *vdev = to_video_device(vb->vb2_queue->dev);
+   struct v4l2_fh *fh = vdev->queue->owner;
+   struct v4l2_event event;
+
+   memset(, 0, sizeof(event));
+   event.type = V4L2_EVENT_BUF_QUEUED;
+   event.u.buf_queued.index = vb->index;
+
+   v4l2_event_queue_fh(fh, );
+}
+
 static void vb2_warn_zero_bytesused(struct vb2_buffer *vb)
 {
static bool check_once;
@@ -455,6 +468,7 @@ static const struct vb2_buf_ops v4l2_buf_ops = {
.fill_user_buffer   = __fill_v4l2_buffer,
.fill_vb2_buffer= __fill_vb2_buffer,
.copy_timestamp = __copy_timestamp,
+   .buffer_queued  = __buffer_queued,
 };
 
 /**
-- 
2.13.5



[PATCH v3 11/15] [media] vivid: mark vivid queues as ordered

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

To enable vivid to be used with explicit synchronization we need
to mark its queues as ordered. vivid queues are already ordered by
default so we no changes are needed.

Signed-off-by: Gustavo Padovan 
Acked-by: Hans Verkuil 
---
 drivers/media/platform/vivid/vivid-core.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/media/platform/vivid/vivid-core.c 
b/drivers/media/platform/vivid/vivid-core.c
index 608bcceed463..239790e8ccc6 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -1063,6 +1063,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
q->type = dev->multiplanar ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE 
:
V4L2_BUF_TYPE_VIDEO_CAPTURE;
q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_READ;
+   q->ordered = 1;
q->drv_priv = dev;
q->buf_struct_size = sizeof(struct vivid_buffer);
q->ops = _vid_cap_qops;
@@ -1083,6 +1084,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
q->type = dev->multiplanar ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE :
V4L2_BUF_TYPE_VIDEO_OUTPUT;
q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_WRITE;
+   q->ordered = 1;
q->drv_priv = dev;
q->buf_struct_size = sizeof(struct vivid_buffer);
q->ops = _vid_out_qops;
@@ -1103,6 +1105,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
q->type = dev->has_raw_vbi_cap ? V4L2_BUF_TYPE_VBI_CAPTURE :
  V4L2_BUF_TYPE_SLICED_VBI_CAPTURE;
q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_READ;
+   q->ordered = 1;
q->drv_priv = dev;
q->buf_struct_size = sizeof(struct vivid_buffer);
q->ops = _vbi_cap_qops;
@@ -1123,6 +1126,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
q->type = dev->has_raw_vbi_out ? V4L2_BUF_TYPE_VBI_OUTPUT :
  V4L2_BUF_TYPE_SLICED_VBI_OUTPUT;
q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_WRITE;
+   q->ordered = 1;
q->drv_priv = dev;
q->buf_struct_size = sizeof(struct vivid_buffer);
q->ops = _vbi_out_qops;
@@ -1142,6 +1146,7 @@ static int vivid_create_instance(struct platform_device 
*pdev, int inst)
q = >vb_sdr_cap_q;
q->type = V4L2_BUF_TYPE_SDR_CAPTURE;
q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_READ;
+   q->ordered = 1;
q->drv_priv = dev;
q->buf_struct_size = sizeof(struct vivid_buffer);
q->ops = _sdr_cap_qops;
-- 
2.13.5



[PATCH v3 12/15] [media] vb2: add videobuf2 dma-buf fence helpers

2017-09-07 Thread Gustavo Padovan
From: Javier Martinez Canillas 

Add a videobuf2-fence.h header file that contains different helpers
for DMA buffer sharing explicit fence support in videobuf2.

Signed-off-by: Javier Martinez Canillas 
Signed-off-by: Gustavo Padovan 
---
 include/media/videobuf2-fence.h | 49 +
 1 file changed, 49 insertions(+)
 create mode 100644 include/media/videobuf2-fence.h

diff --git a/include/media/videobuf2-fence.h b/include/media/videobuf2-fence.h
new file mode 100644
index ..ed5612ca03d6
--- /dev/null
+++ b/include/media/videobuf2-fence.h
@@ -0,0 +1,49 @@
+/*
+ * videobuf2-fence.h - DMA buffer sharing fence helpers for videobuf 2
+ *
+ * Copyright (C) 2016 Samsung Electronics
+ *
+ * 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.
+ */
+
+#include 
+#include 
+
+static DEFINE_SPINLOCK(vb2_fence_lock);
+
+static inline const char *vb2_fence_get_driver_name(struct dma_fence *fence)
+{
+   return "vb2_fence";
+}
+
+static inline const char *vb2_fence_get_timeline_name(struct dma_fence *fence)
+{
+   return "vb2_fence_timeline";
+}
+
+static inline bool vb2_fence_enable_signaling(struct dma_fence *fence)
+{
+   return true;
+}
+
+static const struct dma_fence_ops vb2_fence_ops = {
+   .get_driver_name = vb2_fence_get_driver_name,
+   .get_timeline_name = vb2_fence_get_timeline_name,
+   .enable_signaling = vb2_fence_enable_signaling,
+   .wait = dma_fence_default_wait,
+};
+
+static inline struct dma_fence *vb2_fence_alloc(void)
+{
+   struct dma_fence *vb2_fence = kzalloc(sizeof(*vb2_fence), GFP_KERNEL);
+
+   if (!vb2_fence)
+   return NULL;
+
+   dma_fence_init(vb2_fence, _fence_ops, _fence_lock,
+  dma_fence_context_alloc(1), 1);
+
+   return vb2_fence;
+}
-- 
2.13.5



[PATCH v3 14/15] fs/files: export close_fd() symbol

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

Rename __close_fd() to close_fd() and export it to be able close files
in modules using file descriptors.

The usecase that motivates this change happens in V4L2 where we send
events to userspace with a fd that has file installed in it. But if for
some reason we have to cancel the video stream we need to close the files
that haven't been shared with userspace yet. Thus the export of
close_fd() becomes necessary.

fd_install() happens when we call an ioctl to queue a buffer, but we only
share the fd with userspace later, and that may happen in a kernel thread
instead.

Cc: Alexander Viro 
Cc: linux-fsde...@vger.kernel.org
Cc: Riley Andrews 
Signed-off-by: Gustavo Padovan 
---
This is more like a question, I don't know how unhappy people will be with
this proposal to expose close_fd(). I'm all ears for more interesting
ways of doing it!
---
 drivers/android/binder.c | 2 +-
 fs/file.c| 5 +++--
 fs/open.c| 2 +-
 include/linux/fdtable.h  | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index f7665c31feca..5a9bc73012df 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -440,7 +440,7 @@ static long task_close_fd(struct binder_proc *proc, 
unsigned int fd)
if (proc->files == NULL)
return -ESRCH;
 
-   retval = __close_fd(proc->files, fd);
+   retval = close_fd(proc->files, fd);
/* can't restart close syscall because file table entry was cleared */
if (unlikely(retval == -ERESTARTSYS ||
 retval == -ERESTARTNOINTR ||
diff --git a/fs/file.c b/fs/file.c
index 1fc7fbbb4510..111d387ac190 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -618,7 +618,7 @@ EXPORT_SYMBOL(fd_install);
 /*
  * The same warnings as for __alloc_fd()/__fd_install() apply here...
  */
-int __close_fd(struct files_struct *files, unsigned fd)
+int close_fd(struct files_struct *files, unsigned fd)
 {
struct file *file;
struct fdtable *fdt;
@@ -640,6 +640,7 @@ int __close_fd(struct files_struct *files, unsigned fd)
spin_unlock(>file_lock);
return -EBADF;
 }
+EXPORT_SYMBOL(close_fd);
 
 void do_close_on_exec(struct files_struct *files)
 {
@@ -856,7 +857,7 @@ int replace_fd(unsigned fd, struct file *file, unsigned 
flags)
struct files_struct *files = current->files;
 
if (!file)
-   return __close_fd(files, fd);
+   return close_fd(files, fd);
 
if (fd >= rlimit(RLIMIT_NOFILE))
return -EBADF;
diff --git a/fs/open.c b/fs/open.c
index 35bb784763a4..30907d967443 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1152,7 +1152,7 @@ EXPORT_SYMBOL(filp_close);
  */
 SYSCALL_DEFINE1(close, unsigned int, fd)
 {
-   int retval = __close_fd(current->files, fd);
+   int retval = close_fd(current->files, fd);
 
/* can't restart close syscall because file table entry was cleared */
if (unlikely(retval == -ERESTARTSYS ||
diff --git a/include/linux/fdtable.h b/include/linux/fdtable.h
index 6e84b2cae6ad..511fd38d5e4b 100644
--- a/include/linux/fdtable.h
+++ b/include/linux/fdtable.h
@@ -115,7 +115,7 @@ extern int __alloc_fd(struct files_struct *files,
  unsigned start, unsigned end, unsigned flags);
 extern void __fd_install(struct files_struct *files,
  unsigned int fd, struct file *file);
-extern int __close_fd(struct files_struct *files,
+extern int close_fd(struct files_struct *files,
  unsigned int fd);
 
 extern struct kmem_cache *files_cachep;
-- 
2.13.5



[PATCH v3 13/15] [media] vb2: add infrastructure to support out-fences

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

Add vb2_setup_out_fence() and the needed members to struct vb2_buffer.

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/videobuf2-core.c | 55 
 include/media/videobuf2-core.h   | 34 
 2 files changed, 89 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index bbbae0eed567..34adf1916194 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -23,8 +23,11 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include 
+#include 
 #include 
 
 #include 
@@ -1317,6 +1320,58 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned 
int index, void *pb)
 }
 EXPORT_SYMBOL_GPL(vb2_core_prepare_buf);
 
+int vb2_setup_out_fence(struct vb2_queue *q)
+{
+   struct vb2_fence *fence;
+
+   fence = kzalloc(sizeof(*fence), GFP_KERNEL);
+   if (!fence)
+   return -ENOMEM;
+
+   fence->out_fence_fd = get_unused_fd_flags(O_CLOEXEC);
+   if (fence->out_fence_fd < 0) {
+   kfree(fence);
+   return fence->out_fence_fd;
+   }
+
+   fence->out_fence = vb2_fence_alloc();
+   if (!fence->out_fence)
+   goto err_fence;
+
+   fence->sync_file = sync_file_create(fence->out_fence);
+   if (!fence->sync_file) {
+   dma_fence_put(fence->out_fence);
+   goto err_fence;
+   }
+
+   spin_lock(>out_fence_lock);
+   list_add_tail(>entry, >out_fence_list);
+   spin_unlock(>out_fence_lock);
+
+   return 0;
+
+err_fence:
+   kfree(fence);
+   put_unused_fd(fence->out_fence_fd);
+   return -ENOMEM;
+}
+EXPORT_SYMBOL_GPL(vb2_setup_out_fence);
+
+void vb2_cleanup_out_fence(struct vb2_queue *q)
+{
+   struct vb2_fence *fence;
+
+   spin_lock(>out_fence_lock);
+   fence = list_last_entry(>out_fence_list,
+   struct vb2_fence, entry);
+   put_unused_fd(fence->out_fence_fd);
+   fput(fence->sync_file->file);
+   list_del(>entry);
+   spin_unlock(>out_fence_lock);
+   kfree(fence);
+}
+EXPORT_SYMBOL_GPL(vb2_cleanup_out_fence);
+
 /**
  * vb2_start_streaming() - Attempt to start streaming.
  * @q: videobuf2 queue
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 20099dc22f26..84e5e7216a1e 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -427,6 +427,24 @@ struct vb2_buf_ops {
void (*buffer_queued)(struct vb2_buffer *vb);
 };
 
+/*
+ * struct vb2_fence - storage for fence data before queueing to the driver.
+ *
+ * @out_fence_fd:  the fd where to install the sync_file
+ * @out_fence: the fence associated to the sync_file
+ * @sync_file: the sync_file to be shared with userspace via the
+ * out_fence_fd
+ * @files: stores files struct for cleanup purposes
+ * @entry: the list head element for the out_fence_list
+ */
+struct vb2_fence {
+   int out_fence_fd;
+   struct dma_fence *out_fence;
+   struct sync_file *sync_file;
+   struct files_struct *files;
+   struct list_head entry;
+};
+
 /**
  * struct vb2_queue - a videobuf queue
  *
@@ -734,6 +752,22 @@ int vb2_core_create_bufs(struct vb2_queue *q, enum 
vb2_memory memory,
 int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb);
 
 /**
+ * vb2_setup_out_fence() - setup new out-fence
+ * @q: The vb2_queue where to setup it
+ *
+ * Setup the file descriptor, the fence and the sync_file for the next
+ * buffer to be queued and add everything to the tail of the q->out_fence_list.
+ */
+int vb2_setup_out_fence(struct vb2_queue *q);
+
+/**
+ * vb2_cleanup_out_fence() - cleanup out-fence
+ * @q: The vb2_queue to use for cleanup
+ *
+ * Clean up the last fence on the list. Used only when QBUF fails.
+ */
+void vb2_cleanup_out_fence(struct vb2_queue *q);
+/**
  * vb2_core_qbuf() - Queue a buffer from userspace
  *
  * @q: videobuf2 queue
-- 
2.13.5



[PATCH v3 15/15] [media] vb2: add out-fence support to QBUF

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

If V4L2_BUF_FLAG_OUT_FENCE flag is present on the QBUF call we create
an out_fence and sent to userspace on the V4L2_EVENT_BUF_QUEUED when
the buffer is queued to the driver.

The out fence fd returned references the next buffer to be queued to the
driver and not the buffer in the actual QBUF call. So there a list in
videobuf2 core to keep track of the sync_file and fence created and assign
them to buffers when they are queued to the V4L2 driver.

The fence is signaled on buffer_done(), when the job on the buffer is
finished.

v4:
- return the out_fence_fd in the BUF_QUEUED event(Hans)

v3: - add WARN_ON_ONCE(q->ordered) on requeueing (Hans)
- set the OUT_FENCE flag if there is a fence pending (Hans)
- call fd_install() after vb2_core_qbuf() (Hans)
- clean up fence if vb2_core_qbuf() fails (Hans)
- add list to store sync_file and fence for the next queued buffer

v2: check if the queue is ordered.

Signed-off-by: Gustavo Padovan 
---
 drivers/media/v4l2-core/videobuf2-core.c | 51 
 drivers/media/v4l2-core/videobuf2-v4l2.c | 24 ++-
 include/media/videobuf2-core.h   | 11 +++
 3 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 34adf1916194..ab58170776bc 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -353,6 +354,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum 
vb2_memory memory,
vb->planes[plane].length = plane_sizes[plane];
vb->planes[plane].min_length = plane_sizes[plane];
}
+   vb->out_fence_fd = -1;
q->bufs[vb->index] = vb;
 
/* Allocate video buffer memory for the MMAP type */
@@ -933,10 +935,24 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
case VB2_BUF_STATE_QUEUED:
return;
case VB2_BUF_STATE_REQUEUEING:
+   /*
+* Explicit synchronization requires ordered queues for now,
+* so WARN_ON if we are requeuing on an ordered queue.
+*/
+   if (vb->out_fence)
+   WARN_ON_ONCE(q->ordered);
+
if (q->start_streaming_called)
__enqueue_in_driver(vb);
return;
default:
+   if (state == VB2_BUF_STATE_ERROR)
+   dma_fence_set_error(vb->out_fence, -ENOENT);
+   dma_fence_signal(vb->out_fence);
+   dma_fence_put(vb->out_fence);
+   vb->out_fence = NULL;
+   vb->out_fence_fd = -1;
+
/* Inform any processes that may be waiting for buffers */
wake_up(>done_wq);
break;
@@ -1224,10 +1240,21 @@ static int __prepare_dmabuf(struct vb2_buffer *vb, 
const void *pb)
 static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb->vb2_queue;
+   struct vb2_fence *fence;
 
if (vb->in_fence && !dma_fence_is_signaled(vb->in_fence))
return;
 
+   spin_lock(>out_fence_lock);
+   fence = list_first_entry(>out_fence_list, struct vb2_fence, entry);
+   if (fence) {
+   vb->out_fence = dma_fence_get(fence->out_fence);
+   vb->out_fence_fd = fence->out_fence_fd;
+   list_del(>entry);
+   kfree(fence);
+   }
+   spin_unlock(>out_fence_lock);
+
vb->state = VB2_BUF_STATE_ACTIVE;
atomic_inc(>owned_by_drv_count);
 
@@ -1348,6 +1375,8 @@ int vb2_setup_out_fence(struct vb2_queue *q)
list_add_tail(>entry, >out_fence_list);
spin_unlock(>out_fence_lock);
 
+   fence->files = current->files;
+
return 0;
 
 err_fence:
@@ -1450,6 +1479,7 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
index, void *pb,
  struct dma_fence *fence)
 {
struct vb2_buffer *vb;
+   struct vb2_fence *vb2_fence;
int ret;
 
vb = q->bufs[index];
@@ -1513,6 +1543,15 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int 
index, void *pb,
goto err;
}
 
+   spin_lock(>out_fence_lock);
+   vb2_fence = list_last_entry(>out_fence_list, struct vb2_fence,
+entry);
+   spin_unlock(>out_fence_lock);
+   if (vb2_fence)
+   fd_install(vb2_fence->out_fence_fd,
+  vb2_fence->sync_file->file);
+
+
/*
 * For explicit synchronization: If the fence didn't signal
 * yet we setup a callback to queue the buffer once the fence
@@ -1760,6 +1799,7 @@ static void 

[PATCH v3 08/15] [media] vb2: add .buffer_queued() to notify queueing in the driver

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

With the upcoming explicit synchronization support to V4L2 we need a
way to notify userspace when buffers are queued to the driver - buffers
with fences attached to it can only be queued once the fence signal, so
the queueing to the driver might be deferred.

Yet, userspace still wants to be notified, so the buffer_queued() callback
was added to vb2_buf_ops for that purpose.

Signed-off-by: Gustavo Padovan 
---
 include/media/videobuf2-core.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 41cda762ff0a..5ed8d3402474 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -415,6 +415,8 @@ struct vb2_ops {
  * will return an error.
  * @copy_timestamp:copy the timestamp from a userspace structure to
  * the vb2_buffer struct.
+ * @buffer_queued: VB2 uses this to notify the VB2-client that the buffer
+ * was queued to the driver.
  */
 struct vb2_buf_ops {
int (*verify_planes_array)(struct vb2_buffer *vb, const void *pb);
@@ -422,6 +424,7 @@ struct vb2_buf_ops {
int (*fill_vb2_buffer)(struct vb2_buffer *vb, const void *pb,
struct vb2_plane *planes);
void (*copy_timestamp)(struct vb2_buffer *vb, const void *pb);
+   void (*buffer_queued)(struct vb2_buffer *vb);
 };
 
 /**
-- 
2.13.5



[PATCH v3 01/15] [media] v4l: Document explicit synchronization behaviour

2017-09-07 Thread Gustavo Padovan
From: Gustavo Padovan 

Add section to VIDIOC_QBUF about it

v2:
- mention that fences are files (Hans)
- rework for the new API

Signed-off-by: Gustavo Padovan 
---
 Documentation/media/uapi/v4l/vidioc-qbuf.rst | 31 
 1 file changed, 31 insertions(+)

diff --git a/Documentation/media/uapi/v4l/vidioc-qbuf.rst 
b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
index 1f3612637200..fae0b1431672 100644
--- a/Documentation/media/uapi/v4l/vidioc-qbuf.rst
+++ b/Documentation/media/uapi/v4l/vidioc-qbuf.rst
@@ -117,6 +117,37 @@ immediately with an ``EAGAIN`` error code when no buffer 
is available.
 The struct :c:type:`v4l2_buffer` structure is specified in
 :ref:`buffer`.
 
+Explicit Synchronization
+
+
+Explicit Synchronization allows us to control the synchronization of
+shared buffers from userspace by passing fences to the kernel and/or
+receiving them from it. Fences passed to the kernel are named in-fences and
+the kernel should wait them to signal before using the buffer, i.e., queueing
+it to the driver. On the other side, the kernel can create out-fences for the
+buffers it queues to the drivers, out-fences signal when the driver is
+finished with buffer, that is the buffer is ready. The fence are represented
+by file and passed as file descriptor to userspace.
+
+The in-fences are communicated to the kernel at the ``VIDIOC_QBUF`` ioctl
+using the ``V4L2_BUF_FLAG_IN_FENCE`` buffer
+flags and the `fence_fd` field. If an in-fence needs to be passed to the 
kernel,
+`fence_fd` should be set to the fence file descriptor number and the
+``V4L2_BUF_FLAG_IN_FENCE`` should be set as well. Failure to set both will
+cause ``VIDIOC_QBUF`` to return with error.
+
+To get a out-fence back from V4L2 the ``V4L2_BUF_FLAG_OUT_FENCE`` flag should
+be set to notify it that the next queued buffer should have a fence attached to
+it. That means the out-fence may not be associated with the buffer in the
+current ``VIDIOC_QBUF`` ioctl call because the ordering in which videobuf2 core
+queues the buffers to the drivers can't be guaranteed. To become aware of the
+of the next queued buffer and the out-fence attached to it the
+``V4L2_EVENT_BUF_QUEUED`` event should be used. It will trigger an event
+for every buffer queued to the V4L2 driver.
+
+At streamoff the out-fences will either signal normally if the drivers wait
+for the operations on the buffers to finish or signal with error if the
+driver cancel the pending operations.
 
 Return Value
 
-- 
2.13.5



[PATCH] Staging: atomisp: fix alloc_cast.cocci warnings

2017-09-07 Thread Branislav Radocaj
Remove casting the values returned by memory allocation functions
like kmalloc, kzalloc, kmem_cache_alloc, kmem_cache_zalloc etc.

Semantic patch information:
This makes an effort to find cases of casting of values returned by
kmalloc, kzalloc, kcalloc, kmem_cache_alloc, kmem_cache_zalloc,
kmem_cache_alloc_node, kmalloc_node and kzalloc_node and removes
the casting as it is not required. The result in the patch case may
need some reformatting.

Generated by: scripts/coccinelle/api/alloc/alloc_cast.cocci

Signed-off-by: Branislav Radocaj 
---
 drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
index eecd8cf71951..56765d6a0498 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
@@ -133,7 +133,7 @@ sh_css_load_blob_info(const char *fw, const struct 
ia_css_fw_info *bi, struct ia
char *namebuffer;
int namelength = (int)strlen(name);
 
-   namebuffer = (char *) kmalloc(namelength + 1, GFP_KERNEL);
+   namebuffer = kmalloc(namelength + 1, GFP_KERNEL);
if (namebuffer == NULL)
return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
 
@@ -149,7 +149,7 @@ sh_css_load_blob_info(const char *fw, const struct 
ia_css_fw_info *bi, struct ia
size_t configstruct_size = sizeof(struct 
ia_css_config_memory_offsets);
size_t statestruct_size = sizeof(struct 
ia_css_state_memory_offsets);
 
-   char *parambuf = (char *)kmalloc(paramstruct_size + 
configstruct_size + statestruct_size,
+   char *parambuf = kmalloc(paramstruct_size + configstruct_size + 
statestruct_size,
GFP_KERNEL);
if (parambuf == NULL)
return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
-- 
2.11.0



dw2102 kernel oops

2017-09-07 Thread Sebastian Schmachtel
Hi,

I recently bought a Tevii S650 USB DVB-S2 device (dw2102 driver). But
when i connect it, my Desktop seems to get some HID inputs (some random
keys are typed in a terminal). Pretty strange, but I set
disable_rc_polling=1 for dvb_usb to get rid of this problem as the
pattern reminded me of a Remote.
But now i get oops (tried on two different linux systems). I verified
the Hardware on Windows 7 and it works fine... Currently i'm using
4.12.0-1-amd64 (debian testing), but I checked everything
4.9-4.12(Debian) and even 4.13 (vanilla) has the same problems.

Can anyone help me, or has some suggestions how to further debug?

[   11.106776] dvb-usb: found a 'TeVii S650 USB2.0' in warm state.
[   11.106830] dvb-usb: will pass the complete MPEG2 transport stream to
the software demuxer.
[   11.106847] dvbdev: DVB: registering new adapter (TeVii S650 USB2.0)
[   11.106850] dw2102: read eeprom failed.
[   11.106909] dvb-usb: MAC address reading failed.
[   11.126813] [drm] ring test on 5 succeeded in 2 usecs
[   11.126823] [drm] UVD initialized successfully.
[   11.131852] Invalid probe, probably not a CX24116 device
[   11.144435] Invalid probe, probably not a DS3000
[   11.144495] dvb-usb: no frontend was attached by 'TeVii S650 USB2.0'
[   11.144631] dvb-usb: TeVii S650 USB2.0 successfully initialized and
connected.
[   11.144676] usbcore: registered new interface driver dw2102
[   11.144724] BUG: unable to handle kernel NULL pointer dereference at
0050
[   11.144794] IP: dw2102_disconnect+0x1a/0x70 [dvb_usb_dw2102]
[   11.144852] PGD 0
[   11.144852] P4D 0

[   11.145006] Oops:  [#1] SMP
[   11.145061] Modules linked in: ds3000 cx24116 xfs amdkfd
snd_hda_codec_realtek snd_hda_codec_generic radeon(+) snd_hda_codec_hdmi
btusb btrtl btbcm btintel snd_hda_intel bluetooth snd_hda_codec
dvb_usb_dw2102 edac_mce_amd dvb_usb snd_hda_core ttm snd_hwdep dvb_core
snd_pcm_oss evdev snd_mixer_oss rc_core joydev ecdh_generic
drm_kms_helper kvm_amd snd_pcm rfkill kvm drm ccp snd_timer snd
irqbypass sg rng_core soundcore i2c_algo_bit pcspkr shpchp sp5100_tco
wmi button acpi_cpufreq loop firewire_sbp2 firewire_core crc_itu_t
parport_pc ppdev lp sunrpc parport ip_tables x_tables autofs4 ext4 crc16
jbd2 fscrypto ecb mbcache algif_skcipher af_alg dm_crypt dm_mod raid10
raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor
hid_logitech_hidpp hid_logitech_dj raid6_pq libcrc32c crc32c_generic
[   11.145353]  raid0 multipath linear raid1 hid_generic md_mod usbhid
hid sd_mod uas usb_storage crct10dif_pclmul crc32_pclmul crc32c_intel
ghash_clmulni_intel pcbc aesni_intel aes_x86_64 crypto_simd glue_helper
cryptd ahci xhci_pci libahci i2c_piix4 xhci_hcd libata r8169 mii usbcore
scsi_mod usb_common gpio_amdpt gpio_generic i2c_designware_platform
i2c_designware_core
[   11.145511] CPU: 0 PID: 57 Comm: kworker/0:1 Not tainted
4.12.0-1-amd64 #1 Debian 4.12.6-1
[   11.145582] Hardware name: MSI MS-7A37/B350M MORTAR (MS-7A37), BIOS
1.55 06/22/2017
[   11.145658] Workqueue: usb_hub_wq hub_event [usbcore]
[   11.145720] task: 8c220c2c9000 task.stack: b75841bb4000
[   11.145783] RIP: 0010:dw2102_disconnect+0x1a/0x70 [dvb_usb_dw2102]
[   11.145844] RSP: 0018:b75841bb7c28 EFLAGS: 00010246
[   11.145904] RAX: 8c2209884000 RBX: 8c2206767c30 RCX:

[   11.145968] RDX:  RSI: 8c2206767c00 RDI:
8c2206767c00
[   11.146031] RBP: 8c2206767c00 R08:  R09:

[   11.146095] R10: 8c220dc08240 R11: 0018 R12:

[   11.146158] R13: 8c2206fd1000 R14: 8c2206fd1098 R15:
ffed
[   11.146222] FS:  () GS:8c220e60()
knlGS:
[   11.146292] CS:  0010 DS:  ES:  CR0: 80050033
[   11.146352] CR2: 0050 CR3: 000408af3000 CR4:
003406f0
[   11.146416] Call Trace:
[   11.146477]  ? usb_unbind_interface+0x71/0x270 [usbcore]
[   11.146541]  ? device_release_driver_internal+0x154/0x210
[   11.146602]  ? bus_remove_device+0xf5/0x160
[   11.146659]  ? device_del+0x1dc/0x310
[   11.146720]  ? usb_disable_device+0x93/0x250 [usbcore]
[   11.146785]  ? usb_disconnect+0x90/0x270 [usbcore]
[   11.146848]  ? hub_event+0x1d9/0x14d0 [usbcore]
[   11.146907]  ? process_one_work+0x181/0x370
[   11.146964]  ? worker_thread+0x4d/0x3a0
[   11.147021]  ? kthread+0xfc/0x130
[   11.147075]  ? process_one_work+0x370/0x370
[   11.147132]  ? kthread_create_on_node+0x70/0x70
[   11.147191]  ? ret_from_fork+0x25/0x30
[   11.147246] Code: 00 0f 1f 44 00 00 b8 01 00 00 00 c3 0f 1f 44 00 00
0f 1f 44 00 00 41 54 55 48 89 fd 53 48 8b 87 c8 00 00 00 4c 8b a0 d0 27
00 00 <49> 8b 5c 24 50 48 85 db 74 18 48 8b 83 a8 00 00 00 48 8b 78 10
[   11.147366] RIP: dw2102_disconnect+0x1a/0x70 [dvb_usb_dw2102] RSP:
b75841bb7c28
[   11.147433] CR2: 0050
[   11.147489] ---[ end trace 9643f635d6b5f94e ]---

Sebastian


Re: [PATCH] em28xx: add support for Hauppauge WinTV-dualHD DVB tuner

2017-09-07 Thread Mauro Carvalho Chehab
Hi Brad,

Em Wed, 31 May 2017 15:01:00 -0500
Brad Love  escreveu:

> Christian et al,
> 
> I am an engineer at Hauppauge. This repo is the staging area for all the
> patches I am testing, with the intention of getting them upstreamed. I
> will be inaccessible for the next 18 days however, so I will not be able
> to put any effort until I get back.

Any news on such patchset?

On a side note, I took a quick look on some of the patches at the
git repository at:


https://github.com/b-rad-NDi/Ubuntu-media-tree-kernel-builder/tree/master/patches/ubuntu-zesty-4.10.0/extra

I suspect that some of the patches there could have some side effect on
existing drivers, like this one that unconditionally changes the size
of URB:


https://github.com/b-rad-NDi/Ubuntu-media-tree-kernel-builder/blob/master/patches/ubuntu-zesty-4.10.0/extra/0003-em28xx-usb-packet-size-tweaks.patch

So, it would be good to be able to test this set also with older
em28xx devices that also support bulk transfers.

Thanks,
Mauro


Re: [PATCH v3 2/3] leds: as3645a: Add LED flash class driver

2017-09-07 Thread Sakari Ailus
On Thu, Sep 07, 2017 at 02:50:27PM +0200, Pavel Machek wrote:
> Hi!
> 
> > On Mon, Aug 28, 2017 at 01:04:51PM +0200, Pavel Machek wrote:
> > > On Wed 2017-08-23 11:10:59, Sakari Ailus wrote:
> > > > Add a LED flash class driver for the as3654a flash controller. A V4L2 
> > > > flash
> > > > driver for it already exists (drivers/media/i2c/as3645a.c), and this 
> > > > driver
> > > > is based on that.
> > > 
> > > We do not want to have two drivers for same hardware... how is that
> > > supposed to work?
> > 
> > No, we don't. The intent is to remove the other driver later on, as it's
> > implemented as a V4L2 sub-device driver. It also lacks DT support.
> 
> Could we perhaps remove the driver with the same patch that merges
> this?

This patch is already merged, but I can submit another patch removing the
other driver.

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


[GIT FIXES FOR v4.14] a800 probe fix

2017-09-07 Thread Sean Young
Hi Mauro,

This is a single fix for doing dma from the stack. After this, the a800 works
again.

Thanks,
Sean

--
The following changes since commit 1efdf1776e2253b77413c997bed862410e4b6aaf:

  media: leds: as3645a: add V4L2_FLASH_LED_CLASS dependency (2017-09-05 
16:32:45 -0400)

are available in the git repository at:

  git://linuxtv.org/syoung/media_tree.git for-v4.14d

for you to fetch changes up to c55f09661483373cb55b8c67a22ca3371167214c:

  media: dvb: i2c transfers over usb cannot be done from stack (2017-09-07 
12:59:01 +0100)


Sean Young (1):
  media: dvb: i2c transfers over usb cannot be done from stack

 drivers/media/dvb-frontends/dib3000mc.c | 50 ++--
 drivers/media/dvb-frontends/dvb-pll.c   | 22 +---
 drivers/media/tuners/mt2060.c   | 59 ++---
 3 files changed, 103 insertions(+), 28 deletions(-)

diff --git a/drivers/media/dvb-frontends/dib3000mc.c 
b/drivers/media/dvb-frontends/dib3000mc.c
index 224283fe100a..4d086a7248e9 100644
--- a/drivers/media/dvb-frontends/dib3000mc.c
+++ b/drivers/media/dvb-frontends/dib3000mc.c
@@ -55,29 +55,57 @@ struct dib3000mc_state {
 
 static u16 dib3000mc_read_word(struct dib3000mc_state *state, u16 reg)
 {
-   u8 wb[2] = { (reg >> 8) | 0x80, reg & 0xff };
-   u8 rb[2];
struct i2c_msg msg[2] = {
-   { .addr = state->i2c_addr >> 1, .flags = 0,.buf = wb, 
.len = 2 },
-   { .addr = state->i2c_addr >> 1, .flags = I2C_M_RD, .buf = rb, 
.len = 2 },
+   { .addr = state->i2c_addr >> 1, .flags = 0,.len = 2 },
+   { .addr = state->i2c_addr >> 1, .flags = I2C_M_RD, .len = 2 },
};
+   u16 word;
+   u8 *b;
+
+   b = kmalloc(4, GFP_KERNEL);
+   if (!b)
+   return 0;
+
+   b[0] = (reg >> 8) | 0x80;
+   b[1] = reg;
+   b[2] = 0;
+   b[3] = 0;
+
+   msg[0].buf = b;
+   msg[1].buf = b + 2;
 
if (i2c_transfer(state->i2c_adap, msg, 2) != 2)
dprintk("i2c read error on %d\n",reg);
 
-   return (rb[0] << 8) | rb[1];
+   word = (b[2] << 8) | b[3];
+   kfree(b);
+
+   return word;
 }
 
 static int dib3000mc_write_word(struct dib3000mc_state *state, u16 reg, u16 
val)
 {
-   u8 b[4] = {
-   (reg >> 8) & 0xff, reg & 0xff,
-   (val >> 8) & 0xff, val & 0xff,
-   };
struct i2c_msg msg = {
-   .addr = state->i2c_addr >> 1, .flags = 0, .buf = b, .len = 4
+   .addr = state->i2c_addr >> 1, .flags = 0, .len = 4
};
-   return i2c_transfer(state->i2c_adap, , 1) != 1 ? -EREMOTEIO : 0;
+   int rc;
+   u8 *b;
+
+   b = kmalloc(4, GFP_KERNEL);
+   if (!b)
+   return -ENOMEM;
+
+   b[0] = reg >> 8;
+   b[1] = reg;
+   b[2] = val >> 8;
+   b[3] = val;
+
+   msg.buf = b;
+
+   rc = i2c_transfer(state->i2c_adap, , 1) != 1 ? -EREMOTEIO : 0;
+   kfree(b);
+
+   return rc;
 }
 
 static int dib3000mc_identify(struct dib3000mc_state *state)
diff --git a/drivers/media/dvb-frontends/dvb-pll.c 
b/drivers/media/dvb-frontends/dvb-pll.c
index 7bec3e028bee..5553b89b804e 100644
--- a/drivers/media/dvb-frontends/dvb-pll.c
+++ b/drivers/media/dvb-frontends/dvb-pll.c
@@ -753,13 +753,19 @@ struct dvb_frontend *dvb_pll_attach(struct dvb_frontend 
*fe, int pll_addr,
struct i2c_adapter *i2c,
unsigned int pll_desc_id)
 {
-   u8 b1 [] = { 0 };
-   struct i2c_msg msg = { .addr = pll_addr, .flags = I2C_M_RD,
-  .buf = b1, .len = 1 };
+   u8 *b1;
+   struct i2c_msg msg = { .addr = pll_addr, .flags = I2C_M_RD, .len = 1 };
struct dvb_pll_priv *priv = NULL;
int ret;
const struct dvb_pll_desc *desc;
 
+   b1 = kmalloc(1, GFP_KERNEL);
+   if (!b1)
+   return NULL;
+
+   b1[0] = 0;
+   msg.buf = b1;
+
if ((id[dvb_pll_devcount] > DVB_PLL_UNDEFINED) &&
(id[dvb_pll_devcount] < ARRAY_SIZE(pll_list)))
pll_desc_id = id[dvb_pll_devcount];
@@ -773,15 +779,19 @@ struct dvb_frontend *dvb_pll_attach(struct dvb_frontend 
*fe, int pll_addr,
fe->ops.i2c_gate_ctrl(fe, 1);
 
ret = i2c_transfer (i2c, , 1);
-   if (ret != 1)
+   if (ret != 1) {
+   kfree(b1);
return NULL;
+   }
if (fe->ops.i2c_gate_ctrl)
 fe->ops.i2c_gate_ctrl(fe, 0);
}
 
priv = kzalloc(sizeof(struct dvb_pll_priv), GFP_KERNEL);
-   if (priv == NULL)
+   if (!priv) {
+   kfree(b1);
return NULL;
+   }
 
priv->pll_i2c_address = pll_addr;
priv->i2c = i2c;
@@ -811,6 +821,8 @@ struct dvb_frontend 

Re: [PATCH v3 2/3] leds: as3645a: Add LED flash class driver

2017-09-07 Thread Pavel Machek
Hi!

> On Mon, Aug 28, 2017 at 01:04:51PM +0200, Pavel Machek wrote:
> > On Wed 2017-08-23 11:10:59, Sakari Ailus wrote:
> > > Add a LED flash class driver for the as3654a flash controller. A V4L2 
> > > flash
> > > driver for it already exists (drivers/media/i2c/as3645a.c), and this 
> > > driver
> > > is based on that.
> > 
> > We do not want to have two drivers for same hardware... how is that
> > supposed to work?
> 
> No, we don't. The intent is to remove the other driver later on, as it's
> implemented as a V4L2 sub-device driver. It also lacks DT support.

Could we perhaps remove the driver with the same patch that merges
this?

Having two drivers would be confusing.

> > Yes, we might want to have both LED and v4l2 interface for a single
> > LED, because v4l2 is just too hairy to use, but we should not
> > duplicate drivers for that.
> > 
> > We _might_ want to do some helpers; as these LED drivers are all quite
> > similar, it should be possible to have "flash led driver" and just
> > link it to v4l2 and LED interfaces...
> 
> This is what the new LED (flash) class driver does. Feature-wise it's a
> superset of the old one. There's a minor matter left, though, which is
> querying the flash strobe status which the old driver did and the new one
> does not do yet. I don't know if anyone ever even used that feature though.

Yes, I don't think anyone will notice...

Anyway,

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


signature.asc
Description: Digital signature


Re: [PATCH 1/1] media: Check for active and has_no_links overrun

2017-09-07 Thread Hans Verkuil
On 08/29/17 15:46, Sakari Ailus wrote:
> From: Sakari Ailus 
> 
> The active and has_no_links arrays will overrun in
> media_entity_pipeline_start() if there's an entity which has more than
> MEDIA_ENTITY_MAX_PAD pads. Ensure in media_entity_init() that there are
> fewer pads than that.
> 
> Signed-off-by: Sakari Ailus 

Acked-by: Hans Verkuil 

Thanks!

Hans

> ---
>  drivers/media/media-entity.c | 13 -
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 2ace0410d277..f7c6d64e6031 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -214,12 +214,20 @@ void media_gobj_destroy(struct media_gobj *gobj)
>   gobj->mdev = NULL;
>  }
>  
> +/*
> + * TODO: Get rid of this.
> + */
> +#define MEDIA_ENTITY_MAX_PADS512
> +
>  int media_entity_pads_init(struct media_entity *entity, u16 num_pads,
>  struct media_pad *pads)
>  {
>   struct media_device *mdev = entity->graph_obj.mdev;
>   unsigned int i;
>  
> + if (num_pads >= MEDIA_ENTITY_MAX_PADS)
> + return -E2BIG;
> +
>   entity->num_pads = num_pads;
>   entity->pads = pads;
>  
> @@ -280,11 +288,6 @@ static struct media_entity *stack_pop(struct media_graph 
> *graph)
>  #define link_top(en) ((en)->stack[(en)->top].link)
>  #define stack_top(en)((en)->stack[(en)->top].entity)
>  
> -/*
> - * TODO: Get rid of this.
> - */
> -#define MEDIA_ENTITY_MAX_PADS512
> -
>  /**
>   * media_graph_walk_init - Allocate resources for graph walk
>   * @graph: Media graph structure that will be used to walk the graph
> 



Re: [PATCH v8 06/21] v4l: fwnode: Support generic parsing of graph endpoints in a device

2017-09-07 Thread Hans Verkuil
On 09/07/17 11:58, Sakari Ailus wrote:
> Hi Hans,
> 
> On Thu, Sep 07, 2017 at 10:51:21AM +0200, Hans Verkuil wrote:
>> On 09/07/17 09:34, Sakari Ailus wrote:
>>> Hi Hans,
>>>
>>> On Wed, Sep 06, 2017 at 09:41:40AM +0200, Hans Verkuil wrote:
 On 09/05/2017 03:05 PM, Sakari Ailus wrote:
> The current practice is that drivers iterate over their endpoints and
> parse each endpoint separately. This is very similar in a number of
> drivers, implement a generic function for the job. Driver specific matters
> can be taken into account in the driver specific callback.
>
> Signed-off-by: Sakari Ailus 
> ---
>  drivers/media/v4l2-core/v4l2-async.c  |  19 +
>  drivers/media/v4l2-core/v4l2-fwnode.c | 140 
> ++
>  include/media/v4l2-async.h|  24 +-
>  include/media/v4l2-fwnode.h   |  53 +
>  4 files changed, 234 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> b/drivers/media/v4l2-core/v4l2-async.c
> index 3d81ff6a496f..7bd595c4094a 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -22,6 +22,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev 
> *asd)
> @@ -224,6 +225,24 @@ void v4l2_async_notifier_unregister(struct 
> v4l2_async_notifier *notifier)
>  }
>  EXPORT_SYMBOL(v4l2_async_notifier_unregister);
>  
> +void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)
> +{
> + unsigned int i;
> +
> + if (!notifier->max_subdevs)
> + return;
> +
> + for (i = 0; i < notifier->num_subdevs; i++)
> + kfree(notifier->subdevs[i]);
> +
> + notifier->max_subdevs = 0;
> + notifier->num_subdevs = 0;
> +
> + kvfree(notifier->subdevs);
> + notifier->subdevs = NULL;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);
> +
>  int v4l2_async_register_subdev(struct v4l2_subdev *sd)
>  {
>   struct v4l2_async_notifier *notifier;
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 706f9e7b90f1..e6932d7d47b6 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -19,6 +19,7 @@
>   */
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -26,6 +27,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  
>  enum v4l2_fwnode_bus_type {
> @@ -313,6 +315,144 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link 
> *link)
>  }
>  EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
>  
> +static int v4l2_async_notifier_realloc(struct v4l2_async_notifier 
> *notifier,
> +unsigned int max_subdevs)
> +{
> + struct v4l2_async_subdev **subdevs;
> +
> + if (max_subdevs <= notifier->max_subdevs)
> + return 0;
> +
> + subdevs = kvmalloc_array(
> + max_subdevs, sizeof(*notifier->subdevs),
> + GFP_KERNEL | __GFP_ZERO);
> + if (!subdevs)
> + return -ENOMEM;
> +
> + if (notifier->subdevs) {
> + memcpy(subdevs, notifier->subdevs,
> +sizeof(*subdevs) * notifier->num_subdevs);
> +
> + kvfree(notifier->subdevs);
> + }
> +
> + notifier->subdevs = subdevs;
> + notifier->max_subdevs = max_subdevs;
> +
> + return 0;
> +}
> +
> +static int v4l2_async_notifier_fwnode_parse_endpoint(
> + struct device *dev, struct v4l2_async_notifier *notifier,
> + struct fwnode_handle *endpoint, unsigned int asd_struct_size,
> + int (*parse_endpoint)(struct device *dev,
> + struct v4l2_fwnode_endpoint *vep,
> + struct v4l2_async_subdev *asd))
> +{
> + struct v4l2_async_subdev *asd;
> + struct v4l2_fwnode_endpoint *vep;
> + struct fwnode_endpoint ep;
> + int ret = 0;
> +
> + asd = kzalloc(asd_struct_size, GFP_KERNEL);
> + if (!asd)
> + return -ENOMEM;
> +
> + asd->match.fwnode.fwnode =
> + fwnode_graph_get_remote_port_parent(endpoint);
> + if (!asd->match.fwnode.fwnode) {
> + dev_warn(dev, "bad remote port parent\n");
> + ret = -EINVAL;
> + goto out_err;
> + }
> +
> + /* Ignore endpoints the parsing of which failed. */
> + vep = v4l2_fwnode_endpoint_alloc_parse(endpoint);
> + if (IS_ERR(vep)) {
> + ret = PTR_ERR(vep);
> + dev_warn(dev, "unable to parse V4L2 fwnode endpoint (%d)\n",
> +  ret);
> + 

Re: [PATCH v8 14/21] v4l: async: Allow binding notifiers to sub-devices

2017-09-07 Thread Hans Verkuil
On 09/07/17 10:32, Sakari Ailus wrote:
> Hi Hans,
> 
> On Wed, Sep 06, 2017 at 10:46:31AM +0200, Hans Verkuil wrote:
>> On 09/05/2017 03:05 PM, Sakari Ailus wrote:

>>> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
>>> index 3bc8a7c0d83f..12739be44bd1 100644
>>> --- a/include/media/v4l2-async.h
>>> +++ b/include/media/v4l2-async.h
>>> @@ -102,7 +102,9 @@ struct v4l2_async_notifier_operations {
>>>   * @num_subdevs: number of subdevices used in the subdevs array
>>>   * @max_subdevs: number of subdevices allocated in the subdevs array
>>>   * @subdevs:   array of pointers to subdevice descriptors
>>> - * @v4l2_dev:  pointer to struct v4l2_device
>>> + * @v4l2_dev:  v4l2_device of the master, for subdev notifiers NULL
>>> + * @sd:sub-device that registered the notifier, NULL otherwise
>>> + * @master:master notifier carrying @v4l2_dev
>>
>> I think this description is out of date. It is really the parent notifier,
>> right? Should 'master' be renamed to 'parent'?
> 
> You could view it as one, yes. What is known is that the notifier is
> related, and through which the v4l2_dev can be found. I'll rename it as
> "parent".
> 
> I'll use root in the commit message as well.
> 
>>
>> Same problem with the description of @v4l2_dev: it's the v4l2_device of the
>> root/top-level notifier.
> 
> "v4l2_device of the root notifier, NULL otherwise"?

Ack.

Hans

> 
>>
>>>   * @waiting:   list of struct v4l2_async_subdev, waiting for their 
>>> drivers
>>>   * @done:  list of struct v4l2_subdev, already probed
>>>   * @list:  member in a global list of notifiers
>>> @@ -113,6 +115,8 @@ struct v4l2_async_notifier {
>>> unsigned int max_subdevs;
>>> struct v4l2_async_subdev **subdevs;
>>> struct v4l2_device *v4l2_dev;
>>> +   struct v4l2_subdev *sd;
>>> +   struct v4l2_async_notifier *master;
>>> struct list_head waiting;
>>> struct list_head done;
>>> struct list_head list;
>>> @@ -128,6 +132,16 @@ int v4l2_async_notifier_register(struct v4l2_device 
>>> *v4l2_dev,
>>>  struct v4l2_async_notifier *notifier);
>>>  
>>>  /**
>>> + * v4l2_async_subdev_notifier_register - registers a subdevice asynchronous
>>> + *  notifier for a sub-device
>>> + *
>>> + * @sd: pointer to  v4l2_subdev
>>> + * @notifier: pointer to  v4l2_async_notifier
>>> + */
>>> +int v4l2_async_subdev_notifier_register(struct v4l2_subdev *sd,
>>> +   struct v4l2_async_notifier *notifier);
>>> +
>>> +/**
>>>   * v4l2_async_notifier_unregister - unregisters a subdevice asynchronous 
>>> notifier
>>>   *
>>>   * @notifier: pointer to  v4l2_async_notifier
>>>
>>
>> This v8 is much better and is getting close.
> 
> Thanks!
> 



[GIT PULL for v4.14-rc1] media updates

2017-09-07 Thread Mauro Carvalho Chehab
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Hi Linus

Please pull from:
  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.14-1

For the Brazil's independence day's pull request :-)
This is one of the biggest media pull requests, with 625 patches
affecting almost all parts of media (RC, DVB, V4L2, CEC, docs).

This pull request contains:

- - a lot of new drivers:
   - DVB frontends: mxl5xx, stv0910, stv6111;
   - camera flash: as3645a led driver;
   - HDMI receiver: adv748X;
   - camera sensor: Omnivision 6650 5M driver (ov6650);
   - HDMI CEC: ao-cec meson driver;
   - V4L2: Qualcom camss driver;
   - Remote controller: gpio-ir-tx, pwm-ir-tx and zx-irdec drivers.

- - The DDbridge DVB driver got a massive update, with makes
  it in sync with modern hardware from that vendor;

- - There's an important milestone on this series: the DVB documentation
  was written in 2003, but only started to be updated in 2007. It
  also used to contain several gaps from the time it was kept out of
  tree, mentioning error codes and device nodes that never existed
  upstream. On this series, it received a massive update: all
  non-deprecated digital TV APIs are now in sync with the current
  implementation;

- - Some DVB APIs that aren't used by any upstream driver got removed;

- - Other parts of the media documentation algo got updated, fixing some
  bugs on its PDF output and making it compatible with Sphinx version 
  1.6. As the number of hacks required to build PDF output reduced,
  I hope we'll have less troubles as newer versions of our
  documentation toolchain are released (famous last words);

- - As usual, lots of driver cleanups and improvements.
 
Thanks!
Mauro

- ---


The following changes since commit aae4e7a8bc44722fe70d58920a36916b1043195e:

  Linux 4.13-rc4 (2017-08-06 18:44:49 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
tags/media/v4.14-1

for you to fetch changes up to 1efdf1776e2253b77413c997bed862410e4b6aaf:

  media: leds: as3645a: add V4L2_FLASH_LED_CLASS dependency (2017-09-05 
16:32:45 -0400)

- 
media updates for v4.14-rc1

- 
Akihiro Tsukada (1):
  media: media/dvb: earth-pt3: fix hang-up in a rare case

Amitoj Kaur Chawla (2):
  media: staging: atomisp: Remove unnecessary return statement in void 
function
  media: staging: atomisp: Use kvfree() instead of kfree()/vfree()

Andrzej Pietrasiewicz (5):
  media: s5p-jpeg: don't overwrite result's "size" member
  media: s5p-jpeg: set w/h when encoding
  media: s5p-jpeg: disable encoder/decoder in exynos4-like hardware after 
use
  media: s5p-jpeg: fix number of components macro
  media: s5p-jpeg: directly use parsed subsampling on exynos5433

Anton Sviridenko (2):
  media: solo6x10: fix detection of TW2864B chips
  media: solo6x10: export hardware GPIO pins 8:31 to gpiolib interface

Anton Vasilyev (1):
  media: dvb-usb: Add memory free on error path in dw2102_probe()

Arnd Bergmann (16):
  media: platform: video-mux: fix Kconfig dependency
  media: usbvision-i2c: fix format overflow warning
  media: venus: mark PM functions as __maybe_unused
  media: venus: fix compile-test build on non-qcom ARM platform
  media: fix warning on v4l2_subdev_call() result interpreted as bool
  media: v4l: omap_vout: vrfb: include linux/slab.h
  media: imx: add VIDEO_V4L2_SUBDEV_API dependency
  media: i2c: add KConfig dependencies
  media: v4l: use WARN_ON(1) instead of __WARN()
  media: v4l: omap_vout: vrfb: initialize DMA flags
  media: staging/imx: remove confusing IS_ERR_OR_NULL usage
  media: omap3isp: fix uninitialized variable use
  media: staging: atomisp: imx: remove dead code
  media: au0828: fix RC_CORE dependency
  media: staging/imx: always select VIDEOBUF2_DMA_CONTIG
  media: leds: as3645a: add V4L2_FLASH_LED_CLASS dependency

Arvind Yadav (57):
  media: vb2 dma-contig: Constify dma_buf_ops structures
  media: vb2 vmalloc: Constify dma_buf_ops structures
  media: vb2 dma-sg: Constify dma_buf_ops structures
  media: staging: atomisp: lm3554: constify acpi_device_id
  media: staging: atomisp: ov2680: constify acpi_device_id
  media: staging: atomisp: ov8858: constify acpi_device_id
  media: staging: atomisp: gc0310: constify acpi_device_id
  media: staging: atomisp: ov2722: constify acpi_device_id
  media: staging: atomisp: ov5693: constify acpi_device_id
  media: staging: atomisp: mt9m114: constify acpi_device_id
  media: staging: atomisp: gc2235: constify acpi_device_id
  media: exynos4-is: fimc-is-i2c: constify dev_pm_ops structures
  media: marvell-ccic: constify pci_device_id
  media: netup_unidvb: constify pci_device_id
  media: cx23885: constify 

Re: [PATCH v3 2/3] leds: as3645a: Add LED flash class driver

2017-09-07 Thread Sakari Ailus
Hi Pavel,

On Mon, Aug 28, 2017 at 01:04:51PM +0200, Pavel Machek wrote:
> On Wed 2017-08-23 11:10:59, Sakari Ailus wrote:
> > Add a LED flash class driver for the as3654a flash controller. A V4L2 flash
> > driver for it already exists (drivers/media/i2c/as3645a.c), and this driver
> > is based on that.
> 
> We do not want to have two drivers for same hardware... how is that
> supposed to work?

No, we don't. The intent is to remove the other driver later on, as it's
implemented as a V4L2 sub-device driver. It also lacks DT support.

> 
> Yes, we might want to have both LED and v4l2 interface for a single
> LED, because v4l2 is just too hairy to use, but we should not
> duplicate drivers for that.
> 
> We _might_ want to do some helpers; as these LED drivers are all quite
> similar, it should be possible to have "flash led driver" and just
> link it to v4l2 and LED interfaces...

This is what the new LED (flash) class driver does. Feature-wise it's a
superset of the old one. There's a minor matter left, though, which is
querying the flash strobe status which the old driver did and the new one
does not do yet. I don't know if anyone ever even used that feature though.

-- 
Regards,

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


Re: [PATCH v3 05/14] [media] cxd2880: Add tuner part of the driver

2017-09-07 Thread Mauro Carvalho Chehab
Em Thu, 7 Sep 2017 19:12:57 +0900
"Takiguchi, Yasunari"  escreveu:

> Dear Mauro
> 
> Thanks for your review and reply.
> 
> We are going to discuss how to change our code with your comments internally.
> 
> I reply for your  2 comments,
> 
> >> [Change list]
> >> Changes in V3
> >>drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
> >>   -removed code relevant to ISDB-T
> > 
> > Just curiosity here: why is it removed?
> We decided to withhold the ISDB-T functionality as it contains some company 
> proprietary code.

I'm sorry to hear. I hope that such code could be released
on some future.

> >> +  if (ret)
> >> +  return ret;
> >> +  if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl) {
> >> +  data[0] = 0x01;
> >> +  data[1] = 0x01;
> >> +  data[2] = 0x01;
> >> +  data[3] = 0x01;
> >> +  data[4] = 0x01;
> >> +  data[5] = 0x01;
> >> +  } else {
> >> +  data[0] = 0x00;
> >> +  data[1] = 0x00;
> >> +  data[2] = 0x00;
> >> +  data[3] = 0x00;
> >> +  data[4] = 0x00;
> >> +  data[5] = 0x00;
> >> +  }
> > 
> > Instead, just do:
> > 
> > if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl)
> > memset(data, 0x01, sizeof(data));
> > else
> > memset(data, 0x00, sizeof(data));
> > 
> >> +  ret = tnr_dmd->io->write_regs(tnr_dmd->io,
> >> +CXD2880_IO_TGT_SYS,
> >> +0xef, data, 6);
> >> +  if (ret)
> >> +  return ret;
> >> +
> >> +  ret = tnr_dmd->io->write_reg(tnr_dmd->io,
> >> +   CXD2880_IO_TGT_DMD,
> >> +   0x00, 0x2d);
> >> +  if (ret)
> >> +  return ret;
> > 
> >> +  if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl)
> >> +  data[0] = 0x00;
> >> +  else
> >> +  data[0] = 0x01;
> > 
> > Not actually needed, as the previous logic already set data[0]
> > accordingly.
> > 
> >> +  ret = tnr_dmd->io->write_reg(tnr_dmd->io,
> >> +   CXD2880_IO_TGT_DMD,
> >> +   0xb1, data[0]);
> 
> In this case、logic of data[0]( logic of if() ) is different from that of 
> previous one.
> And with setting register for address 0xb1, a bug might occur in the future, 
> if our software specification (sequence) is changed.
> So we would like to keep setting value of data[0] for address 0xb1.

OK. Better to document it then, as otherwise someone might
end by sending cleanup patches that would touch it.

> 
> Thanks,
> Takiguchi



Thanks,
Mauro


Re: [PATCH v3 05/14] [media] cxd2880: Add tuner part of the driver

2017-09-07 Thread Takiguchi, Yasunari
Dear Mauro

Thanks for your review and reply.

We are going to discuss how to change our code with your comments internally.

I reply for your  2 comments,

>> [Change list]
>> Changes in V3
>>drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
>>   -removed code relevant to ISDB-T
> 
> Just curiosity here: why is it removed?
We decided to withhold the ISDB-T functionality as it contains some company 
proprietary code.


>> +if (ret)
>> +return ret;
>> +if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl) {
>> +data[0] = 0x01;
>> +data[1] = 0x01;
>> +data[2] = 0x01;
>> +data[3] = 0x01;
>> +data[4] = 0x01;
>> +data[5] = 0x01;
>> +} else {
>> +data[0] = 0x00;
>> +data[1] = 0x00;
>> +data[2] = 0x00;
>> +data[3] = 0x00;
>> +data[4] = 0x00;
>> +data[5] = 0x00;
>> +}
> 
> Instead, just do:
> 
>   if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl)
>   memset(data, 0x01, sizeof(data));
>   else
>   memset(data, 0x00, sizeof(data));
> 
>> +ret = tnr_dmd->io->write_regs(tnr_dmd->io,
>> +  CXD2880_IO_TGT_SYS,
>> +  0xef, data, 6);
>> +if (ret)
>> +return ret;
>> +
>> +ret = tnr_dmd->io->write_reg(tnr_dmd->io,
>> + CXD2880_IO_TGT_DMD,
>> + 0x00, 0x2d);
>> +if (ret)
>> +return ret;
> 
>> +if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl)
>> +data[0] = 0x00;
>> +else
>> +data[0] = 0x01;
> 
> Not actually needed, as the previous logic already set data[0]
> accordingly.
> 
>> +ret = tnr_dmd->io->write_reg(tnr_dmd->io,
>> + CXD2880_IO_TGT_DMD,
>> + 0xb1, data[0]);

In this case、logic of data[0]( logic of if() ) is different from that of 
previous one.
And with setting register for address 0xb1, a bug might occur in the future, 
if our software specification (sequence) is changed.
So we would like to keep setting value of data[0] for address 0xb1.

Thanks,
Takiguchi


Re: [PATCH v8 06/21] v4l: fwnode: Support generic parsing of graph endpoints in a device

2017-09-07 Thread Sakari Ailus
Hi Hans,

On Thu, Sep 07, 2017 at 10:51:21AM +0200, Hans Verkuil wrote:
> On 09/07/17 09:34, Sakari Ailus wrote:
> > Hi Hans,
> > 
> > On Wed, Sep 06, 2017 at 09:41:40AM +0200, Hans Verkuil wrote:
> >> On 09/05/2017 03:05 PM, Sakari Ailus wrote:
> >>> The current practice is that drivers iterate over their endpoints and
> >>> parse each endpoint separately. This is very similar in a number of
> >>> drivers, implement a generic function for the job. Driver specific matters
> >>> can be taken into account in the driver specific callback.
> >>>
> >>> Signed-off-by: Sakari Ailus 
> >>> ---
> >>>  drivers/media/v4l2-core/v4l2-async.c  |  19 +
> >>>  drivers/media/v4l2-core/v4l2-fwnode.c | 140 
> >>> ++
> >>>  include/media/v4l2-async.h|  24 +-
> >>>  include/media/v4l2-fwnode.h   |  53 +
> >>>  4 files changed, 234 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> >>> b/drivers/media/v4l2-core/v4l2-async.c
> >>> index 3d81ff6a496f..7bd595c4094a 100644
> >>> --- a/drivers/media/v4l2-core/v4l2-async.c
> >>> +++ b/drivers/media/v4l2-core/v4l2-async.c
> >>> @@ -22,6 +22,7 @@
> >>>  
> >>>  #include 
> >>>  #include 
> >>> +#include 
> >>>  #include 
> >>>  
> >>>  static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev 
> >>> *asd)
> >>> @@ -224,6 +225,24 @@ void v4l2_async_notifier_unregister(struct 
> >>> v4l2_async_notifier *notifier)
> >>>  }
> >>>  EXPORT_SYMBOL(v4l2_async_notifier_unregister);
> >>>  
> >>> +void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)
> >>> +{
> >>> + unsigned int i;
> >>> +
> >>> + if (!notifier->max_subdevs)
> >>> + return;
> >>> +
> >>> + for (i = 0; i < notifier->num_subdevs; i++)
> >>> + kfree(notifier->subdevs[i]);
> >>> +
> >>> + notifier->max_subdevs = 0;
> >>> + notifier->num_subdevs = 0;
> >>> +
> >>> + kvfree(notifier->subdevs);
> >>> + notifier->subdevs = NULL;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);
> >>> +
> >>>  int v4l2_async_register_subdev(struct v4l2_subdev *sd)
> >>>  {
> >>>   struct v4l2_async_notifier *notifier;
> >>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> >>> b/drivers/media/v4l2-core/v4l2-fwnode.c
> >>> index 706f9e7b90f1..e6932d7d47b6 100644
> >>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> >>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> >>> @@ -19,6 +19,7 @@
> >>>   */
> >>>  #include 
> >>>  #include 
> >>> +#include 
> >>>  #include 
> >>>  #include 
> >>>  #include 
> >>> @@ -26,6 +27,7 @@
> >>>  #include 
> >>>  #include 
> >>>  
> >>> +#include 
> >>>  #include 
> >>>  
> >>>  enum v4l2_fwnode_bus_type {
> >>> @@ -313,6 +315,144 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link 
> >>> *link)
> >>>  }
> >>>  EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
> >>>  
> >>> +static int v4l2_async_notifier_realloc(struct v4l2_async_notifier 
> >>> *notifier,
> >>> +unsigned int max_subdevs)
> >>> +{
> >>> + struct v4l2_async_subdev **subdevs;
> >>> +
> >>> + if (max_subdevs <= notifier->max_subdevs)
> >>> + return 0;
> >>> +
> >>> + subdevs = kvmalloc_array(
> >>> + max_subdevs, sizeof(*notifier->subdevs),
> >>> + GFP_KERNEL | __GFP_ZERO);
> >>> + if (!subdevs)
> >>> + return -ENOMEM;
> >>> +
> >>> + if (notifier->subdevs) {
> >>> + memcpy(subdevs, notifier->subdevs,
> >>> +sizeof(*subdevs) * notifier->num_subdevs);
> >>> +
> >>> + kvfree(notifier->subdevs);
> >>> + }
> >>> +
> >>> + notifier->subdevs = subdevs;
> >>> + notifier->max_subdevs = max_subdevs;
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static int v4l2_async_notifier_fwnode_parse_endpoint(
> >>> + struct device *dev, struct v4l2_async_notifier *notifier,
> >>> + struct fwnode_handle *endpoint, unsigned int asd_struct_size,
> >>> + int (*parse_endpoint)(struct device *dev,
> >>> + struct v4l2_fwnode_endpoint *vep,
> >>> + struct v4l2_async_subdev *asd))
> >>> +{
> >>> + struct v4l2_async_subdev *asd;
> >>> + struct v4l2_fwnode_endpoint *vep;
> >>> + struct fwnode_endpoint ep;
> >>> + int ret = 0;
> >>> +
> >>> + asd = kzalloc(asd_struct_size, GFP_KERNEL);
> >>> + if (!asd)
> >>> + return -ENOMEM;
> >>> +
> >>> + asd->match.fwnode.fwnode =
> >>> + fwnode_graph_get_remote_port_parent(endpoint);
> >>> + if (!asd->match.fwnode.fwnode) {
> >>> + dev_warn(dev, "bad remote port parent\n");
> >>> + ret = -EINVAL;
> >>> + goto out_err;
> >>> + }
> >>> +
> >>> + /* Ignore endpoints the parsing of which failed. */
> >>> + vep = v4l2_fwnode_endpoint_alloc_parse(endpoint);
> >>> + if (IS_ERR(vep)) {
> >>> + ret = PTR_ERR(vep);
> >>> + dev_warn(dev, "unable to parse V4L2 fwnode endpoint (%d)\n",
> >>> +  ret);
> >>> + goto out_err;
> >>> + }
> >>> +
> >>> + ep = 

[PATCH] media: cx23885: make const array buf static, reduces object code size

2017-09-07 Thread Colin King
From: Colin Ian King 

Don't populate the array buf on the stack, instead make it static.
Makes the object code smaller by over 240 bytes:

Before:
   textdata bss dec hex filename
  21689   22992 416   45097b029 cx23885-cards.o

After:
   textdata bss dec hex filename
  21348   23088 416   44852af34 cx23885-cards.o

Signed-off-by: Colin Ian King 
---
 drivers/media/pci/cx23885/cx23885-cards.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/cx23885/cx23885-cards.c 
b/drivers/media/pci/cx23885/cx23885-cards.c
index 78a8836d03e4..28eab9c518c5 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -1323,7 +1323,7 @@ static void hauppauge_eeprom(struct cx23885_dev *dev, u8 
*eeprom_data)
 static void tbs_card_init(struct cx23885_dev *dev)
 {
int i;
-   const u8 buf[] = {
+   static const u8 buf[] = {
0xe0, 0x06, 0x66, 0x33, 0x65,
0x01, 0x17, 0x06, 0xde};
 
-- 
2.14.1



Re: [PATCH v8 06/21] v4l: fwnode: Support generic parsing of graph endpoints in a device

2017-09-07 Thread Hans Verkuil
On 09/07/17 09:34, Sakari Ailus wrote:
> Hi Hans,
> 
> On Wed, Sep 06, 2017 at 09:41:40AM +0200, Hans Verkuil wrote:
>> On 09/05/2017 03:05 PM, Sakari Ailus wrote:
>>> The current practice is that drivers iterate over their endpoints and
>>> parse each endpoint separately. This is very similar in a number of
>>> drivers, implement a generic function for the job. Driver specific matters
>>> can be taken into account in the driver specific callback.
>>>
>>> Signed-off-by: Sakari Ailus 
>>> ---
>>>  drivers/media/v4l2-core/v4l2-async.c  |  19 +
>>>  drivers/media/v4l2-core/v4l2-fwnode.c | 140 
>>> ++
>>>  include/media/v4l2-async.h|  24 +-
>>>  include/media/v4l2-fwnode.h   |  53 +
>>>  4 files changed, 234 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-async.c 
>>> b/drivers/media/v4l2-core/v4l2-async.c
>>> index 3d81ff6a496f..7bd595c4094a 100644
>>> --- a/drivers/media/v4l2-core/v4l2-async.c
>>> +++ b/drivers/media/v4l2-core/v4l2-async.c
>>> @@ -22,6 +22,7 @@
>>>  
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  
>>>  static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev 
>>> *asd)
>>> @@ -224,6 +225,24 @@ void v4l2_async_notifier_unregister(struct 
>>> v4l2_async_notifier *notifier)
>>>  }
>>>  EXPORT_SYMBOL(v4l2_async_notifier_unregister);
>>>  
>>> +void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)
>>> +{
>>> +   unsigned int i;
>>> +
>>> +   if (!notifier->max_subdevs)
>>> +   return;
>>> +
>>> +   for (i = 0; i < notifier->num_subdevs; i++)
>>> +   kfree(notifier->subdevs[i]);
>>> +
>>> +   notifier->max_subdevs = 0;
>>> +   notifier->num_subdevs = 0;
>>> +
>>> +   kvfree(notifier->subdevs);
>>> +   notifier->subdevs = NULL;
>>> +}
>>> +EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);
>>> +
>>>  int v4l2_async_register_subdev(struct v4l2_subdev *sd)
>>>  {
>>> struct v4l2_async_notifier *notifier;
>>> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
>>> b/drivers/media/v4l2-core/v4l2-fwnode.c
>>> index 706f9e7b90f1..e6932d7d47b6 100644
>>> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
>>> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
>>> @@ -19,6 +19,7 @@
>>>   */
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>  #include 
>>> @@ -26,6 +27,7 @@
>>>  #include 
>>>  #include 
>>>  
>>> +#include 
>>>  #include 
>>>  
>>>  enum v4l2_fwnode_bus_type {
>>> @@ -313,6 +315,144 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link 
>>> *link)
>>>  }
>>>  EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
>>>  
>>> +static int v4l2_async_notifier_realloc(struct v4l2_async_notifier 
>>> *notifier,
>>> +  unsigned int max_subdevs)
>>> +{
>>> +   struct v4l2_async_subdev **subdevs;
>>> +
>>> +   if (max_subdevs <= notifier->max_subdevs)
>>> +   return 0;
>>> +
>>> +   subdevs = kvmalloc_array(
>>> +   max_subdevs, sizeof(*notifier->subdevs),
>>> +   GFP_KERNEL | __GFP_ZERO);
>>> +   if (!subdevs)
>>> +   return -ENOMEM;
>>> +
>>> +   if (notifier->subdevs) {
>>> +   memcpy(subdevs, notifier->subdevs,
>>> +  sizeof(*subdevs) * notifier->num_subdevs);
>>> +
>>> +   kvfree(notifier->subdevs);
>>> +   }
>>> +
>>> +   notifier->subdevs = subdevs;
>>> +   notifier->max_subdevs = max_subdevs;
>>> +
>>> +   return 0;
>>> +}
>>> +
>>> +static int v4l2_async_notifier_fwnode_parse_endpoint(
>>> +   struct device *dev, struct v4l2_async_notifier *notifier,
>>> +   struct fwnode_handle *endpoint, unsigned int asd_struct_size,
>>> +   int (*parse_endpoint)(struct device *dev,
>>> +   struct v4l2_fwnode_endpoint *vep,
>>> +   struct v4l2_async_subdev *asd))
>>> +{
>>> +   struct v4l2_async_subdev *asd;
>>> +   struct v4l2_fwnode_endpoint *vep;
>>> +   struct fwnode_endpoint ep;
>>> +   int ret = 0;
>>> +
>>> +   asd = kzalloc(asd_struct_size, GFP_KERNEL);
>>> +   if (!asd)
>>> +   return -ENOMEM;
>>> +
>>> +   asd->match.fwnode.fwnode =
>>> +   fwnode_graph_get_remote_port_parent(endpoint);
>>> +   if (!asd->match.fwnode.fwnode) {
>>> +   dev_warn(dev, "bad remote port parent\n");
>>> +   ret = -EINVAL;
>>> +   goto out_err;
>>> +   }
>>> +
>>> +   /* Ignore endpoints the parsing of which failed. */
>>> +   vep = v4l2_fwnode_endpoint_alloc_parse(endpoint);
>>> +   if (IS_ERR(vep)) {
>>> +   ret = PTR_ERR(vep);
>>> +   dev_warn(dev, "unable to parse V4L2 fwnode endpoint (%d)\n",
>>> +ret);
>>> +   goto out_err;
>>> +   }
>>> +
>>> +   ep = vep->base;
>>> +
>>> +   ret = parse_endpoint ? parse_endpoint(dev, vep, asd) : 0;
>>> +   v4l2_fwnode_endpoint_free(vep);
>>> +   if (ret == -ENOTCONN) {
>>> +   dev_dbg(dev, "ignoring endpoint %u,%u\n", ep.port, ep.id);
>>> +   

Re: [PATCH v8 14/21] v4l: async: Allow binding notifiers to sub-devices

2017-09-07 Thread Sakari Ailus
Hi Hans,

On Wed, Sep 06, 2017 at 10:46:31AM +0200, Hans Verkuil wrote:
> On 09/05/2017 03:05 PM, Sakari Ailus wrote:
> > Registering a notifier has required the knowledge of struct v4l2_device
> > for the reason that sub-devices generally are registered to the
> > v4l2_device (as well as the media device, also available through
> > v4l2_device).
> > 
> > This information is not available for sub-device drivers at probe time.
> > 
> > What this patch does is that it allows registering notifiers without
> > having v4l2_device around. Instead the sub-device pointer is stored to the
> 
> to -> in

Fixed.

> 
> > notifier. Once the sub-device of the driver that registered the notifier
> > is registered, the notifier will gain the knowledge of the v4l2_device,
> > and the binding of async sub-devices from the sub-device driver's notifier
> > may proceed.
> > 
> > The master notifier's complete callback is only called when all sub-device
> > notifiers are completed.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/v4l2-core/v4l2-async.c | 209 
> > ++-
> >  include/media/v4l2-async.h   |  16 ++-
> >  2 files changed, 194 insertions(+), 31 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> > b/drivers/media/v4l2-core/v4l2-async.c
> > index 79f216723a3f..620b2cd29fc3 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -53,6 +53,10 @@ static int v4l2_async_notifier_call_complete(struct 
> > v4l2_async_notifier *n)
> > return n->ops->complete(n);
> >  }
> >  
> > +static int v4l2_async_match_notify(struct v4l2_async_notifier *notifier,
> > +  struct v4l2_subdev *sd,
> > +  struct v4l2_async_subdev *asd);
> > +
> >  static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev 
> > *asd)
> >  {
> >  #if IS_ENABLED(CONFIG_I2C)
> > @@ -129,14 +133,119 @@ static struct v4l2_async_subdev 
> > *v4l2_async_find_match(
> > return NULL;
> >  }
> >  
> > +/* Get the sub-device notifier registered by a sub-device driver. */
> > +static struct v4l2_async_notifier *v4l2_async_get_subdev_notifier(
> > +   struct v4l2_subdev *sd)
> > +{
> > +   struct v4l2_async_notifier *n;
> > +
> > +   list_for_each_entry(n, _list, list)
> > +   if (n->sd == sd)
> > +   return n;
> > +
> > +   return NULL;
> > +}
> > +
> > +/* Return true if all sub-device notifiers are complete, false otherwise. 
> > */
> > +static bool v4l2_async_subdev_notifiers_complete(
> > +   struct v4l2_async_notifier *notifier)
> > +{
> > +   struct v4l2_subdev *sd;
> > +
> > +   if (!list_empty(>waiting))
> > +   return false;
> > +
> > +   list_for_each_entry(sd, >done, async_list) {
> > +   struct v4l2_async_notifier *subdev_notifier =
> > +   v4l2_async_get_subdev_notifier(sd);
> > +
> > +   if (!subdev_notifier)
> > +   continue;
> > +
> > +   if (!v4l2_async_subdev_notifiers_complete(subdev_notifier))
> > +   return false;
> > +   }
> > +
> > +   return true;
> > +}
> > +
> > +/* Get v4l2_device related to the notifier if one can be found. */
> > +static struct v4l2_device *v4l2_async_notifier_get_v4l2_dev(
> > +   struct v4l2_async_notifier *notifier)
> > +{
> > +   while (notifier->master)
> > +   notifier = notifier->master;
> > +
> > +   return notifier->v4l2_dev;
> > +}
> > +
> > +/* Test all async sub-devices in a notifier for a match. */
> > +static int v4l2_async_notifier_try_all_subdevs(
> > +   struct v4l2_async_notifier *notifier)
> > +{
> > +   struct v4l2_subdev *sd, *tmp;
> > +
> > +   if (!v4l2_async_notifier_get_v4l2_dev(notifier))
> > +   return 0;
> > +
> > +   list_for_each_entry_safe(sd, tmp, _list, async_list) {
> > +   struct v4l2_async_subdev *asd;
> > +   int ret;
> > +
> > +   asd = v4l2_async_find_match(notifier, sd);
> > +   if (!asd)
> > +   continue;
> > +
> > +   ret = v4l2_async_match_notify(notifier, sd, asd);
> > +   if (ret < 0)
> > +   return ret;
> > +   }
> > +
> > +   return 0;
> > +}
> > +
> > +/* Try completing a notifier. */
> > +static int v4l2_async_notifier_try_complete(
> > +   struct v4l2_async_notifier *notifier)
> > +{
> > +   do {
> > +   int ret;
> > +
> > +   /* Any local async sub-devices left? */
> > +   if (!list_empty(>waiting))
> > +   return 0;
> > +
> > +   /*
> > +* Any sub-device notifiers waiting for async subdevs
> > +* to be bound?
> > +*/
> > +   if (!v4l2_async_subdev_notifiers_complete(notifier))
> > +   return 0;
> > +
> > +   /* Proceed completing the notifier */
> > +   ret = v4l2_async_notifier_call_complete(notifier);
> > +   if (ret 

Re: [PATCH v8 08/21] rcar-vin: Use generic parser for parsing fwnode endpoints

2017-09-07 Thread Sakari Ailus
Hi Hans,

On Wed, Sep 06, 2017 at 09:44:32AM +0200, Hans Verkuil wrote:
> On 09/05/2017 03:05 PM, Sakari Ailus wrote:
> >  static int rvin_digital_graph_init(struct rvin_dev *vin)
> >  {
> > -   struct v4l2_async_subdev **subdevs = NULL;
> > int ret;
> >  
> > -   ret = rvin_digital_graph_parse(vin);
> > +   ret = v4l2_async_notifier_parse_fwnode_endpoints(
> > +   vin->dev, >notifier,
> > +   sizeof(struct rvin_graph_entity), rvin_digital_parse_v4l2);
> > if (ret)
> > return ret;
> >  
> > -   if (!vin->digital.asd.match.fwnode.fwnode) {
> > -   vin_dbg(vin, "No digital subdevice found\n");
> > -   return -ENODEV;
> > -   }
> > -
> > -   /* Register the subdevices notifier. */
> > -   subdevs = devm_kzalloc(vin->dev, sizeof(*subdevs), GFP_KERNEL);
> > -   if (subdevs == NULL)
> > -   return -ENOMEM;
> > -
> > -   subdevs[0] = >digital.asd;
> > -
> > -   vin_dbg(vin, "Found digital subdevice %pOF\n",
> > -   to_of_node(subdevs[0]->match.fwnode.fwnode));
> > +   if (vin->notifier.num_subdevs > 0)
> > +   vin_dbg(vin, "Found digital subdevice %pOF\n",
> > +   to_of_node(
> > +   vin->notifier.subdevs[0]->match.fwnode.fwnode));
> 
> As mentioned in my review of patch 6/21, this violates the documentation of 
> the
> v4l2_async_notifier_parse_fwnode_endpoints function.
> 
> However, I think the problem is with the documentation and not with this code,
> so:
> 
> Acked-by: Hans Verkuil 

Thanks. I still don't like to encourage accessing the notifier fields
directly from drivers, and in this case there isn't a need to either. The
following changes work, too, and I'll use them in the next version:

diff --git a/drivers/media/platform/rcar-vin/rcar-core.c 
b/drivers/media/platform/rcar-vin/rcar-core.c
index bd551f0be213..d1e5909da087 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -177,10 +177,10 @@ static int rvin_digital_graph_init(struct rvin_dev *vin)
if (ret)
return ret;
 
-   if (vin->notifier.num_subdevs > 0)
+   if (vin->digital)
vin_dbg(vin, "Found digital subdevice %pOF\n",
to_of_node(
-   vin->notifier.subdevs[0]->match.fwnode.fwnode));
+   vin->digital->asd.match.fwnode.fwnode));
 
vin->notifier.bound = rvin_digital_notify_bound;
vin->notifier.unbind = rvin_digital_notify_unbind;

I also changed EPERM still used in this version to ENOTCONN.

-- 
Regards,

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


Re: [PATCH v8 12/21] v4l: async: Introduce helpers for calling async ops callbacks

2017-09-07 Thread Sakari Ailus
On Wed, Sep 06, 2017 at 09:50:36AM +0200, Hans Verkuil wrote:
> On 09/05/2017 03:05 PM, Sakari Ailus wrote:
> > Add three helper functions to call async operations callbacks. Besides
> > simplifying callbacks, this allows async notifiers to have no ops set,
> > i.e. it can be left NULL.
> 
> What is the use-case of that?

Going forward, the sub-notifiers that are just for binding associated
devices (later in the patchset, 17th and 18th patches) there is no need for
callbacks. This allows having no ops either.

> 
> Anyway:
> 
> Acked-by: Hans Verkuil 

Thanks!

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


Re: [PATCH v8 06/21] v4l: fwnode: Support generic parsing of graph endpoints in a device

2017-09-07 Thread Sakari Ailus
Hi Hans,

On Wed, Sep 06, 2017 at 09:41:40AM +0200, Hans Verkuil wrote:
> On 09/05/2017 03:05 PM, Sakari Ailus wrote:
> > The current practice is that drivers iterate over their endpoints and
> > parse each endpoint separately. This is very similar in a number of
> > drivers, implement a generic function for the job. Driver specific matters
> > can be taken into account in the driver specific callback.
> > 
> > Signed-off-by: Sakari Ailus 
> > ---
> >  drivers/media/v4l2-core/v4l2-async.c  |  19 +
> >  drivers/media/v4l2-core/v4l2-fwnode.c | 140 
> > ++
> >  include/media/v4l2-async.h|  24 +-
> >  include/media/v4l2-fwnode.h   |  53 +
> >  4 files changed, 234 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-async.c 
> > b/drivers/media/v4l2-core/v4l2-async.c
> > index 3d81ff6a496f..7bd595c4094a 100644
> > --- a/drivers/media/v4l2-core/v4l2-async.c
> > +++ b/drivers/media/v4l2-core/v4l2-async.c
> > @@ -22,6 +22,7 @@
> >  
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  
> >  static bool match_i2c(struct v4l2_subdev *sd, struct v4l2_async_subdev 
> > *asd)
> > @@ -224,6 +225,24 @@ void v4l2_async_notifier_unregister(struct 
> > v4l2_async_notifier *notifier)
> >  }
> >  EXPORT_SYMBOL(v4l2_async_notifier_unregister);
> >  
> > +void v4l2_async_notifier_release(struct v4l2_async_notifier *notifier)
> > +{
> > +   unsigned int i;
> > +
> > +   if (!notifier->max_subdevs)
> > +   return;
> > +
> > +   for (i = 0; i < notifier->num_subdevs; i++)
> > +   kfree(notifier->subdevs[i]);
> > +
> > +   notifier->max_subdevs = 0;
> > +   notifier->num_subdevs = 0;
> > +
> > +   kvfree(notifier->subdevs);
> > +   notifier->subdevs = NULL;
> > +}
> > +EXPORT_SYMBOL_GPL(v4l2_async_notifier_release);
> > +
> >  int v4l2_async_register_subdev(struct v4l2_subdev *sd)
> >  {
> > struct v4l2_async_notifier *notifier;
> > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c 
> > b/drivers/media/v4l2-core/v4l2-fwnode.c
> > index 706f9e7b90f1..e6932d7d47b6 100644
> > --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> > @@ -19,6 +19,7 @@
> >   */
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -26,6 +27,7 @@
> >  #include 
> >  #include 
> >  
> > +#include 
> >  #include 
> >  
> >  enum v4l2_fwnode_bus_type {
> > @@ -313,6 +315,144 @@ void v4l2_fwnode_put_link(struct v4l2_fwnode_link 
> > *link)
> >  }
> >  EXPORT_SYMBOL_GPL(v4l2_fwnode_put_link);
> >  
> > +static int v4l2_async_notifier_realloc(struct v4l2_async_notifier 
> > *notifier,
> > +  unsigned int max_subdevs)
> > +{
> > +   struct v4l2_async_subdev **subdevs;
> > +
> > +   if (max_subdevs <= notifier->max_subdevs)
> > +   return 0;
> > +
> > +   subdevs = kvmalloc_array(
> > +   max_subdevs, sizeof(*notifier->subdevs),
> > +   GFP_KERNEL | __GFP_ZERO);
> > +   if (!subdevs)
> > +   return -ENOMEM;
> > +
> > +   if (notifier->subdevs) {
> > +   memcpy(subdevs, notifier->subdevs,
> > +  sizeof(*subdevs) * notifier->num_subdevs);
> > +
> > +   kvfree(notifier->subdevs);
> > +   }
> > +
> > +   notifier->subdevs = subdevs;
> > +   notifier->max_subdevs = max_subdevs;
> > +
> > +   return 0;
> > +}
> > +
> > +static int v4l2_async_notifier_fwnode_parse_endpoint(
> > +   struct device *dev, struct v4l2_async_notifier *notifier,
> > +   struct fwnode_handle *endpoint, unsigned int asd_struct_size,
> > +   int (*parse_endpoint)(struct device *dev,
> > +   struct v4l2_fwnode_endpoint *vep,
> > +   struct v4l2_async_subdev *asd))
> > +{
> > +   struct v4l2_async_subdev *asd;
> > +   struct v4l2_fwnode_endpoint *vep;
> > +   struct fwnode_endpoint ep;
> > +   int ret = 0;
> > +
> > +   asd = kzalloc(asd_struct_size, GFP_KERNEL);
> > +   if (!asd)
> > +   return -ENOMEM;
> > +
> > +   asd->match.fwnode.fwnode =
> > +   fwnode_graph_get_remote_port_parent(endpoint);
> > +   if (!asd->match.fwnode.fwnode) {
> > +   dev_warn(dev, "bad remote port parent\n");
> > +   ret = -EINVAL;
> > +   goto out_err;
> > +   }
> > +
> > +   /* Ignore endpoints the parsing of which failed. */
> > +   vep = v4l2_fwnode_endpoint_alloc_parse(endpoint);
> > +   if (IS_ERR(vep)) {
> > +   ret = PTR_ERR(vep);
> > +   dev_warn(dev, "unable to parse V4L2 fwnode endpoint (%d)\n",
> > +ret);
> > +   goto out_err;
> > +   }
> > +
> > +   ep = vep->base;
> > +
> > +   ret = parse_endpoint ? parse_endpoint(dev, vep, asd) : 0;
> > +   v4l2_fwnode_endpoint_free(vep);
> > +   if (ret == -ENOTCONN) {
> > +   dev_dbg(dev, "ignoring endpoint %u,%u\n", ep.port, ep.id);
> > +   kfree(asd);
> 
> Shouldn't there be a call to 

Re: BUGREPORT: IR keytable 1.12.3

2017-09-07 Thread Sean Young
Hi Oliver,

On Wed, Sep 06, 2017 at 09:20:14PM +0200, "Oliver Müller" wrote:
> BUG IR keytable 1.12.3
>  
> OS: Distributor ID:    Debian
> Description:    Debian GNU/Linux 9.1 (stretch)
> Release:    9.1
> Codename:    stretch
>  
> Kernel: 4.9.0-3-amd64 #1 SMP Debian 4.9.30-2+deb9u3 (2017-08-06) x86_64 
> GNU/Linux
>  
> Programversion: IR keytable control version 1.12.3
>  
> IR-Device: I: Bus=0003 Vendor=0471 Product=20cc Version=0100
>    N: Name="PHILIPS MCE USB IR Receiver- Spinel plus"
>    P: Phys=usb-:06:00.0-2/input0
>    S: 
> Sysfs=/devices/pci:00/:00:15.2/:06:00.0/usb1/1-2/1-2:1.0/0003:0471:20CC.0006/input/input14
>    U: Uniq=
>    H: Handlers=sysrq kbd leds event3
>    B: PROP=0
>    B: EV=120013
>    B: KEY=c 400 0 58000 8001f84000c004 e0beffdf01cf 
> fffe
>    B: MSC=10
>    B: LED=1f
>  
> ir-keytable gives /sys/class/rc/: No such file or directory

Could you please post the output of dmesg (or journalctl -b -k), it would be
useful to see the output of the usb probe.

> using ir-keytable -d /dev/input/event3 I get this output with no mention of 
> the protocol(s):
> Name: PHILIPS MCE USB IR Receiver- Spi
> bus: 3, vendor/product: 0471:20cc, version: 0x0100
>
> if I use ir-keytable -s rc0 instead it comes back to /sys/class/rc/: No such 
> file or directory which is also true
>  
> ir-keytable -d /dev/input/event3 -t works, ir-keytable -d /dev/input/event3 
> -r also works
>  
> after I introduce the new keymap, like so ir-keytable -d /dev/input/event3 -c 
> -w /etc/rc_keymaps/rc6_mce_zotac_zbox-ad05br it doesn't work. I can't read 
> the newly introduced keymap nor can I test it. Of course can't I be sure 
> which protocol to use because it's not displayed in the initial output.
>  
> thx in advance

Thanks

Sean