cron job: media_tree daily build: ERRORS

[Hans Verkuil]

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

Results of the daily build of media_tree:

date:   Thu Oct 20 05:00:15 CEST 2016
media-tree git hash:43ea43b9d8b27b7acd443ec59319faa3cdb8a616
media_build git hash:   dac8db4dd7fa3cc87715cb19ace554e080690b39
v4l-utils git hash: 79186f9d3d9d3b6bee4a611bd92435d11807
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.7.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: WARNINGS
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: ERRORS
linux-2.6.39.4-i686: WARNINGS
linux-3.0.60-i686: WARNINGS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: WARNINGS
linux-3.5.7-i686: WARNINGS
linux-3.6.11-i686: WARNINGS
linux-3.7.4-i686: WARNINGS
linux-3.8-i686: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: OK
linux-3.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-rc1-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: ERRORS
linux-2.6.39.4-x86_64: WARNINGS
linux-3.0.60-x86_64: WARNINGS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: WARNINGS
linux-3.5.7-x86_64: WARNINGS
linux-3.6.11-x86_64: WARNINGS
linux-3.7.4-x86_64: WARNINGS
linux-3.8-x86_64: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: OK
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: OK
linux-4.9-rc1-x86_64: OK
apps: WARNINGS
spec-git: OK
smatch: ERRORS
sparse: WARNINGS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

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

Re: [PATCH v2 53/58] firewire: don't break long lines

[Stefan Richter]

On Oct 19 Mauro Carvalho Chehab wrote:
> Em Wed, 19 Oct 2016 08:03:01 +0900
> Takashi Sakamoto  escreveu:
> > A structure for firedtv (struct firedtv) has a member for a pointer to
> > struct device. In this case, we can use dev_dbg() for debug printing.
[...]
> Stefan,
> 
> Would the above be OK for you?

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

Re: [PATCH v2 00/21] Basic i.MX IPUv3 capture support

[Sakari Ailus]

On Fri, Oct 14, 2016 at 07:34:20PM +0200, Philipp Zabel wrote:
> Hi,
> 
> the second round removes the prepare_stream callback and instead lets the
> intermediate subdevices propagate s_stream calls to their sources rather
> than individually calling s_stream on each subdevice from the bridge driver.
> This is similar to how drm bridges recursively call into their next neighbor.
> It makes it easier to do bringup ordering on a per-link level, as long as the
> source preparation can be done at s_power, and the sink can just prepare, call
> s_stream on its source, and then enable itself inside s_stream. Obviously this
> would only work in a generic fashion if all asynchronous subdevices with both
> inputs and outputs would propagate s_stream to their source subdevices.

Hi Philipp,

I'll review the async patches tomorrow / the day after. I have not forgotten
them. :-)

-- 
Kind regards,

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

Re: [PATCH 1/2] [media] vb2: Store dma_dir in vb2_queue

[Sakari Ailus]

Hi Thierry,

On Wed, Oct 19, 2016 at 10:24:16AM +0200, Thierry Escande wrote:
> From: Pawel Osciak 
> 
> Store dma_dir in struct vb2_queue and reuse it, instead of recalculating
> it each time.
> 
> Signed-off-by: Pawel Osciak 
> Tested-by: Pawel Osciak 
> Reviewed-by: Tomasz Figa 
> Reviewed-by: Owen Lin 
> Signed-off-by: Thierry Escande 
> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 12 +++-
>  drivers/media/v4l2-core/videobuf2-v4l2.c |  2 ++
>  include/media/videobuf2-core.h   |  2 ++
>  3 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
> b/drivers/media/v4l2-core/videobuf2-core.c
> index 21900202..f12103c 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -194,8 +194,6 @@ static void __enqueue_in_driver(struct vb2_buffer *vb);
>  static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>  {
>   struct vb2_queue *q = vb->vb2_queue;
> - enum dma_data_direction dma_dir =
> - q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>   void *mem_priv;
>   int plane;
>   int ret = -ENOMEM;
> @@ -209,7 +207,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>  
>   mem_priv = call_ptr_memop(vb, alloc,
>   q->alloc_devs[plane] ? : q->dev,
> - q->dma_attrs, size, dma_dir, q->gfp_flags);
> + q->dma_attrs, size, q->dma_dir, q->gfp_flags);

My bad, I guess I expressed myself unclearly.

Could you introduce the macro in this patch? You can then remove q->dma_dir
altogether.

-- 
Kind regards,

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

Re: [PATCH 2/3] Staging: media: radio-bcm2048: Fix indentation

[Mauro Carvalho Chehab]

Em Wed, 19 Oct 2016 22:47:13 +0200
Jean-Baptiste Abbadie  escreveu:

> Align multiple lines statement with parentheses

Looks OK to me. Greg, do you want to pick it on your tree or do you
prefer if I pick myself?

If you prefer to pick it:

Acked-by: Mauro Carvalho Chehab 

> 
> Signed-off-by: Jean-Baptiste Abbadie 
> ---
>  drivers/staging/media/bcm2048/radio-bcm2048.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
> b/drivers/staging/media/bcm2048/radio-bcm2048.c
> index 188d045d44ad..f66bea631e8e 100644
> --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
> +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
> @@ -997,7 +997,7 @@ static int bcm2048_set_fm_search_tune_mode(struct 
> bcm2048_device *bdev,
>   timeout = BCM2048_AUTO_SEARCH_TIMEOUT;
>  
>   if (!wait_for_completion_timeout(>compl,
> - msecs_to_jiffies(timeout)))
> +  msecs_to_jiffies(timeout)))
>   dev_err(>client->dev, "IRQ timeout.\n");
>  
>   if (value)
> @@ -2202,7 +2202,7 @@ static ssize_t bcm2048_fops_read(struct file *file, 
> char __user *buf,
>   }
>   /* interruptible_sleep_on(>read_queue); */
>   if (wait_event_interruptible(bdev->read_queue,
> - bdev->rds_data_available) < 0) {
> +  bdev->rds_data_available) < 0) {
>   retval = -EINTR;
>   goto done;
>   }



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

Re: [PATCH 3/3] Staging: media: radio-bcm2048: Remove FSF address from GPL notice

[Mauro Carvalho Chehab]

Em Wed, 19 Oct 2016 22:47:14 +0200
Jean-Baptiste Abbadie  escreveu:

> Removes the superfluous statement about writing to the FSF in the GPL
> notice

Looks OK to me. Greg, do you want to pick it on your tree or do you
prefer if I pick myself?

If you prefer to pick it:

Acked-by: Mauro Carvalho Chehab 

> 
> Signed-off-by: Jean-Baptiste Abbadie 
> ---
>  drivers/staging/media/bcm2048/radio-bcm2048.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
> b/drivers/staging/media/bcm2048/radio-bcm2048.c
> index f66bea631e8e..607dd5285149 100644
> --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
> +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
> @@ -17,10 +17,6 @@
>   * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>   * General Public License for more details.
>   *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> - * 02110-1301 USA
>   */
>  
>  /*



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

Re: [PATCH 1/3] Staging: media: radio-bcm2048: Fix symbolic permissions

[Mauro Carvalho Chehab]

Em Wed, 19 Oct 2016 22:47:12 +0200
Jean-Baptiste Abbadie  escreveu:

You should run get_maintainers.pl and check whomever submitted the driver, 
in order to get review. In particular, this driver looks to be
submitted by Hans, although I guess he didn't authored:

commit 899127b67df098e6d878f27be05dc91401cc6685
Author: Hans Verkuil 
Date:   Mon Nov 4 08:34:42 2013 -0300

[media] This adds support for the BCM2048 radio module found in Nokia N900

Add suport for Nokia N900 radio. This driver is far from being ready
to be added at the main tree, as it creates its own sysfs interface,
and violates lots of Coding Style rules, doing even evil things like
returning from a function inside a macro.

So, it is being added at staging with the condition that it will be
soon be fixed.



> This replaces the S_* style permissions by numbers for the __ATTR macros

I really prefer to see permissions like 0644, instead of those weird
S_* macros, as I can understand right away what is permitted.

> 
> Signed-off-by: Jean-Baptiste Abbadie 
> ---
>  drivers/staging/media/bcm2048/radio-bcm2048.c | 58 
> +--
>  1 file changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
> b/drivers/staging/media/bcm2048/radio-bcm2048.c
> index fe637ce8f4e7..188d045d44ad 100644
> --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
> +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
> @@ -2057,67 +2057,67 @@ property_signed_read(fm_rssi, int, "%d")
>  DEFINE_SYSFS_PROPERTY(region, unsigned, int, "%u", 0)
>  
>  static struct device_attribute attrs[] = {
> - __ATTR(power_state, S_IRUGO | S_IWUSR, bcm2048_power_state_read,
> + __ATTR(power_state, 0644, bcm2048_power_state_read,
>  bcm2048_power_state_write),
> - __ATTR(mute, S_IRUGO | S_IWUSR, bcm2048_mute_read,
> + __ATTR(mute, 0644, bcm2048_mute_read,
>  bcm2048_mute_write),
> - __ATTR(audio_route, S_IRUGO | S_IWUSR, bcm2048_audio_route_read,
> + __ATTR(audio_route, 0644, bcm2048_audio_route_read,
>  bcm2048_audio_route_write),
> - __ATTR(dac_output, S_IRUGO | S_IWUSR, bcm2048_dac_output_read,
> + __ATTR(dac_output, 0644, bcm2048_dac_output_read,
>  bcm2048_dac_output_write),
> - __ATTR(fm_hi_lo_injection, S_IRUGO | S_IWUSR,
> + __ATTR(fm_hi_lo_injection, 0644,
>  bcm2048_fm_hi_lo_injection_read,
>  bcm2048_fm_hi_lo_injection_write),
> - __ATTR(fm_frequency, S_IRUGO | S_IWUSR, bcm2048_fm_frequency_read,
> + __ATTR(fm_frequency, 0644, bcm2048_fm_frequency_read,
>  bcm2048_fm_frequency_write),
> - __ATTR(fm_af_frequency, S_IRUGO | S_IWUSR,
> + __ATTR(fm_af_frequency, 0644,
>  bcm2048_fm_af_frequency_read,
>  bcm2048_fm_af_frequency_write),
> - __ATTR(fm_deemphasis, S_IRUGO | S_IWUSR, bcm2048_fm_deemphasis_read,
> + __ATTR(fm_deemphasis, 0644, bcm2048_fm_deemphasis_read,
>  bcm2048_fm_deemphasis_write),
> - __ATTR(fm_rds_mask, S_IRUGO | S_IWUSR, bcm2048_fm_rds_mask_read,
> + __ATTR(fm_rds_mask, 0644, bcm2048_fm_rds_mask_read,
>  bcm2048_fm_rds_mask_write),
> - __ATTR(fm_best_tune_mode, S_IRUGO | S_IWUSR,
> + __ATTR(fm_best_tune_mode, 0644,
>  bcm2048_fm_best_tune_mode_read,
>  bcm2048_fm_best_tune_mode_write),
> - __ATTR(fm_search_rssi_threshold, S_IRUGO | S_IWUSR,
> + __ATTR(fm_search_rssi_threshold, 0644,
>  bcm2048_fm_search_rssi_threshold_read,
>  bcm2048_fm_search_rssi_threshold_write),
> - __ATTR(fm_search_mode_direction, S_IRUGO | S_IWUSR,
> + __ATTR(fm_search_mode_direction, 0644,
>  bcm2048_fm_search_mode_direction_read,
>  bcm2048_fm_search_mode_direction_write),
> - __ATTR(fm_search_tune_mode, S_IRUGO | S_IWUSR,
> + __ATTR(fm_search_tune_mode, 0644,
>  bcm2048_fm_search_tune_mode_read,
>  bcm2048_fm_search_tune_mode_write),
> - __ATTR(rds, S_IRUGO | S_IWUSR, bcm2048_rds_read,
> + __ATTR(rds, 0644, bcm2048_rds_read,
>  bcm2048_rds_write),
> - __ATTR(rds_b_block_mask, S_IRUGO | S_IWUSR,
> + __ATTR(rds_b_block_mask, 0644,
>  bcm2048_rds_b_block_mask_read,
>  bcm2048_rds_b_block_mask_write),
> - __ATTR(rds_b_block_match, S_IRUGO | S_IWUSR,
> + __ATTR(rds_b_block_match, 0644,
>  bcm2048_rds_b_block_match_read,
>  bcm2048_rds_b_block_match_write),
> - __ATTR(rds_pi_mask, S_IRUGO | S_IWUSR, bcm2048_rds_pi_mask_read,
> + __ATTR(rds_pi_mask, 0644, bcm2048_rds_pi_mask_read,
>  bcm2048_rds_pi_mask_write),
> - __ATTR(rds_pi_match, S_IRUGO | S_IWUSR, bcm2048_rds_pi_match_read,
> + __ATTR(rds_pi_match, 0644, bcm2048_rds_pi_match_read,
>  bcm2048_rds_pi_match_write),
> - 

[PATCH 1/3] Staging: media: radio-bcm2048: Fix symbolic permissions

[Jean-Baptiste Abbadie]

This replaces the S_* style permissions by numbers for the __ATTR macros

Signed-off-by: Jean-Baptiste Abbadie 
---
 drivers/staging/media/bcm2048/radio-bcm2048.c | 58 +--
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index fe637ce8f4e7..188d045d44ad 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -2057,67 +2057,67 @@ property_signed_read(fm_rssi, int, "%d")
 DEFINE_SYSFS_PROPERTY(region, unsigned, int, "%u", 0)
 
 static struct device_attribute attrs[] = {
-   __ATTR(power_state, S_IRUGO | S_IWUSR, bcm2048_power_state_read,
+   __ATTR(power_state, 0644, bcm2048_power_state_read,
   bcm2048_power_state_write),
-   __ATTR(mute, S_IRUGO | S_IWUSR, bcm2048_mute_read,
+   __ATTR(mute, 0644, bcm2048_mute_read,
   bcm2048_mute_write),
-   __ATTR(audio_route, S_IRUGO | S_IWUSR, bcm2048_audio_route_read,
+   __ATTR(audio_route, 0644, bcm2048_audio_route_read,
   bcm2048_audio_route_write),
-   __ATTR(dac_output, S_IRUGO | S_IWUSR, bcm2048_dac_output_read,
+   __ATTR(dac_output, 0644, bcm2048_dac_output_read,
   bcm2048_dac_output_write),
-   __ATTR(fm_hi_lo_injection, S_IRUGO | S_IWUSR,
+   __ATTR(fm_hi_lo_injection, 0644,
   bcm2048_fm_hi_lo_injection_read,
   bcm2048_fm_hi_lo_injection_write),
-   __ATTR(fm_frequency, S_IRUGO | S_IWUSR, bcm2048_fm_frequency_read,
+   __ATTR(fm_frequency, 0644, bcm2048_fm_frequency_read,
   bcm2048_fm_frequency_write),
-   __ATTR(fm_af_frequency, S_IRUGO | S_IWUSR,
+   __ATTR(fm_af_frequency, 0644,
   bcm2048_fm_af_frequency_read,
   bcm2048_fm_af_frequency_write),
-   __ATTR(fm_deemphasis, S_IRUGO | S_IWUSR, bcm2048_fm_deemphasis_read,
+   __ATTR(fm_deemphasis, 0644, bcm2048_fm_deemphasis_read,
   bcm2048_fm_deemphasis_write),
-   __ATTR(fm_rds_mask, S_IRUGO | S_IWUSR, bcm2048_fm_rds_mask_read,
+   __ATTR(fm_rds_mask, 0644, bcm2048_fm_rds_mask_read,
   bcm2048_fm_rds_mask_write),
-   __ATTR(fm_best_tune_mode, S_IRUGO | S_IWUSR,
+   __ATTR(fm_best_tune_mode, 0644,
   bcm2048_fm_best_tune_mode_read,
   bcm2048_fm_best_tune_mode_write),
-   __ATTR(fm_search_rssi_threshold, S_IRUGO | S_IWUSR,
+   __ATTR(fm_search_rssi_threshold, 0644,
   bcm2048_fm_search_rssi_threshold_read,
   bcm2048_fm_search_rssi_threshold_write),
-   __ATTR(fm_search_mode_direction, S_IRUGO | S_IWUSR,
+   __ATTR(fm_search_mode_direction, 0644,
   bcm2048_fm_search_mode_direction_read,
   bcm2048_fm_search_mode_direction_write),
-   __ATTR(fm_search_tune_mode, S_IRUGO | S_IWUSR,
+   __ATTR(fm_search_tune_mode, 0644,
   bcm2048_fm_search_tune_mode_read,
   bcm2048_fm_search_tune_mode_write),
-   __ATTR(rds, S_IRUGO | S_IWUSR, bcm2048_rds_read,
+   __ATTR(rds, 0644, bcm2048_rds_read,
   bcm2048_rds_write),
-   __ATTR(rds_b_block_mask, S_IRUGO | S_IWUSR,
+   __ATTR(rds_b_block_mask, 0644,
   bcm2048_rds_b_block_mask_read,
   bcm2048_rds_b_block_mask_write),
-   __ATTR(rds_b_block_match, S_IRUGO | S_IWUSR,
+   __ATTR(rds_b_block_match, 0644,
   bcm2048_rds_b_block_match_read,
   bcm2048_rds_b_block_match_write),
-   __ATTR(rds_pi_mask, S_IRUGO | S_IWUSR, bcm2048_rds_pi_mask_read,
+   __ATTR(rds_pi_mask, 0644, bcm2048_rds_pi_mask_read,
   bcm2048_rds_pi_mask_write),
-   __ATTR(rds_pi_match, S_IRUGO | S_IWUSR, bcm2048_rds_pi_match_read,
+   __ATTR(rds_pi_match, 0644, bcm2048_rds_pi_match_read,
   bcm2048_rds_pi_match_write),
-   __ATTR(rds_wline, S_IRUGO | S_IWUSR, bcm2048_rds_wline_read,
+   __ATTR(rds_wline, 0644, bcm2048_rds_wline_read,
   bcm2048_rds_wline_write),
-   __ATTR(rds_pi, S_IRUGO, bcm2048_rds_pi_read, NULL),
-   __ATTR(rds_rt, S_IRUGO, bcm2048_rds_rt_read, NULL),
-   __ATTR(rds_ps, S_IRUGO, bcm2048_rds_ps_read, NULL),
-   __ATTR(fm_rds_flags, S_IRUGO, bcm2048_fm_rds_flags_read, NULL),
-   __ATTR(region_bottom_frequency, S_IRUGO,
+   __ATTR(rds_pi, 0444, bcm2048_rds_pi_read, NULL),
+   __ATTR(rds_rt, 0444, bcm2048_rds_rt_read, NULL),
+   __ATTR(rds_ps, 0444, bcm2048_rds_ps_read, NULL),
+   __ATTR(fm_rds_flags, 0444, bcm2048_fm_rds_flags_read, NULL),
+   __ATTR(region_bottom_frequency, 0444,
   bcm2048_region_bottom_frequency_read, NULL),
-   __ATTR(region_top_frequency, S_IRUGO,
+   __ATTR(region_top_frequency, 0444,
   bcm2048_region_top_frequency_read, NULL),
-   __ATTR(fm_carrier_error, S_IRUGO,
+   __ATTR(fm_carrier_error, 0444,

[PATCH 2/3] Staging: media: radio-bcm2048: Fix indentation

[Jean-Baptiste Abbadie]

Align multiple lines statement with parentheses

Signed-off-by: Jean-Baptiste Abbadie 
---
 drivers/staging/media/bcm2048/radio-bcm2048.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index 188d045d44ad..f66bea631e8e 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -997,7 +997,7 @@ static int bcm2048_set_fm_search_tune_mode(struct 
bcm2048_device *bdev,
timeout = BCM2048_AUTO_SEARCH_TIMEOUT;
 
if (!wait_for_completion_timeout(>compl,
-   msecs_to_jiffies(timeout)))
+msecs_to_jiffies(timeout)))
dev_err(>client->dev, "IRQ timeout.\n");
 
if (value)
@@ -2202,7 +2202,7 @@ static ssize_t bcm2048_fops_read(struct file *file, char 
__user *buf,
}
/* interruptible_sleep_on(>read_queue); */
if (wait_event_interruptible(bdev->read_queue,
-   bdev->rds_data_available) < 0) {
+bdev->rds_data_available) < 0) {
retval = -EINTR;
goto done;
}
-- 
2.10.0

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

[PATCH 3/3] Staging: media: radio-bcm2048: Remove FSF address from GPL notice

[Jean-Baptiste Abbadie]

Removes the superfluous statement about writing to the FSF in the GPL
notice

Signed-off-by: Jean-Baptiste Abbadie 
---
 drivers/staging/media/bcm2048/radio-bcm2048.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index f66bea631e8e..607dd5285149 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -17,10 +17,6 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * General Public License for more details.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA
  */
 
 /*
-- 
2.10.0

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

[PATCH 0/3] media: radio-bcm2048: multiple small cleanups

[Jean-Baptiste Abbadie]

Hello,

This is a series of minor patches to fix checkpatch.pl issues.

Regards,
Jean-Baptiste

Jean-Baptiste Abbadie (3):
  Staging: media: radio-bcm2048: Fix symbolic permissions
  Staging: media: radio-bcm2048: Fix indentation
  Staging: media: radio-bcm2048: Remove FSF address from GPL notice

 drivers/staging/media/bcm2048/radio-bcm2048.c | 66 +--
 1 file changed, 31 insertions(+), 35 deletions(-)

-- 
2.10.0

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

Re: [PATCH 2/3] Staging: media: radio-bcm2048: Fix alignment issues

[Johannes Stezenbach]

On Wed, Oct 19, 2016 at 07:17:12PM +0200, Jean-Baptiste Abbadie wrote:
> Signed-off-by: Jean-Baptiste Abbadie 
> ---
>  drivers/staging/media/bcm2048/radio-bcm2048.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
> b/drivers/staging/media/bcm2048/radio-bcm2048.c
> index 188d045d44ad..f66bea631e8e 100644
> --- a/drivers/staging/media/bcm2048/radio-bcm2048.c
> +++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
> @@ -997,7 +997,7 @@ static int bcm2048_set_fm_search_tune_mode(struct 
> bcm2048_device *bdev,
>   timeout = BCM2048_AUTO_SEARCH_TIMEOUT;
>  
>   if (!wait_for_completion_timeout(>compl,
> - msecs_to_jiffies(timeout)))
> +  msecs_to_jiffies(timeout)))
>   dev_err(>client->dev, "IRQ timeout.\n");
>  
>   if (value)
> @@ -2202,7 +2202,7 @@ static ssize_t bcm2048_fops_read(struct file *file, 
> char __user *buf,
>   }
>   /* interruptible_sleep_on(>read_queue); */
>   if (wait_event_interruptible(bdev->read_queue,
> - bdev->rds_data_available) < 0) {
> +  bdev->rds_data_available) < 0) {

FWIW, a better Subject: would be "fix indentation" because
"alignment issue" usually means some address not
aligned to some border.

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

Re: [PATCH v2 08/21] [media] imx: Add i.MX IPUv3 capture driver

[Marek Vasut]

On 10/19/2016 06:22 PM, Jack Mitchell wrote:
> Hi Philipp,
> 
> On 17/10/16 13:12, Philipp Zabel wrote:
>> Hi Jack,
>>
>> Am Montag, den 17.10.2016, 12:32 +0100 schrieb Jack Mitchell:
>>> Hi Philipp,
>>>
>>> I'm looking at how I would enable a parallel greyscale camera using this
>>> set of drivers and am a little bit confused. Do you have an example
>>> somewhere of a devicetree with an input node.
>>
>> In your board device tree it should look somewhat like this:
>>
>>  {
>> sensor@48 {
>> compatible = "aptina,mt9v032m";
>> /* ... */
>>
>> port {
>> cam_out: endpoint {
>> remote-endpoint = <_in>;
>> }
>> };
>> };
>> };
>>
>> /*
>>  * This is the input port node corresponding to the 'CSI0' pad group,
>>  * not necessarily the CSI0 port of IPU1 or IPU2. On i.MX6Q it's port@1
>>  * of the mipi_ipu1_mux, on i.MX6DL it's port@4 of the ipu_csi0_mux,
>>  * the csi0 label is added in patch 13/21.
>>  */
>>  {
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> csi_in: endpoint@0 {
>> bus-width = <8>;
>> data-shift = <12>;
>> hsync-active = <1>;
>> vsync-active = <1>;
>> pclk-sample = <1>;
>> remote-endpoint = <_out>;
>> };
>> };
>>
>>>  I also have a further note below:
>> [...]
 +if (raw && priv->smfc) {
>>>
> 
> Thank you, I think I have something which is kind of right.
> 
> (Apologies in advance for the formatting)
> 
> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts
> b/arch/arm/boot/dts/imx6q-sabrelite.dts
> index 66d10d8..90e6b92 100644
> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> @@ -52,3 +52,62 @@
>   {
>  status = "okay";
>  };
> +
> + {
> +sensor@10 {
> +compatible = "onsemi,ar0135";
> +reg = <0x10>;
> +
> +pinctrl-names = "default";
> +pinctrl-0 = <_ar0135>;
> +
> +reset-gpio = < 6 GPIO_ACTIVE_HIGH>;
> +
> +clocks = < IMX6QDL_CLK_CKO2>;
> +clock-names = "xclk";
> +
> +xclk = <2400>;
> +
> +port {
> +parallel_camera_output: endpoint {
> +remote-endpoint = <_in_from_parallel_camera>;
> +};
> +};
> +};
> +};
> +
> + {
> +csi_in_from_parallel_camera: endpoint@0 {
> +bus-width = <8>;
> +data-shift = <12>;
> +hsync-active = <1>;
> +vsync-active = <1>;
> +pclk-sample = <1>;
> +remote-endpoint = <_camera_output>;
> +};
> +};
> +
> + {
> +
> +imx6q-sabrelite {
> +
> +pinctrl_ar0135: ar0135grp {
> +fsl,pins = <
> +MX6QDL_PAD_GPIO_6__GPIO1_IO06   0x8000
> +MX6QDL_PAD_CSI0_DAT12__IPU1_CSI0_DATA120x8000
> +MX6QDL_PAD_CSI0_DAT13__IPU1_CSI0_DATA130x8000
> +MX6QDL_PAD_CSI0_DAT14__IPU1_CSI0_DATA140x8000
> +MX6QDL_PAD_CSI0_DAT15__IPU1_CSI0_DATA150x8000
> +MX6QDL_PAD_CSI0_DAT16__IPU1_CSI0_DATA160x8000
> +MX6QDL_PAD_CSI0_DAT17__IPU1_CSI0_DATA170x8000
> +MX6QDL_PAD_CSI0_DAT18__IPU1_CSI0_DATA180x8000
> +MX6QDL_PAD_CSI0_DAT19__IPU1_CSI0_DATA190x8000
> +MX6QDL_PAD_CSI0_PIXCLK__IPU1_CSI0_PIXCLK   0x8000
> +MX6QDL_PAD_CSI0_MCLK__IPU1_CSI0_HSYNC  0x8000
> +MX6QDL_PAD_CSI0_VSYNC__IPU1_CSI0_VSYNC 0x8000
> +MX6QDL_PAD_CSI0_DATA_EN__IPU1_CSI0_DATA_EN 0x8000
> +>;
> +};
> +};
> +};
> 
> 
> However, I can't seem to link the entities together properly, am I
> missing something obvious?
> 
> root@vicon:~# media-ctl -p
> Media controller API version 0.1.0
> 
> Media device information
> 
> driver  imx-media
> model   i.MX IPUv3
> serial
> bus info
> hw revision 0x0
> driver version  0.0.0
> 
> Device topology
> - entity 1: IPU0 CSI0 (2 pads, 1 link)
> type V4L2 subdev subtype Unknown flags 0
> pad0: Sink
> pad1: Source
> -> "imx-ipuv3-capture.0":0 [ENABLED]
> 
> - entity 4: imx-ipuv3-capture.0 (1 pad, 1 link)
> type Node subtype V4L flags 0
> device node name /dev/video0
> pad0: Sink
> <- "IPU0 CSI0":1 [ENABLED]
> 
> - entity 10: IPU0 CSI1 (2 pads, 1 link)
>  type V4L2 subdev subtype Unknown flags 0
> pad0: Sink
> pad1: Source
> -> "imx-ipuv3-capture.1":0 [ENABLED]
> 
> - entity 13: imx-ipuv3-capture.1 (1 pad, 1 link)
>  type Node subtype V4L flags 0
>  device node name /dev/video1
> pad0: Sink
> <- "IPU0 CSI1":1 [ENABLED]
> 
> - entity 19: IPU1 CSI0 (2 pads, 1 link)
>  type V4L2 subdev subtype Unknown flags 0
> pad0: Sink
> pad1: Source
> -> "imx-ipuv3-capture.0":0 [ENABLED]
> 
> - entity 22: 

Re: em28xx WinTV dualHD in Raspbian

[Mauro Carvalho Chehab]

Em Wed, 19 Oct 2016 20:50:09 +0200
 escreveu:

> > Based on this log:
> > 
> > Oct 18 23:08:01 mediapi kernel: [ 7590.369200] em28xx_dvb: disagrees about 
> > version of symbol dvb_dmxdev_init Oct 18 23:08:01 mediapi kernel: [ 
> > 7590.369228] em28xx_dvb: Unknown symbol dvb_dmxdev_init (err -22)
> > 
> > it seems you messed the modules install or you have the V4L2 stack compiled 
> > builtin with a different version.   
> 
> How to fix this?
> 
> - I reinstalled the current firmware and kernel on the raspberry.
> - I installed the headers with sudo apt-get install raspberrypi-kernel-headers
> - Then I have cloned, build and installed the modules:
> 
> git clone git://linuxtv.org/media_build.git
> cd media_build 
> ./build
> sudo make install
> 
> But the same errors appear again.

There are 3 options:

1) If the V4L core is builtin on your Kernel [1], then you'll
need recompile the Kernel yourself and be sure that V4L will be
disabled. Then, you can build V4L2 using the media_build tree.
You'll likely need to fix the DRM driver dependencies through,
by modifying the Kernel source code (actually, some Kconfig file).

2) To use the em28xx driver that came at the RPi Kernel,
instead of using media_build.git. You'll also need to recompile
the RPi proprietary Kernel from its sources;

3) use the upstream Kernel. If I remember correctly, the VC4 driver
(needed for GPU support) was added on Kernel 4.7. If you're not using
a camera sensor connected to your RPi, the VC4 driver should be enough
for your needs.

[1] If you're using the RPi proprietary Kernel, I bet you have the
V4L2 core built in. I remember I found the same issue last year, while
playing with RPi proprietary Kernel - part of their GPU driver was
depending  on CONFIG_MEDIA - I think - for no good reason. I used to 
have a patch fixing it somewhere, but I was unable to find it.

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



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

AW: em28xx WinTV dualHD in Raspbian

[ps00de]

> Based on this log:
> 
> Oct 18 23:08:01 mediapi kernel: [ 7590.369200] em28xx_dvb: disagrees about 
> version of symbol dvb_dmxdev_init Oct 18 23:08:01 mediapi kernel: [ 
> 7590.369228] em28xx_dvb: Unknown symbol dvb_dmxdev_init (err -22)
>
> It seems you messed the modules install or you have the V4L2 stack compiled 
> builtin with a different version. 

I've done a cleanup: sudo rm -rf /lib/modules/`uname -r`/kernel/drivers/media/
And reinstalled v4l (make install) again.

Same result (no /dev/dvb), but other log:
Oct 19 20:59:54 mediapi kernel: [7.515009] media: Linux media interface: 
v0.10
Oct 19 20:59:54 mediapi kernel: [7.537922] Linux video capture interface: 
v2.00
Oct 19 20:59:54 mediapi kernel: [7.554643] em28xx: New device HCW dualHD @ 
480 Mbps (2040:0265, interface 0, class 0)
Oct 19 20:59:54 mediapi kernel: [7.554666] em28xx: DVB interface 0 found: 
isoc
Oct 19 20:59:54 mediapi kernel: [7.554959] em28xx: chip ID is em28174
Oct 19 20:59:54 mediapi kernel: [8.752360] em28174 #0: EEPROM ID = 26 00 01 
00, EEPROM hash = 0x7ee3cbc8
Oct 19 20:59:54 mediapi kernel: [8.752378] em28174 #0: EEPROM info:
Oct 19 20:59:54 mediapi kernel: [8.752387] em28174 #0:  microcode start 
address = 0x0004, boot configuration = 0x01
Oct 19 20:59:54 mediapi kernel: [8.758923] em28174 #0:  AC97 audio (5 
sample rates)
Oct 19 20:59:54 mediapi kernel: [8.758936] em28174 #0:  500mA max power
Oct 19 20:59:54 mediapi kernel: [8.758946] em28174 #0:  Table at offset 
0x27, strings=0x0e6a, 0x1888, 0x087e
Oct 19 20:59:54 mediapi kernel: [8.759389] em28174 #0: Identified as 
Hauppauge WinTV-dualHD DVB (card=99)
Oct 19 20:59:54 mediapi kernel: [8.764360] tveeprom 4-0050: Hauppauge model 
204109, rev B2I6, serial# 11540068
Oct 19 20:59:54 mediapi kernel: [8.764381] tveeprom 4-0050: tuner model is 
SiLabs Si2157 (idx 186, type 4)
Oct 19 20:59:54 mediapi kernel: [8.764394] tveeprom 4-0050: TV standards 
PAL(B/G) NTSC(M) PAL(I) SECAM(L/L') PAL(D/D1/K) ATSC/DVB Digital (eeprom 0xfc)
Oct 19 20:59:54 mediapi kernel: [8.764404] tveeprom 4-0050: audio processor 
is None (idx 0)
Oct 19 20:59:54 mediapi kernel: [8.764414] tveeprom 4-0050: has no radio, 
has IR receiver, has no IR transmitter
Oct 19 20:59:54 mediapi kernel: [8.764424] em28174 #0: dvb set to isoc mode.
Oct 19 20:59:54 mediapi kernel: [8.764854] usbcore: registered new 
interface driver em28xx
Oct 19 20:59:54 mediapi kernel: [8.800215] em28174 #0: Registering input 
extension
Oct 19 20:59:54 mediapi kernel: [8.835377] Registered IR keymap rc-hauppauge
Oct 19 20:59:54 mediapi kernel: [8.836063] input: em28xx IR (em28174 #0) as 
/devices/platform/soc/3f98.usb/usb1/1-1/1-1.3/rc/rc0/input0
Oct 19 20:59:54 mediapi kernel: [8.836345] rc rc0: em28xx IR (em28174 #0) 
as /devices/platform/soc/3f98.usb/usb1/1-1/1-1.3/rc/rc0
Oct 19 20:59:54 mediapi kernel: [8.838384] em28174 #0: Input extension 
successfully initalized
Oct 19 20:59:54 mediapi kernel: [8.838406] em28xx: Registered (Em28xx Input 
Extension) extension
Oct 19 20:59:54 mediapi kernel: [9.612549] Adding 102396k swap on 
/var/swap.  Priority:-1 extents:3 across:200700k SSFS

Any idea?

Thanks,
Patrick

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

Re: em28xx WinTV dualHD in Raspbian

[ps00de]

> Based on this log:
> 
> Oct 18 23:08:01 mediapi kernel: [ 7590.369200] em28xx_dvb: disagrees about 
> version of symbol dvb_dmxdev_init Oct 18 23:08:01 mediapi kernel: [ 
> 7590.369228] em28xx_dvb: Unknown symbol dvb_dmxdev_init (err -22)
> 
> it seems you messed the modules install or you have the V4L2 stack compiled 
> builtin with a different version. 

How to fix this?

- I reinstalled the current firmware and kernel on the raspberry.
- I installed the headers with sudo apt-get install raspberrypi-kernel-headers
- Then I have cloned, build and installed the modules:

git clone git://linuxtv.org/media_build.git
cd media_build 
./build
sudo make install

But the same errors appear again.

Thanks,
Patrick

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

Re: [PATCH 2/3] Staging: media: radio-bcm2048: Fix alignment issues

[Greg Kroah-Hartman]

On Wed, Oct 19, 2016 at 08:10:25PM +0200, Jean-Baptiste Abbadie wrote:
> On 19/10/16 19:51, Greg Kroah-Hartman wrote:
> > I can't take a patch with no changelog text, sorry.
> Hello,
> 
> Should I add the changelog in the same thread or start a new thread ?

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

Re: [PATCH v2 54/58] i2c: don't break long lines

[Lad, Prabhakar]

Hi Mauro,

Thanks for the patch.


On Tue, Oct 18, 2016 at 9:46 PM, Mauro Carvalho Chehab
 wrote:
> Due to the 80-cols restrictions, and latter due to checkpatch
> warnings, several strings were broken into multiple lines. This
> is not considered a good practice anymore, as it makes harder
> to grep for strings at the source code.
>
> As we're right now fixing other drivers due to KERN_CONT, we need
> to be able to identify what printk strings don't end with a "\n".
> It is a way easier to detect those if we don't break long lines.
>
> So, join those continuation lines.
>
> The patch was generated via the script below, and manually
> adjusted if needed.
>
> 
> use Text::Tabs;
> while (<>) {
> if ($next ne "") {
> $c=$_;
> if ($c =~ /^\s+\"(.*)/) {
> $c2=$1;
> $next =~ s/\"\n$//;
> $n = expand($next);
> $funpos = index($n, '(');
> $pos = index($c2, '",');
> if ($funpos && $pos > 0) {
> $s1 = substr $c2, 0, $pos + 2;
> $s2 = ' ' x ($funpos + 1) . substr $c2, $pos 
> + 2;
> $s2 =~ s/^\s+//;
>
> $s2 = ' ' x ($funpos + 1) . $s2 if ($s2 ne 
> "");
>
> print unexpand("$next$s1\n");
> print unexpand("$s2\n") if ($s2 ne "");
> } else {
> print "$next$c2\n";
> }
> $next="";
> next;
> } else {
> print $next;
> }
> $next="";
> } else {
> if (m/\"$/) {
> if (!m/\\n\"$/) {
> $next=$_;
> next;
> }
> }
> }
> print $_;
> }
> 
>
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Lad, Prabhakar 

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

Re: [PATCH 2/3] Staging: media: radio-bcm2048: Fix alignment issues

[Jean-Baptiste Abbadie]

On 19/10/16 19:51, Greg Kroah-Hartman wrote:
I can't take a patch with no changelog text, sorry. 

Hello,

Should I add the changelog in the same thread or start a new thread ?

Sorry, for the mistakes that's my first submitted patch.

Regards,

Jean-Baptiste

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

Re: [PATCH 2/3] Staging: media: radio-bcm2048: Fix alignment issues

[Greg Kroah-Hartman]

On Wed, Oct 19, 2016 at 07:17:12PM +0200, Jean-Baptiste Abbadie wrote:
> Signed-off-by: Jean-Baptiste Abbadie 

I can't take a patch with no changelog text, sorry.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags

[Dave Hansen]

On 10/19/2016 10:01 AM, Michal Hocko wrote:
> The question I had earlier was whether this has to be an explicit FOLL
> flag used by g-u-p users or we can just use it internally when mm !=
> current->mm

The reason I chose not to do that was that deferred work gets run under
a basically random 'current'.  If we just use 'mm != current->mm', then
the deferred work will sometimes have pkeys enforced and sometimes not,
basically randomly.

We want to be consistent with whether they are enforced or not, so we
explicitly indicate that by calling the remote variant vs. plain.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] Staging: media: radio-bcm2048: Remove FSF address from GPL notice

[Jean-Baptiste Abbadie]

Signed-off-by: Jean-Baptiste Abbadie 
---
 drivers/staging/media/bcm2048/radio-bcm2048.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index f66bea631e8e..607dd5285149 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -17,10 +17,6 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * General Public License for more details.
  *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA
  */
 
 /*
-- 
2.10.0

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

[PATCH 1/3] Staging: media: radio-bcm2048: Fix symbolic permissions

[Jean-Baptiste Abbadie]

This is a series of minor patch to fix checkpatch.pl issues.

Signed-off-by: Jean-Baptiste Abbadie 
---
 drivers/staging/media/bcm2048/radio-bcm2048.c | 58 +--
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index fe637ce8f4e7..188d045d44ad 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -2057,67 +2057,67 @@ property_signed_read(fm_rssi, int, "%d")
 DEFINE_SYSFS_PROPERTY(region, unsigned, int, "%u", 0)
 
 static struct device_attribute attrs[] = {
-   __ATTR(power_state, S_IRUGO | S_IWUSR, bcm2048_power_state_read,
+   __ATTR(power_state, 0644, bcm2048_power_state_read,
   bcm2048_power_state_write),
-   __ATTR(mute, S_IRUGO | S_IWUSR, bcm2048_mute_read,
+   __ATTR(mute, 0644, bcm2048_mute_read,
   bcm2048_mute_write),
-   __ATTR(audio_route, S_IRUGO | S_IWUSR, bcm2048_audio_route_read,
+   __ATTR(audio_route, 0644, bcm2048_audio_route_read,
   bcm2048_audio_route_write),
-   __ATTR(dac_output, S_IRUGO | S_IWUSR, bcm2048_dac_output_read,
+   __ATTR(dac_output, 0644, bcm2048_dac_output_read,
   bcm2048_dac_output_write),
-   __ATTR(fm_hi_lo_injection, S_IRUGO | S_IWUSR,
+   __ATTR(fm_hi_lo_injection, 0644,
   bcm2048_fm_hi_lo_injection_read,
   bcm2048_fm_hi_lo_injection_write),
-   __ATTR(fm_frequency, S_IRUGO | S_IWUSR, bcm2048_fm_frequency_read,
+   __ATTR(fm_frequency, 0644, bcm2048_fm_frequency_read,
   bcm2048_fm_frequency_write),
-   __ATTR(fm_af_frequency, S_IRUGO | S_IWUSR,
+   __ATTR(fm_af_frequency, 0644,
   bcm2048_fm_af_frequency_read,
   bcm2048_fm_af_frequency_write),
-   __ATTR(fm_deemphasis, S_IRUGO | S_IWUSR, bcm2048_fm_deemphasis_read,
+   __ATTR(fm_deemphasis, 0644, bcm2048_fm_deemphasis_read,
   bcm2048_fm_deemphasis_write),
-   __ATTR(fm_rds_mask, S_IRUGO | S_IWUSR, bcm2048_fm_rds_mask_read,
+   __ATTR(fm_rds_mask, 0644, bcm2048_fm_rds_mask_read,
   bcm2048_fm_rds_mask_write),
-   __ATTR(fm_best_tune_mode, S_IRUGO | S_IWUSR,
+   __ATTR(fm_best_tune_mode, 0644,
   bcm2048_fm_best_tune_mode_read,
   bcm2048_fm_best_tune_mode_write),
-   __ATTR(fm_search_rssi_threshold, S_IRUGO | S_IWUSR,
+   __ATTR(fm_search_rssi_threshold, 0644,
   bcm2048_fm_search_rssi_threshold_read,
   bcm2048_fm_search_rssi_threshold_write),
-   __ATTR(fm_search_mode_direction, S_IRUGO | S_IWUSR,
+   __ATTR(fm_search_mode_direction, 0644,
   bcm2048_fm_search_mode_direction_read,
   bcm2048_fm_search_mode_direction_write),
-   __ATTR(fm_search_tune_mode, S_IRUGO | S_IWUSR,
+   __ATTR(fm_search_tune_mode, 0644,
   bcm2048_fm_search_tune_mode_read,
   bcm2048_fm_search_tune_mode_write),
-   __ATTR(rds, S_IRUGO | S_IWUSR, bcm2048_rds_read,
+   __ATTR(rds, 0644, bcm2048_rds_read,
   bcm2048_rds_write),
-   __ATTR(rds_b_block_mask, S_IRUGO | S_IWUSR,
+   __ATTR(rds_b_block_mask, 0644,
   bcm2048_rds_b_block_mask_read,
   bcm2048_rds_b_block_mask_write),
-   __ATTR(rds_b_block_match, S_IRUGO | S_IWUSR,
+   __ATTR(rds_b_block_match, 0644,
   bcm2048_rds_b_block_match_read,
   bcm2048_rds_b_block_match_write),
-   __ATTR(rds_pi_mask, S_IRUGO | S_IWUSR, bcm2048_rds_pi_mask_read,
+   __ATTR(rds_pi_mask, 0644, bcm2048_rds_pi_mask_read,
   bcm2048_rds_pi_mask_write),
-   __ATTR(rds_pi_match, S_IRUGO | S_IWUSR, bcm2048_rds_pi_match_read,
+   __ATTR(rds_pi_match, 0644, bcm2048_rds_pi_match_read,
   bcm2048_rds_pi_match_write),
-   __ATTR(rds_wline, S_IRUGO | S_IWUSR, bcm2048_rds_wline_read,
+   __ATTR(rds_wline, 0644, bcm2048_rds_wline_read,
   bcm2048_rds_wline_write),
-   __ATTR(rds_pi, S_IRUGO, bcm2048_rds_pi_read, NULL),
-   __ATTR(rds_rt, S_IRUGO, bcm2048_rds_rt_read, NULL),
-   __ATTR(rds_ps, S_IRUGO, bcm2048_rds_ps_read, NULL),
-   __ATTR(fm_rds_flags, S_IRUGO, bcm2048_fm_rds_flags_read, NULL),
-   __ATTR(region_bottom_frequency, S_IRUGO,
+   __ATTR(rds_pi, 0444, bcm2048_rds_pi_read, NULL),
+   __ATTR(rds_rt, 0444, bcm2048_rds_rt_read, NULL),
+   __ATTR(rds_ps, 0444, bcm2048_rds_ps_read, NULL),
+   __ATTR(fm_rds_flags, 0444, bcm2048_fm_rds_flags_read, NULL),
+   __ATTR(region_bottom_frequency, 0444,
   bcm2048_region_bottom_frequency_read, NULL),
-   __ATTR(region_top_frequency, S_IRUGO,
+   __ATTR(region_top_frequency, 0444,
   bcm2048_region_top_frequency_read, NULL),
-   __ATTR(fm_carrier_error, S_IRUGO,
+   __ATTR(fm_carrier_error, 0444,
   

[PATCH 2/3] Staging: media: radio-bcm2048: Fix alignment issues

[Jean-Baptiste Abbadie]

Signed-off-by: Jean-Baptiste Abbadie 
---
 drivers/staging/media/bcm2048/radio-bcm2048.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index 188d045d44ad..f66bea631e8e 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -997,7 +997,7 @@ static int bcm2048_set_fm_search_tune_mode(struct 
bcm2048_device *bdev,
timeout = BCM2048_AUTO_SEARCH_TIMEOUT;
 
if (!wait_for_completion_timeout(>compl,
-   msecs_to_jiffies(timeout)))
+msecs_to_jiffies(timeout)))
dev_err(>client->dev, "IRQ timeout.\n");
 
if (value)
@@ -2202,7 +2202,7 @@ static ssize_t bcm2048_fops_read(struct file *file, char 
__user *buf,
}
/* interruptible_sleep_on(>read_queue); */
if (wait_event_interruptible(bdev->read_queue,
-   bdev->rds_data_available) < 0) {
+bdev->rds_data_available) < 0) {
retval = -EINTR;
goto done;
}
-- 
2.10.0

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

Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags

[Michal Hocko]

On Wed 19-10-16 09:49:43, Dave Hansen wrote:
> On 10/19/2016 02:07 AM, Michal Hocko wrote:
> > On Wed 19-10-16 09:58:15, Lorenzo Stoakes wrote:
> >> On Tue, Oct 18, 2016 at 05:30:50PM +0200, Michal Hocko wrote:
> >>> I am wondering whether we can go further. E.g. it is not really clear to
> >>> me whether we need an explicit FOLL_REMOTE when we can in fact check
> >>> mm != current->mm and imply that. Maybe there are some contexts which
> >>> wouldn't work, I haven't checked.
> >>
> >> This flag is set even when /proc/self/mem is used. I've not looked deeply 
> >> into
> >> this flag but perhaps accessing your own memory this way can be considered
> >> 'remote' since you're not accessing it directly. On the other hand, 
> >> perhaps this
> >> is just mistaken in this case?
> > 
> > My understanding of the flag is quite limited as well. All I know it is
> > related to protection keys and it is needed to bypass protection check.
> > See arch_vma_access_permitted. See also 1b2ee1266ea6 ("mm/core: Do not
> > enforce PKEY permissions on remote mm access").
> 
> Yeah, we need the flag to tell us when PKEYs should be applied or not.
> The current task's PKRU (pkey rights register) should really only be
> used to impact access to the task's memory, but has no bearing on how a
> given task should access remote memory.

The question I had earlier was whether this has to be an explicit FOLL
flag used by g-u-p users or we can just use it internally when mm !=
current->mm

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

Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags

[Dave Hansen]

On 10/19/2016 02:07 AM, Michal Hocko wrote:
> On Wed 19-10-16 09:58:15, Lorenzo Stoakes wrote:
>> On Tue, Oct 18, 2016 at 05:30:50PM +0200, Michal Hocko wrote:
>>> I am wondering whether we can go further. E.g. it is not really clear to
>>> me whether we need an explicit FOLL_REMOTE when we can in fact check
>>> mm != current->mm and imply that. Maybe there are some contexts which
>>> wouldn't work, I haven't checked.
>>
>> This flag is set even when /proc/self/mem is used. I've not looked deeply 
>> into
>> this flag but perhaps accessing your own memory this way can be considered
>> 'remote' since you're not accessing it directly. On the other hand, perhaps 
>> this
>> is just mistaken in this case?
> 
> My understanding of the flag is quite limited as well. All I know it is
> related to protection keys and it is needed to bypass protection check.
> See arch_vma_access_permitted. See also 1b2ee1266ea6 ("mm/core: Do not
> enforce PKEY permissions on remote mm access").

Yeah, we need the flag to tell us when PKEYs should be applied or not.
The current task's PKRU (pkey rights register) should really only be
used to impact access to the task's memory, but has no bearing on how a
given task should access remote memory.

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

[PATCH 2/2] [media] vb2: Add support for capture_dma_bidirectional queue flag

[Thierry Escande]

From: Pawel Osciak 

When this flag is set for CAPTURE queues by the driver on calling
vb2_queue_init(), it forces the buffers on the queue to be
allocated/mapped with DMA_BIDIRECTIONAL direction flag, instead of
DMA_FROM_DEVICE. This allows the device not only to write to the
buffers, but also read out from them. This may be useful e.g. for codec
hardware, which may be using CAPTURE buffers as reference to decode
other buffers.

This flag is ignored for OUTPUT queues, as we don't want to allow HW to
be able to write to OUTPUT buffers.

Signed-off-by: Pawel Osciak 
Tested-by: Pawel Osciak 
Reviewed-by: Tomasz Figa 
Signed-off-by: Thierry Escande 
---
 drivers/media/v4l2-core/videobuf2-v4l2.c |  3 +--
 include/media/videobuf2-core.h   | 14 ++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c 
b/drivers/media/v4l2-core/videobuf2-v4l2.c
index fde1e2d..c92197c 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -659,8 +659,7 @@ int vb2_queue_init(struct vb2_queue *q)
 * queues will always initialize waiting_for_buffers to false.
 */
q->quirk_poll_must_check_waiting_for_buffers = true;
-   q->dma_dir = V4L2_TYPE_IS_OUTPUT(q->type)
-  ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+   q->dma_dir = VB2_DMA_DIR(q);
 
return vb2_core_queue_init(q);
 }
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 38410dd..cd55917 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -433,6 +433,9 @@ struct vb2_buf_ops {
  * @quirk_poll_must_check_waiting_for_buffers: Return POLLERR at poll when QBUF
  *  has not been called. This is a vb1 idiom that has been adopted
  *  also by vb2.
+ * @capture_dma_bidirectional: use DMA_BIDIRECTIONAL for CAPTURE buffers; this
+ * allows HW to read from the CAPTURE buffers in
+ * addition to writing; ignored for OUTPUT queues.
  * @lock:  pointer to a mutex that protects the vb2_queue struct. The
  * driver can set this to a mutex to let the v4l2 core serialize
  * the queuing ioctls. If the driver wants to handle locking
@@ -500,6 +503,7 @@ struct vb2_queue {
unsignedfileio_write_immediately:1;
unsignedallow_zero_bytesused:1;
unsigned   quirk_poll_must_check_waiting_for_buffers:1;
+   unsignedcapture_dma_bidirectional:1;
 
struct mutex*lock;
void*owner;
@@ -556,6 +560,16 @@ struct vb2_queue {
 #endif
 };
 
+/*
+ * Return the corresponding DMA direction given the vb2_queue type (capture or
+ * output). returns DMA_BIRECTIONAL for capture buffers if set by the driver.
+ */
+#define VB2_DMA_DIR(q) (V4L2_TYPE_IS_OUTPUT((q)->type)   \
+   ? DMA_TO_DEVICE  \
+   : (q)->capture_dma_bidirectional \
+ ? DMA_BIDIRECTIONAL\
+ : DMA_FROM_DEVICE)
+
 /**
  * vb2_plane_vaddr() - Return a kernel virtual address of a given plane
  * @vb:vb2_buffer to which the plane in question belongs to
-- 
2.7.4

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

[PATCH 1/2] [media] vb2: Store dma_dir in vb2_queue

[Thierry Escande]

From: Pawel Osciak 

Store dma_dir in struct vb2_queue and reuse it, instead of recalculating
it each time.

Signed-off-by: Pawel Osciak 
Tested-by: Pawel Osciak 
Reviewed-by: Tomasz Figa 
Reviewed-by: Owen Lin 
Signed-off-by: Thierry Escande 
---
 drivers/media/v4l2-core/videobuf2-core.c | 12 +++-
 drivers/media/v4l2-core/videobuf2-v4l2.c |  2 ++
 include/media/videobuf2-core.h   |  2 ++
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 21900202..f12103c 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -194,8 +194,6 @@ static void __enqueue_in_driver(struct vb2_buffer *vb);
 static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb->vb2_queue;
-   enum dma_data_direction dma_dir =
-   q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
void *mem_priv;
int plane;
int ret = -ENOMEM;
@@ -209,7 +207,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 
mem_priv = call_ptr_memop(vb, alloc,
q->alloc_devs[plane] ? : q->dev,
-   q->dma_attrs, size, dma_dir, q->gfp_flags);
+   q->dma_attrs, size, q->dma_dir, q->gfp_flags);
if (IS_ERR(mem_priv)) {
if (mem_priv)
ret = PTR_ERR(mem_priv);
@@ -978,8 +976,6 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const void 
*pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
-   enum dma_data_direction dma_dir =
-   q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
bool reacquired = vb->planes[0].mem_priv == NULL;
 
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1030,7 +1026,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const 
void *pb)
mem_priv = call_ptr_memop(vb, get_userptr,
q->alloc_devs[plane] ? : q->dev,
planes[plane].m.userptr,
-   planes[plane].length, dma_dir);
+   planes[plane].length, q->dma_dir);
if (IS_ERR(mem_priv)) {
dprintk(1, "failed acquiring userspace "
"memory for plane %d\n", plane);
@@ -1096,8 +1092,6 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const 
void *pb)
void *mem_priv;
unsigned int plane;
int ret = 0;
-   enum dma_data_direction dma_dir =
-   q->is_output ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
bool reacquired = vb->planes[0].mem_priv == NULL;
 
memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1156,7 +1150,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const 
void *pb)
/* Acquire each plane's memory */
mem_priv = call_ptr_memop(vb, attach_dmabuf,
q->alloc_devs[plane] ? : q->dev,
-   dbuf, planes[plane].length, dma_dir);
+   dbuf, planes[plane].length, q->dma_dir);
if (IS_ERR(mem_priv)) {
dprintk(1, "failed to attach dmabuf\n");
ret = PTR_ERR(mem_priv);
diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c 
b/drivers/media/v4l2-core/videobuf2-v4l2.c
index 52ef883..fde1e2d 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -659,6 +659,8 @@ int vb2_queue_init(struct vb2_queue *q)
 * queues will always initialize waiting_for_buffers to false.
 */
q->quirk_poll_must_check_waiting_for_buffers = true;
+   q->dma_dir = V4L2_TYPE_IS_OUTPUT(q->type)
+  ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 
return vb2_core_queue_init(q);
 }
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index ac5898a..38410dd 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -489,6 +489,7 @@ struct vb2_buf_ops {
  * when a buffer with the V4L2_BUF_FLAG_LAST is dequeued.
  * @fileio:file io emulator internal data, used only if emulator is active
  * @threadio:  thread io internal data, used only if thread is active
+ * @dma_dir:   DMA direction to use for buffers on this queue
  */
 struct vb2_queue {
unsigned inttype;
@@ -540,6 +541,7 @@ struct vb2_queue {
 
struct vb2_fileio_data  *fileio;
struct vb2_threadio_data*threadio;
+   enum dma_data_direction dma_dir;
 
 #ifdef CONFIG_VIDEO_ADV_DEBUG
/*
-- 
2.7.4

--
To unsubscribe 

Re: [PATCH 05/10] mm: replace get_vaddr_frames() write/force parameters with gup_flags

[Jan Kara]

On Thu 13-10-16 01:20:15, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_vaddr_frames() and
> replaces them with a gup_flags parameter to make the use of FOLL_FORCE 
> explicit
> in callers as use of this flag can result in surprising behaviour (and hence
> bugs) within the mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes 

Looks good. You can add:

Reviewed-by: Jan Kara 

Honza

> ---
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c|  3 ++-
>  drivers/media/platform/omap/omap_vout.c|  2 +-
>  drivers/media/v4l2-core/videobuf2-memops.c |  6 +-
>  include/linux/mm.h |  2 +-
>  mm/frame_vector.c  | 13 ++---
>  5 files changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c 
> b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index aa92dec..fbd13fa 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -488,7 +488,8 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct 
> drm_device *drm_dev,
>   goto err_free;
>   }
>  
> - ret = get_vaddr_frames(start, npages, true, true, g2d_userptr->vec);
> + ret = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
> + g2d_userptr->vec);
>   if (ret != npages) {
>   DRM_ERROR("failed to get user pages from userptr.\n");
>   if (ret < 0)
> diff --git a/drivers/media/platform/omap/omap_vout.c 
> b/drivers/media/platform/omap/omap_vout.c
> index e668dde..a31b95c 100644
> --- a/drivers/media/platform/omap/omap_vout.c
> +++ b/drivers/media/platform/omap/omap_vout.c
> @@ -214,7 +214,7 @@ static int omap_vout_get_userptr(struct videobuf_buffer 
> *vb, u32 virtp,
>   if (!vec)
>   return -ENOMEM;
>  
> - ret = get_vaddr_frames(virtp, 1, true, false, vec);
> + ret = get_vaddr_frames(virtp, 1, FOLL_WRITE, vec);
>   if (ret != 1) {
>   frame_vector_destroy(vec);
>   return -EINVAL;
> diff --git a/drivers/media/v4l2-core/videobuf2-memops.c 
> b/drivers/media/v4l2-core/videobuf2-memops.c
> index 3c3b517..1cd322e 100644
> --- a/drivers/media/v4l2-core/videobuf2-memops.c
> +++ b/drivers/media/v4l2-core/videobuf2-memops.c
> @@ -42,6 +42,10 @@ struct frame_vector *vb2_create_framevec(unsigned long 
> start,
>   unsigned long first, last;
>   unsigned long nr;
>   struct frame_vector *vec;
> + unsigned int flags = FOLL_FORCE;
> +
> + if (write)
> + flags |= FOLL_WRITE;
>  
>   first = start >> PAGE_SHIFT;
>   last = (start + length - 1) >> PAGE_SHIFT;
> @@ -49,7 +53,7 @@ struct frame_vector *vb2_create_framevec(unsigned long 
> start,
>   vec = frame_vector_create(nr);
>   if (!vec)
>   return ERR_PTR(-ENOMEM);
> - ret = get_vaddr_frames(start & PAGE_MASK, nr, write, true, vec);
> + ret = get_vaddr_frames(start & PAGE_MASK, nr, flags, vec);
>   if (ret < 0)
>   goto out_destroy;
>   /* We accept only complete set of PFNs */
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 27ab538..5ff084f6 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1305,7 +1305,7 @@ struct frame_vector {
>  struct frame_vector *frame_vector_create(unsigned int nr_frames);
>  void frame_vector_destroy(struct frame_vector *vec);
>  int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
> -  bool write, bool force, struct frame_vector *vec);
> +  unsigned int gup_flags, struct frame_vector *vec);
>  void put_vaddr_frames(struct frame_vector *vec);
>  int frame_vector_to_pages(struct frame_vector *vec);
>  void frame_vector_to_pfns(struct frame_vector *vec);
> diff --git a/mm/frame_vector.c b/mm/frame_vector.c
> index 81b6749..db77dcb 100644
> --- a/mm/frame_vector.c
> +++ b/mm/frame_vector.c
> @@ -11,10 +11,7 @@
>   * get_vaddr_frames() - map virtual addresses to pfns
>   * @start:   starting user address
>   * @nr_frames:   number of pages / pfns from start to map
> - * @write:   whether pages will be written to by the caller
> - * @force:   whether to force write access even if user mapping is
> - *   readonly. See description of the same argument of
> - get_user_pages().
> + * @gup_flags:   flags modifying lookup behaviour
>   * @vec: structure which receives pages / pfns of the addresses mapped.
>   *   It should have space for at least nr_frames entries.
>   *
> @@ -34,23 +31,17 @@
>   * This function takes care of grabbing mmap_sem as necessary.
>   */
>  int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
> -  bool write, bool force, struct frame_vector *vec)
> +  unsigned int gup_flags, struct frame_vector *vec)
>  {
>   struct mm_struct *mm = current->mm;
>

Re: [PATCH v2 2/2] ARM: dts: koelsch: add HDMI input

[Geert Uytterhoeven]

On Tue, Oct 18, 2016 at 5:01 PM, Ulrich Hecht
 wrote:
> From: Hans Verkuil 
>
> Add support in the dts for the HDMI input. Based on the Lager dts
> patch from Ultich Hecht.

Ulrich ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] [media] winbond-cir: One variable and its check less in wbcir_shutdown() after error detection

[SF Markus Elfring]

 + /* Set CEIR_EN */
 + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01);
 +set_irqmask:
 /*
 * ACPI will set the HW disable bit for SP3 which means that the
 * output signals are left in an undefined state which may cause
 @@ -876,6 +858,14 @@ wbcir_shutdown(struct pnp_dev *device)
 */
 wbcir_set_irqmask(data, WBCIR_IRQ_NONE);
 disable_irq(data->irq);
 + return;
 +clear_bits:
 + /* Clear BUFF_EN, Clear END_EN, Clear MATCH_EN */
 + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x00, 0x07);
 +
 + /* Clear CEIR_EN */
 + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x00, 0x01);
 + goto set_irqmask;
>>>
>>> I'm not convinced that adding a goto which goes backwards is making this
>>> code any more readible, just so that a local variable can be dropped.
>>
>> Thanks for your feedback.
>>
>> Is such a "backward jump" usual and finally required when you would like
>> to move a bit of common error handling code to the end without using extra
>> local variables and a few statements should still be performed after it?
>>
> 
> I'm sorry, I can't parse this.

Can an other update suggestion like "[PATCH 6/6] crypto-caamhash:
Move common error handling code in two functions" explain this technique
a bit better in principle?
https://patchwork.kernel.org/patch/9333861/
https://lkml.kernel.org/r/

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

Re: [PATCH v2 08/21] [media] imx: Add i.MX IPUv3 capture driver

[Jack Mitchell]

Hi Philipp,

On 17/10/16 13:12, Philipp Zabel wrote:

Hi Jack,

Am Montag, den 17.10.2016, 12:32 +0100 schrieb Jack Mitchell:

Hi Philipp,

I'm looking at how I would enable a parallel greyscale camera using this
set of drivers and am a little bit confused. Do you have an example
somewhere of a devicetree with an input node.


In your board device tree it should look somewhat like this:

 {
sensor@48 {
compatible = "aptina,mt9v032m";
/* ... */

port {
cam_out: endpoint {
remote-endpoint = <_in>;
}
};
};
};

/*
 * This is the input port node corresponding to the 'CSI0' pad group,
 * not necessarily the CSI0 port of IPU1 or IPU2. On i.MX6Q it's port@1
 * of the mipi_ipu1_mux, on i.MX6DL it's port@4 of the ipu_csi0_mux,
 * the csi0 label is added in patch 13/21.
 */
 {
#address-cells = <1>;
#size-cells = <0>;

csi_in: endpoint@0 {
bus-width = <8>;
data-shift = <12>;
hsync-active = <1>;
vsync-active = <1>;
pclk-sample = <1>;
remote-endpoint = <_out>;
};
};


 I also have a further note below:

[...]

+   if (raw && priv->smfc) {




Thank you, I think I have something which is kind of right.

(Apologies in advance for the formatting)

diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts 
b/arch/arm/boot/dts/imx6q-sabrelite.dts

index 66d10d8..90e6b92 100644
--- a/arch/arm/boot/dts/imx6q-sabrelite.dts
+++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
@@ -52,3 +52,62 @@
  {
status = "okay";
 };
+
+ {
+   sensor@10 {
+   compatible = "onsemi,ar0135";
+   reg = <0x10>;
+
+   pinctrl-names = "default";
+   pinctrl-0 = <_ar0135>;
+
+   reset-gpio = < 6 GPIO_ACTIVE_HIGH>;
+
+   clocks = < IMX6QDL_CLK_CKO2>;
+   clock-names = "xclk";
+
+   xclk = <2400>;
+
+   port {
+   parallel_camera_output: endpoint {
+   remote-endpoint = 
<_in_from_parallel_camera>;
+   };
+   };
+   };
+};
+
+ {
+   csi_in_from_parallel_camera: endpoint@0 {
+   bus-width = <8>;
+   data-shift = <12>;
+   hsync-active = <1>;
+   vsync-active = <1>;
+   pclk-sample = <1>;
+   remote-endpoint = <_camera_output>;
+   };
+};
+
+ {
+
+imx6q-sabrelite {
+
+   pinctrl_ar0135: ar0135grp {
+   fsl,pins = <
+   MX6QDL_PAD_GPIO_6__GPIO1_IO06   0x8000
+   MX6QDL_PAD_CSI0_DAT12__IPU1_CSI0_DATA12
0x8000
+   MX6QDL_PAD_CSI0_DAT13__IPU1_CSI0_DATA13
0x8000
+   MX6QDL_PAD_CSI0_DAT14__IPU1_CSI0_DATA14
0x8000
+   MX6QDL_PAD_CSI0_DAT15__IPU1_CSI0_DATA15
0x8000
+   MX6QDL_PAD_CSI0_DAT16__IPU1_CSI0_DATA16
0x8000
+   MX6QDL_PAD_CSI0_DAT17__IPU1_CSI0_DATA17
0x8000
+   MX6QDL_PAD_CSI0_DAT18__IPU1_CSI0_DATA18
0x8000
+   MX6QDL_PAD_CSI0_DAT19__IPU1_CSI0_DATA19
0x8000
+   MX6QDL_PAD_CSI0_PIXCLK__IPU1_CSI0_PIXCLK   
0x8000
+   MX6QDL_PAD_CSI0_MCLK__IPU1_CSI0_HSYNC  
0x8000
+   MX6QDL_PAD_CSI0_VSYNC__IPU1_CSI0_VSYNC 
0x8000
+   MX6QDL_PAD_CSI0_DATA_EN__IPU1_CSI0_DATA_EN 
0x8000
+   >;
+   };
+   };
+};


However, I can't seem to link the entities together properly, am I 
missing something obvious?


root@vicon:~# media-ctl -p
Media controller API version 0.1.0

Media device information

driver  imx-media
model   i.MX IPUv3
serial
bus info
hw revision 0x0
driver version  0.0.0

Device topology
- entity 1: IPU0 CSI0 (2 pads, 1 link)
type V4L2 subdev subtype Unknown flags 0
pad0: Sink
pad1: Source
-> "imx-ipuv3-capture.0":0 [ENABLED]

- entity 4: imx-ipuv3-capture.0 (1 pad, 1 link)
type Node subtype V4L flags 0
device node name /dev/video0
pad0: Sink
<- "IPU0 CSI0":1 [ENABLED]

- entity 10: IPU0 CSI1 (2 pads, 1 link)
 type V4L2 subdev subtype Unknown flags 0
pad0: Sink
pad1: Source
-> "imx-ipuv3-capture.1":0 [ENABLED]

- entity 13: imx-ipuv3-capture.1 (1 pad, 1 link)
 type Node subtype V4L flags 0
 device node name /dev/video1
pad0: Sink
<- "IPU0 CSI1":1 [ENABLED]

- 

Re: [media] winbond-cir: Move assignments for three variables in wbcir_shutdown()

[SF Markus Elfring]

>> Move the setting for the local variables "mask", "match" and "rc6_csl"
>> behind the source code for a condition check by this function
>> at the beginning.
>  
> Again, I can't see what the point is?

* How do you think about to set these variables only after the initial
  check succeded?

* Do you care for data access locality?

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

[FYI] Very limited time for code review this week and next week

[Hans Verkuil]

Hi all,

Real-life (tm) is interfering with media development this week and next 
week.

So I doubt I'll have much opportunity to do code reviews during that period.

Just so people are aware of this.

Regards,

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

Re: [media] winbond-cir: Move a variable assignment in wbcir_tx()

[SF Markus Elfring]

>> * How do you think about to avoid a variable assignment in case
>> that this memory allocation failed anyhow?
> 
> There is no memory allocation that can fail at this point.

Do you really know the failure probability for a call of the
function "kmalloc" (within the function "wbcir_tx") under all
possible run time situations?


>> * Do you care for data access locality?
> 
> Not unless you can show measurable performance improvements?

Did any software developer (before me) dare anything in this direction?

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

Re: [media] winbond-cir: Move a variable assignment in two functions

[SF Markus Elfring]

>> Move the assignment for the local variable "data" behind the source code
>> for condition checks by these functions.
> 
> Why?

* Would you like to set these variables only after the initial
  check succeeded?

* Do you care for data access locality also in these cases?

Regards,
Markus

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

Re: [media] winbond-cir: Move a variable assignment in wbcir_tx()

[SF Markus Elfring]

>> Move the assignment for the local variable "data" behind the source code
>> for a memory allocation by this function.
> 
> Sorry, I can't see what the point is?

* How do you think about to avoid a variable assignment in case
  that this memory allocation failed anyhow?

* Do you care for data access locality?

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

[PATCH 0/2] [media] DMA direction support in vb2_queue

[Thierry Escande]

Hi,

This series adds a dma_dir field to the vb2_queue structure in order to
store the DMA direction once for all in vb2_queue_init();

It also adds a new capture_dma_bidirectional flag to the vb2_queue
structure allowing the hardware to read from the CAPTURE buffer. This
flag is ignored for OUTPUT queues. This is used on ChromeOS by the
rockchip-vpu driver.

Changes since v1:
- Renamed use_dma_bidirectional field as capture_dma_bidirectional
- Added a VB2_DMA_DIR() macro

Pawel Osciak (2):
  [media] vb2: Store dma_dir in vb2_queue
  [media] vb2: Add support for capture_dma_bidirectional queue flag

 drivers/media/v4l2-core/videobuf2-core.c | 12 +++-
 drivers/media/v4l2-core/videobuf2-v4l2.c |  6 ++
 include/media/videobuf2-core.h   |  6 ++
 3 files changed, 15 insertions(+), 9 deletions(-)

-- 
2.7.4

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

Re: [media] winbond-cir: Move assignments for three variables in wbcir_shutdown()

[SF Markus Elfring]

 Move the setting for the local variables "mask", "match" and "rc6_csl"
 behind the source code for a condition check by this function
 at the beginning.
>>>
>>> Again, I can't see what the point is?
>>
>> * How do you think about to set these variables only after the initial
>> check succeded?
> 
> I prefer setting variables early so that no thinking about
> whether they're initialized or not is necessary later.

* How do you think about to reduce the scope for these variables then?

* Would you dare to move the corresponding source code into a separate function?

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

Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags

[Michal Hocko]

On Wed 19-10-16 09:58:15, Lorenzo Stoakes wrote:
> On Tue, Oct 18, 2016 at 05:30:50PM +0200, Michal Hocko wrote:
> > I am wondering whether we can go further. E.g. it is not really clear to
> > me whether we need an explicit FOLL_REMOTE when we can in fact check
> > mm != current->mm and imply that. Maybe there are some contexts which
> > wouldn't work, I haven't checked.
> 
> This flag is set even when /proc/self/mem is used. I've not looked deeply into
> this flag but perhaps accessing your own memory this way can be considered
> 'remote' since you're not accessing it directly. On the other hand, perhaps 
> this
> is just mistaken in this case?

My understanding of the flag is quite limited as well. All I know it is
related to protection keys and it is needed to bypass protection check.
See arch_vma_access_permitted. See also 1b2ee1266ea6 ("mm/core: Do not
enforce PKEY permissions on remote mm access").

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

Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Michal Hocko]

On Wed 19-10-16 09:59:03, Jan Kara wrote:
> On Thu 13-10-16 01:20:18, Lorenzo Stoakes wrote:
> > This patch removes the write parameter from __access_remote_vm() and 
> > replaces it
> > with a gup_flags parameter as use of this function previously _implied_
> > FOLL_FORCE, whereas after this patch callers explicitly pass this flag.
> > 
> > We make this explicit as use of FOLL_FORCE can result in surprising 
> > behaviour
> > (and hence bugs) within the mm subsystem.
> > 
> > Signed-off-by: Lorenzo Stoakes 
> 
> So I'm not convinced this (and the following two patches) is actually
> helping much. By grepping for FOLL_FORCE we will easily see that any caller
> of access_remote_vm() gets that semantics and can thus continue search

I am really wondering. Is there anything inherent that would require
FOLL_FORCE for access_remote_vm? I mean FOLL_FORCE is a really
non-trivial thing. It doesn't obey vma permissions so we should really
minimize its usage. Do all of those users really need FOLL_FORCE?

Anyway I would rather see the flag explicit and used at more places than
hidden behind a helper function.
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Michal Hocko]

On Wed 19-10-16 10:06:46, Lorenzo Stoakes wrote:
> On Wed, Oct 19, 2016 at 10:52:05AM +0200, Michal Hocko wrote:
> > yes this is the desirable and expected behavior.
> >
> > > wonder if this is desirable behaviour or whether this ought to be limited 
> > > to
> > > ptrace system calls. Regardless, by making the flag more visible it makes 
> > > it
> > > easier to see that this is happening.
> >
> > mem_open already enforces PTRACE_MODE_ATTACH
> 
> Ah I missed this, that makes a lot of sense, thanks!
> 
> I still wonder whether other invocations of access_remote_vm() in 
> fs/proc/base.c
> (the principle caller of this function) need FOLL_FORCE, for example the 
> various
> calls that simply read data from other processes, so I think the point stands
> about keeping this explicit.

I do agree. Making them explicit will help to clean them up later,
should there be a need.

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

Re: [PATCH v2 08/58] cx23885: don't break long lines

[Hans Verkuil]

On 10/18/2016 10:45 PM, Mauro Carvalho Chehab wrote:
> Due to the 80-cols restrictions, and latter due to checkpatch
> warnings, several strings were broken into multiple lines. This
> is not considered a good practice anymore, as it makes harder
> to grep for strings at the source code.
> 
> As we're right now fixing other drivers due to KERN_CONT, we need
> to be able to identify what printk strings don't end with a "\n".
> It is a way easier to detect those if we don't break long lines.
> 
> So, join those continuation lines.
> 
> The patch was generated via the script below, and manually
> adjusted if needed.
> 
> 
> use Text::Tabs;
> while (<>) {
>   if ($next ne "") {
>   $c=$_;
>   if ($c =~ /^\s+\"(.*)/) {
>   $c2=$1;
>   $next =~ s/\"\n$//;
>   $n = expand($next);
>   $funpos = index($n, '(');
>   $pos = index($c2, '",');
>   if ($funpos && $pos > 0) {
>   $s1 = substr $c2, 0, $pos + 2;
>   $s2 = ' ' x ($funpos + 1) . substr $c2, $pos + 
> 2;
>   $s2 =~ s/^\s+//;
> 
>   $s2 = ' ' x ($funpos + 1) . $s2 if ($s2 ne "");
> 
>   print unexpand("$next$s1\n");
>   print unexpand("$s2\n") if ($s2 ne "");
>   } else {
>   print "$next$c2\n";
>   }
>   $next="";
>   next;
>   } else {
>   print $next;
>   }
>   $next="";
>   } else {
>   if (m/\"$/) {
>   if (!m/\\n\"$/) {
>   $next=$_;
>   next;
>   }
>   }
>   }
>   print $_;
> }
> 
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/pci/cx23885/cimax2.c|  5 ++---
>  drivers/media/pci/cx23885/cx23885-417.c   | 17 +++--
>  drivers/media/pci/cx23885/cx23885-alsa.c  | 15 +++
>  drivers/media/pci/cx23885/cx23885-cards.c |  8 +++-
>  drivers/media/pci/cx23885/cx23885-core.c  | 15 +++
>  drivers/media/pci/cx23885/cx23885-dvb.c   |  3 +--
>  drivers/media/pci/cx23885/cx23885-video.c |  3 +--
>  drivers/media/pci/cx23885/cx23888-ir.c|  7 +++
>  8 files changed, 31 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/media/pci/cx23885/cimax2.c 
> b/drivers/media/pci/cx23885/cimax2.c
> index 631e4f24aea6..43febef2f649 100644
> --- a/drivers/media/pci/cx23885/cimax2.c
> +++ b/drivers/media/pci/cx23885/cimax2.c
> @@ -365,9 +365,8 @@ static void netup_read_ci_status(struct work_struct *work)
>   if (ret != 0)
>   return;
>  
> - ci_dbg_print("%s: Slot Status Addr=[0x%04x], "
> - "Reg=[0x%02x], data=%02x, "
> - "TS config = %02x\n", __func__,
> + ci_dbg_print("%s: Slot Status Addr=[0x%04x], Reg=[0x%02x], 
> data=%02x, TS config = %02x\n",
> +  __func__,
>   state->ci_i2c_addr, 0, buf[0],
>   buf[0]);

The last three lines can probably be concatenated to one line.

>  
> diff --git a/drivers/media/pci/cx23885/cx23885-417.c 
> b/drivers/media/pci/cx23885/cx23885-417.c
> index da892f3e3c29..0444db081b5c 100644
> --- a/drivers/media/pci/cx23885/cx23885-417.c
> +++ b/drivers/media/pci/cx23885/cx23885-417.c
> @@ -770,8 +770,8 @@ static int cx23885_mbox_func(void *priv,
>   mc417_memory_read(dev, dev->cx23417_mailbox - 4, );
>   if (value != 0x12345678) {
>   printk(KERN_ERR
> - "Firmware and/or mailbox pointer not initialized "
> - "or corrupted, signature = 0x%x, cmd = %s\n", value,
> + "Firmware and/or mailbox pointer not initialized or 
> corrupted, signature = 0x%x, cmd = %s\n",
> +value,

Weird indentation (or lack thereof).

Other than these two comment this looks good.

Hans

>   cmd_to_str(command));
>   return -1;
>   }
> @@ -781,8 +781,8 @@ static int cx23885_mbox_func(void *priv,
>*/
>   mc417_memory_read(dev, dev->cx23417_mailbox, );
>   if (flag) {
> - printk(KERN_ERR "ERROR: Mailbox appears to be in use "
> - "(%x), cmd = %s\n", flag, cmd_to_str(command));
> + printk(KERN_ERR "ERROR: Mailbox appears to be in use (%x), cmd 
> = %s\n",
> +flag, cmd_to_str(command));
>   return -1;
>   }
>  
> @@ -935,14 +935,12 @@ static int cx23885_load_firmware(struct cx23885_dev 
> *dev)
>   printk(KERN_ERR
>   "ERROR: Hotplug firmware 

Re: [PATCH 00/10] mm: adjust get_user_pages* functions to explicitly pass FOLL_* flags

[Lorenzo Stoakes]

On Tue, Oct 18, 2016 at 05:30:50PM +0200, Michal Hocko wrote:
> I am wondering whether we can go further. E.g. it is not really clear to
> me whether we need an explicit FOLL_REMOTE when we can in fact check
> mm != current->mm and imply that. Maybe there are some contexts which
> wouldn't work, I haven't checked.

This flag is set even when /proc/self/mem is used. I've not looked deeply into
this flag but perhaps accessing your own memory this way can be considered
'remote' since you're not accessing it directly. On the other hand, perhaps this
is just mistaken in this case?

> I guess there is more work in that area and I do not want to impose all
> that work on you, but I couldn't resist once I saw you playing in that
> area ;) Definitely a good start!

Thanks, I am more than happy to go as far down this rabbit hole as is helpful,
no imposition at all :)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [media] winbond-cir: Move assignments for three variables in wbcir_shutdown()

[David Härdeman]

October 19, 2016 3:38 PM, "SF Markus Elfring"  
wrote:
>>> Move the setting for the local variables "mask", "match" and "rc6_csl"
>>> behind the source code for a condition check by this function
>>> at the beginning.
>> 
>> Again, I can't see what the point is?
> 
> * How do you think about to set these variables only after the initial
> check succeded?

I prefer setting variables early so that no thinking about whether they're 
initialized or not is necessary later. 

> * Do you care for data access locality?

Not unless you can show measurable performance improvements?
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] [media] winbond-cir: Move a variable assignment in wbcir_tx()

[David Härdeman]

October 14, 2016 1:42 PM, "SF Markus Elfring"  
wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 07:34:46 +0200
> 
> Move the assignment for the local variable "data" behind the source code
> for a memory allocation by this function.

Sorry, I can't see what the point is?


> Signed-off-by: Markus Elfring 
> ---
> drivers/media/rc/winbond-cir.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index 59050f5..fd997f0 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -655,7 +655,7 @@ wbcir_txmask(struct rc_dev *dev, u32 mask)
> static int
> wbcir_tx(struct rc_dev *dev, unsigned *b, unsigned count)
> {
> - struct wbcir_data *data = dev->priv;
> + struct wbcir_data *data;
> unsigned *buf;
> unsigned i;
> unsigned long flags;
> @@ -668,6 +668,7 @@ wbcir_tx(struct rc_dev *dev, unsigned *b, unsigned count)
> for (i = 0; i < count; i++)
> buf[i] = DIV_ROUND_CLOSEST(b[i], 10);
> 
> + data = dev->priv;
> /* Not sure if this is possible, but better safe than sorry */
> spin_lock_irqsave(>spinlock, flags);
> if (data->txstate != WBCIR_TXSTATE_INACTIVE) {
> -- 
> 2.10.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: em28xx WinTV dualHD in Raspbian

[Mauro Carvalho Chehab]

Em Tue, 18 Oct 2016 23:55:04 +0200
 escreveu:

> Hi there,
> 
> I am running an updated raspbian image with kernel 4.4.23-v7+, matching
> linux-headers-4.4.23-v7+_4.4.23-v7+-2_armhf from here:
> https://www.niksula.hut.fi/~mhiienka/Rpi/linux-headers-rpi/ and
> dvb-demod-si2168-b40-01.fw (see linuxtv-wiki).
> 
> Before the last apt-get upgrade this works great, but now, it ends up in
> recognizing the device but not create /dev/dvb.
> 
> Log: 
> Oct 18 23:07:59 mediapi kernel: [ 7587.975803] usb 1-1.3: new high-speed USB
> device number 6 using dwc_otg
> Oct 18 23:07:59 mediapi kernel: [ 7588.076796] usb 1-1.3: New USB device
> found, idVendor=2040, idProduct=0265
> Oct 18 23:07:59 mediapi kernel: [ 7588.076817] usb 1-1.3: New USB device
> strings: Mfr=3, Product=1, SerialNumber=2
> Oct 18 23:07:59 mediapi kernel: [ 7588.076842] usb 1-1.3: Product: dualHD
> Oct 18 23:07:59 mediapi kernel: [ 7588.076853] usb 1-1.3: Manufacturer: HCW
> Oct 18 23:07:59 mediapi kernel: [ 7588.076864] usb 1-1.3: SerialNumber:
> 0011540068
> Oct 18 23:08:00 mediapi kernel: [ 7589.111483] media: Linux media interface:
> v0.10
> Oct 18 23:08:00 mediapi kernel: [ 7589.121622] Linux video capture
> interface: v2.00
> Oct 18 23:08:00 mediapi kernel: [ 7589.126137] em28xx: New device HCW dualHD
> @ 480 Mbps (2040:0265, interface 0, class 0)
> Oct 18 23:08:00 mediapi kernel: [ 7589.126157] em28xx: DVB interface 0
> found: isoc
> Oct 18 23:08:00 mediapi kernel: [ 7589.127012] em28xx: chip ID is em28174
> Oct 18 23:08:01 mediapi kernel: [ 7590.338459] em28174 #0: EEPROM ID = 26 00
> 01 00, EEPROM hash = 0x7ee3cbc8
> Oct 18 23:08:01 mediapi kernel: [ 7590.338482] em28174 #0: EEPROM info:
> Oct 18 23:08:01 mediapi kernel: [ 7590.338496] em28174 #0:     microcode
> start address = 0x0004, boot configuration = 0x01
> Oct 18 23:08:01 mediapi kernel: [ 7590.346046] em28174 #0:     AC97 audio (5
> sample rates)
> Oct 18 23:08:01 mediapi kernel: [ 7590.346064] em28174 #0:     500mA max
> power
> Oct 18 23:08:01 mediapi kernel: [ 7590.346079] em28174 #0:     Table at
> offset 0x27, strings=0x0e6a, 0x1888, 0x087e
> Oct 18 23:08:01 mediapi kernel: [ 7590.347683] em28174 #0: Identified as
> Hauppauge WinTV-dualHD DVB (card=99)
> Oct 18 23:08:01 mediapi kernel: [ 7590.355143] tveeprom 4-0050: Hauppauge
> model 204109, rev B2I6, serial# 11540068
> Oct 18 23:08:01 mediapi kernel: [ 7590.355170] tveeprom 4-0050: tuner model
> is SiLabs Si2157 (idx 186, type 4)
> Oct 18 23:08:01 mediapi kernel: [ 7590.355188] tveeprom 4-0050: TV standards
> PAL(B/G) NTSC(M) PAL(I) SECAM(L/L') PAL(D/D1/K) ATSC/DVB Digital (eeprom
> 0xfc)
> Oct 18 23:08:01 mediapi kernel: [ 7590.355204] tveeprom 4-0050: audio
> processor is None (idx 0)
> Oct 18 23:08:01 mediapi kernel: [ 7590.355219] tveeprom 4-0050: has no
> radio, has IR receiver, has no IR transmitter
> Oct 18 23:08:01 mediapi kernel: [ 7590.355233] em28174 #0: dvb set to isoc
> mode.
> Oct 18 23:08:01 mediapi rsyslogd-2007: action 'action 17' suspended, next
> retry is Tue Oct 18 23:08:31 2016 [try http://www.rsyslog.com/e/2007 ]
> Oct 18 23:08:01 mediapi kernel: [ 7590.357918] usbcore: registered new
> interface driver em28xx
> Oct 18 23:08:01 mediapi kernel: [ 7590.369200] em28xx_dvb: disagrees about
> version of symbol dvb_dmxdev_init
> Oct 18 23:08:01 mediapi kernel: [ 7590.369228] em28xx_dvb: Unknown symbol
> dvb_dmxdev_init (err -22)
> […] (multiple „disagrees“ and „unknown symbol“)
> ct 18 23:08:01 mediapi kernel: [ 7590.417607] em28174 #0: Registering input
> extension
> Oct 18 23:08:01 mediapi kernel: [ 7590.455841] Registered IR keymap
> rc-hauppauge
> Oct 18 23:08:01 mediapi kernel: [ 7590.456798] input: em28xx IR (em28174 #0)
> as /devices/platform/soc/3f98.usb/usb1/1-1/1-1.3/rc/rc0/input0
> Oct 18 23:08:01 mediapi kernel: [ 7590.457059] rc rc0: em28xx IR (em28174
> #0) as /devices/platform/soc/3f98.usb/usb1/1-1/1-1.3/rc/rc0
> Oct 18 23:08:01 mediapi kernel: [ 7590.457715] em28174 #0: Input extension
> successfully initalized
> Oct 18 23:08:01 mediapi kernel: [ 7590.457734] em28xx: Registered (Em28xx
> Input Extension) extension
> 
> 
> Before it stopped working, I have executed the apt upgrade, installed the
> new kernel header and run 
> git clone git://linuxtv.org/media_build.git
> cd media_build 
> ./build
> sudo make install
> 
> No errors appeared.
> 
> What I am doing wrong? The last git clone was approx. in August.

Based on this log:

Oct 18 23:08:01 mediapi kernel: [ 7590.369200] em28xx_dvb: disagrees about 
version of symbol dvb_dmxdev_init
Oct 18 23:08:01 mediapi kernel: [ 7590.369228] em28xx_dvb: Unknown symbol 
dvb_dmxdev_init (err -22)

it seems you messed the modules install or you have the V4L2 stack
compiled builtin with a different version. 


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

Re: [PATCH 1/5] [media] winbond-cir: Use kmalloc_array() in wbcir_tx()

[David Härdeman]

October 14, 2016 1:41 PM, "SF Markus Elfring"  
wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 07:19:00 +0200
> 
> A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "kmalloc_array".
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring 

Sure...why not...

Signed-off-by: David Härdeman 


> ---
> drivers/media/rc/winbond-cir.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index 95ae60e..59050f5 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -660,7 +660,7 @@ wbcir_tx(struct rc_dev *dev, unsigned *b, unsigned count)
> unsigned i;
> unsigned long flags;
> 
> - buf = kmalloc(count * sizeof(*b), GFP_KERNEL);
> + buf = kmalloc_array(count, sizeof(*b), GFP_KERNEL);
> if (!buf)
> return -ENOMEM;
> 
> -- 
> 2.10.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/2] media: i2c/ov5645: add the device tree binding document

[Laurent Pinchart]

Hi Todor,

On Wednesday 19 Oct 2016 12:14:55 Todor Tomov wrote:
> On 10/19/2016 11:49 AM, Laurent Pinchart wrote:
> > On Friday 14 Oct 2016 15:01:08 Todor Tomov wrote:
> >> On 09/08/2016 03:22 PM, Laurent Pinchart wrote:
> >>> On Thursday 08 Sep 2016 12:13:54 Todor Tomov wrote:
>  Add the document for ov5645 device tree binding.
>  
>  Signed-off-by: Todor Tomov 
>  ---
>  
>   .../devicetree/bindings/media/i2c/ov5645.txt   | 52 ++
>   1 file changed, 52 insertions(+)
>   create mode 100644
>   Documentation/devicetree/bindings/media/i2c/ov5645.txt
>  
>  diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
>  b/Documentation/devicetree/bindings/media/i2c/ov5645.txt new file mode
>  100644
>  index 000..bcf6dba
>  --- /dev/null
>  +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
>  @@ -0,0 +1,52 @@
>  +* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
>  +
>  +The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image
>  sensor with
>  +an active array size of 2592H x 1944V. It is programmable through a
>  serial I2C
>  +interface.
>  +
>  +Required Properties:
>  +- compatible: Value should be "ovti,ov5645".
>  +- clocks: Reference to the xclk clock.
>  +- clock-names: Should be "xclk".
>  +- clock-frequency: Frequency of the xclk clock.
>  +- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH.
> > 
> > By the way, isn't the pin called pwdnb and isn't it active low ?
> 
> Yes, the pin is called "pwdnb" and is active low (must be up for power to be
> up). I have changed the name to "enable" as it is more generally used -
> this change was suggested by Rob Herring. As the logic switches with this
> change of the name I have stated it is active high which ends up in the
> same condition (enable must be up for the power to be up). I think this is
> correct, isn't it?

I thought that the rule was to name the GPIO properties based on the name of 
the pin. I could be wrong though. Rob, what's your opinion ?

>  +- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW.
> >>> 
> >>> Shouldn't the enable and reset GPIOs be optional ?
> >> 
> >> I don't think so. The operations on the GPIOs are part of the power up
> >> sequence of the sensor so we must have control over them to execute the
> >> exact sequence.
> > 
> > Right, let's keep them mandatory. If we later have to make them optional
> > for a board that pulls one of those signals up (assuming this can work at
> > all) we'll revisit the bindings.
> 
> Ok.
> 
>  +- vdddo-supply: Chip digital IO regulator.
>  +- vdda-supply: Chip analog regulator.
>  +- vddd-supply: Chip digital core regulator.
>  +
>  +The device node must contain one 'port' child node for its digital
>  output
>  +video port, in accordance with the video interface bindings defined in
>  +Documentation/devicetree/bindings/media/video-interfaces.txt.
>  +
>  +Example:
>  +
>  + {
>  +...
>  +
>  +ov5645: ov5645@78 {
>  +compatible = "ovti,ov5645";
>  +reg = <0x78>;
>  +
>  +enable-gpios = < 6 GPIO_ACTIVE_HIGH>;
>  +reset-gpios = < 20 GPIO_ACTIVE_LOW>;
>  +pinctrl-names = "default";
>  +pinctrl-0 = <_rear_default>;
>  +
>  +clocks = < 200>;
>  +clock-names = "xclk";
>  +clock-frequency = <2388>;
>  +
>  +vdddo-supply = <_dovdd_1v8>;
>  +vdda-supply = <_avdd_2v8>;
>  +vddd-supply = <_dvdd_1v2>;
>  +
>  +port {
>  +ov5645_ep: endpoint {
>  +clock-lanes = <1>;
>  +data-lanes = <0 2>;
>  +remote-endpoint = <_ep>;
>  +};
>  +};
>  +};
>  +};

-- 
Regards,

Laurent Pinchart

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

Re: [PATCH 2/2] [media] vb2: Add support for use_dma_bidirectional queue flag

[Sakari Ailus]

On Tue, Oct 18, 2016 at 06:08:53PM +0200, Thierry Escande wrote:
> >#define vb2_dma_dir(q) \

^

VB2_DMA_DIR(), as most of our other macros use capitals as well.

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

Re: [PATCH 5/5] [media] winbond-cir: Move a variable assignment in two functions

[David Härdeman]

October 14, 2016 1:45 PM, "SF Markus Elfring"  
wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 13:13:11 +0200
> 
> Move the assignment for the local variable "data" behind the source code
> for condition checks by these functions.

Why?

> Signed-off-by: Markus Elfring 
> ---
> drivers/media/rc/winbond-cir.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index 3d286b9..716b1fe 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -566,7 +566,7 @@ wbcir_set_carrier_report(struct rc_dev *dev, int enable)
> static int
> wbcir_txcarrier(struct rc_dev *dev, u32 carrier)
> {
> - struct wbcir_data *data = dev->priv;
> + struct wbcir_data *data;
> unsigned long flags;
> u8 val;
> u32 freq;
> @@ -592,6 +592,7 @@ wbcir_txcarrier(struct rc_dev *dev, u32 carrier)
> break;
> }
> 
> + data = dev->priv;
> spin_lock_irqsave(>spinlock, flags);
> if (data->txstate != WBCIR_TXSTATE_INACTIVE) {
> spin_unlock_irqrestore(>spinlock, flags);
> @@ -611,7 +612,7 @@ wbcir_txcarrier(struct rc_dev *dev, u32 carrier)
> static int
> wbcir_txmask(struct rc_dev *dev, u32 mask)
> {
> - struct wbcir_data *data = dev->priv;
> + struct wbcir_data *data;
> unsigned long flags;
> u8 val;
> 
> @@ -637,6 +638,7 @@ wbcir_txmask(struct rc_dev *dev, u32 mask)
> return -EINVAL;
> }
> 
> + data = dev->priv;
> spin_lock_irqsave(>spinlock, flags);
> if (data->txstate != WBCIR_TXSTATE_INACTIVE) {
> spin_unlock_irqrestore(>spinlock, flags);
> -- 
> 2.10.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags

[Jan Kara]

On Thu 13-10-16 01:20:14, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_user_pages_locked()
> and replaces them with a gup_flags parameter to make the use of FOLL_FORCE
> explicit in callers as use of this flag can result in surprising behaviour 
> (and
> hence bugs) within the mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes 

After our discussion the patch looks good to me. You can add:

Reviewed-by: Jan Kara 

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

Re: [PATCH 4/5] [media] winbond-cir: One variable and its check less in wbcir_shutdown() after error detection

[David Härdeman]

October 15, 2016 3:30 PM, "Sean Young"  wrote:
> On Fri, Oct 14, 2016 at 01:44:02PM +0200, SF Markus Elfring wrote:
> 
>> From: Markus Elfring 
>> Date: Fri, 14 Oct 2016 12:48:41 +0200
>> 
>> The local variable "do_wake" was set to "false" after an invalid system
>> setting was detected so that a bit of error handling was triggered.
>> 
>> * Replace these assignments by direct jumps to the source code with the
>> desired exception handling.
>> 
>> * Delete this status variable and a corresponding check which became
>> unnecessary with this refactoring.
>> 
>> Signed-off-by: Markus Elfring 
>> ---
>> drivers/media/rc/winbond-cir.c | 78 
>> ++
>> 1 file changed, 34 insertions(+), 44 deletions(-)
>> 
>> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
>> index 9d05e17..3d286b9 100644
>> --- a/drivers/media/rc/winbond-cir.c
>> +++ b/drivers/media/rc/winbond-cir.c
>> @@ -699,16 +699,13 @@ wbcir_shutdown(struct pnp_dev *device)
>> {
>> struct device *dev = >dev;
>> struct wbcir_data *data = pnp_get_drvdata(device);
>> - bool do_wake = true;
>> u8 match[11];
>> u8 mask[11];
>> u8 rc6_csl;
>> int i;
>> 
>> - if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev)) {
>> - do_wake = false;
>> - goto finish;
>> - }
>> + if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev))
>> + goto clear_bits;
>> 
>> rc6_csl = 0;
>> memset(match, 0, sizeof(match));
>> @@ -716,9 +713,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> switch (protocol) {
>> case IR_PROTOCOL_RC5:
>> if (wake_sc > 0xFFF) {
>> - do_wake = false;
>> dev_err(dev, "RC5 - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> /* Mask = 13 bits, ex toggle */
>> @@ -735,9 +731,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> 
>> case IR_PROTOCOL_NEC:
>> if (wake_sc > 0xFF) {
>> - do_wake = false;
>> dev_err(dev, "NEC - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> mask[0] = mask[1] = mask[2] = mask[3] = 0xFF;
>> @@ -757,9 +752,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> 
>> if (wake_rc6mode == 0) {
>> if (wake_sc > 0x) {
>> - do_wake = false;
>> dev_err(dev, "RC6 - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> /* Command */
>> @@ -813,9 +807,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> } else if (wake_sc <= 0x007F) {
>> rc6_csl = 60;
>> } else {
>> - do_wake = false;
>> dev_err(dev, "RC6 - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> /* Header */
>> @@ -825,49 +818,38 @@ wbcir_shutdown(struct pnp_dev *device)
>> mask[i++] = 0x0F;
>> 
>> } else {
>> - do_wake = false;
>> dev_err(dev, "RC6 - Invalid wake mode\n");
>> + goto clear_bits;
>> }
>> 
>> break;
>> 
>> default:
>> - do_wake = false;
>> - break;
>> + goto clear_bits;
>> }
>> 
>> -finish:
>> - if (do_wake) {
>> - /* Set compare and compare mask */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> - WBCIR_REGSEL_COMPARE | WBCIR_REG_ADDR0,
>> - 0x3F);
>> - outsb(data->wbase + WBCIR_REG_WCEIR_DATA, match, 11);
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> - WBCIR_REGSEL_MASK | WBCIR_REG_ADDR0,
>> - 0x3F);
>> - outsb(data->wbase + WBCIR_REG_WCEIR_DATA, mask, 11);
>> -
>> - /* RC6 Compare String Len */
>> - outb(rc6_csl, data->wbase + WBCIR_REG_WCEIR_CSL);
>> -
>> - /* Clear status bits NEC_REP, BUFF, MSG_END, MATCH */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_STS, 0x17, 0x17);
>> + /* Set compare and compare mask */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> + WBCIR_REGSEL_COMPARE | WBCIR_REG_ADDR0,
>> + 0x3F);
>> + outsb(data->wbase + WBCIR_REG_WCEIR_DATA, match, 11);
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> + WBCIR_REGSEL_MASK | WBCIR_REG_ADDR0,
>> + 0x3F);
>> + outsb(data->wbase + WBCIR_REG_WCEIR_DATA, mask, 11);
>> 
>> - /* Clear BUFF_EN, Clear END_EN, Set MATCH_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x01, 0x07);
>> + /* RC6 Compare String Len */
>> + outb(rc6_csl, data->wbase + WBCIR_REG_WCEIR_CSL);
>> 
>> - /* Set CEIR_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01);
>> -
>> - } else {
>> - /* Clear BUFF_EN, Clear END_EN, Clear MATCH_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x00, 0x07);
>> + /* Clear status bits NEC_REP, BUFF, MSG_END, MATCH */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_STS, 0x17, 0x17);
>> 
>> - /* Clear CEIR_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x00, 0x01);
>> - }
>> + /* Clear BUFF_EN, Clear END_EN, Set MATCH_EN */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x01, 0x07);
>> 
>> + /* Set CEIR_EN */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01);
>> +set_irqmask:
>> /*
>> * ACPI will set the HW disable bit for SP3 which means that the
>> * output signals are left in an undefined state which may cause
>> @@ -876,6 +858,14 @@ 

Re: [PATCH 4/5] [media] winbond-cir: One variable and its check less in wbcir_shutdown() after error detection

[David Härdeman]

October 15, 2016 6:42 PM, "SF Markus Elfring"  
wrote:
>>> + /* Set CEIR_EN */
>>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01);
>>> +set_irqmask:
>>> /*
>>> * ACPI will set the HW disable bit for SP3 which means that the
>>> * output signals are left in an undefined state which may cause
>>> @@ -876,6 +858,14 @@ wbcir_shutdown(struct pnp_dev *device)
>>> */
>>> wbcir_set_irqmask(data, WBCIR_IRQ_NONE);
>>> disable_irq(data->irq);
>>> + return;
>>> +clear_bits:
>>> + /* Clear BUFF_EN, Clear END_EN, Clear MATCH_EN */
>>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x00, 0x07);
>>> +
>>> + /* Clear CEIR_EN */
>>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x00, 0x01);
>>> + goto set_irqmask;
>> 
>> I'm not convinced that adding a goto which goes backwards is making this
>> code any more readible, just so that a local variable can be dropped.
> 
> Thanks for your feedback.
> 
> Is such a "backward jump" usual and finally required when you would like
> to move a bit of common error handling code to the end without using extra
> local variables and a few statements should still be performed after it?
> 

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

Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Lorenzo Stoakes]

On Wed, Oct 19, 2016 at 10:13:52AM +0200, Michal Hocko wrote:
> On Wed 19-10-16 09:59:03, Jan Kara wrote:
> > On Thu 13-10-16 01:20:18, Lorenzo Stoakes wrote:
> > > This patch removes the write parameter from __access_remote_vm() and 
> > > replaces it
> > > with a gup_flags parameter as use of this function previously _implied_
> > > FOLL_FORCE, whereas after this patch callers explicitly pass this flag.
> > >
> > > We make this explicit as use of FOLL_FORCE can result in surprising 
> > > behaviour
> > > (and hence bugs) within the mm subsystem.
> > >
> > > Signed-off-by: Lorenzo Stoakes 
> >
> > So I'm not convinced this (and the following two patches) is actually
> > helping much. By grepping for FOLL_FORCE we will easily see that any caller
> > of access_remote_vm() gets that semantics and can thus continue search
>
> I am really wondering. Is there anything inherent that would require
> FOLL_FORCE for access_remote_vm? I mean FOLL_FORCE is a really
> non-trivial thing. It doesn't obey vma permissions so we should really
> minimize its usage. Do all of those users really need FOLL_FORCE?

I wonder about this also, for example by accessing /proc/self/mem you trigger
access_remote_vm() and consequently get_user_pages_remote() meaning FOLL_FORCE
is implied and you can use /proc/self/mem to override any VMA permissions. I
wonder if this is desirable behaviour or whether this ought to be limited to
ptrace system calls. Regardless, by making the flag more visible it makes it
easier to see that this is happening.

Note that this /proc/self/mem behaviour is what triggered the bug that brought
about this discussion in the first place -
https://marc.info/?l=linux-mm=147363447105059 - as using /proc/self/mem this
way on PROT_NONE memory broke some assumptions.

On the other hand I see your point Jan in that you know any caller of these
functions will have FOLL_FORCE implied, and you have to decide to stop passing
the flag at _some_ point in the stack, however it seems fairly sane to have that
stopping point exist outside of exported gup functions so the gup interface
forces explicitness but callers can wrap things as they need.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 1/2] media: i2c/ov5645: add the device tree binding document

[Laurent Pinchart]

Hi Todor,

On Friday 14 Oct 2016 15:01:08 Todor Tomov wrote:
> On 09/08/2016 03:22 PM, Laurent Pinchart wrote:
> > On Thursday 08 Sep 2016 12:13:54 Todor Tomov wrote:
> >> Add the document for ov5645 device tree binding.
> >> 
> >> Signed-off-by: Todor Tomov 
> >> ---
> >> 
> >>  .../devicetree/bindings/media/i2c/ov5645.txt   | 52 
> >>  1 file changed, 52 insertions(+)
> >>  create mode 100644
> >>  Documentation/devicetree/bindings/media/i2c/ov5645.txt
> >> 
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> >> b/Documentation/devicetree/bindings/media/i2c/ov5645.txt new file mode
> >> 100644
> >> index 000..bcf6dba
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
> >> @@ -0,0 +1,52 @@
> >> +* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
> >> +
> >> +The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image
> >> sensor with
> >> +an active array size of 2592H x 1944V. It is programmable through a
> >> serial I2C
> >> +interface.
> >> +
> >> +Required Properties:
> >> +- compatible: Value should be "ovti,ov5645".
> >> +- clocks: Reference to the xclk clock.
> >> +- clock-names: Should be "xclk".
> >> +- clock-frequency: Frequency of the xclk clock.
> >> +- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH.

By the way, isn't the pin called pwdnb and isn't it active low ?

> >> +- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW.
> > 
> > Shouldn't the enable and reset GPIOs be optional ?
> 
> I don't think so. The operations on the GPIOs are part of the power up
> sequence of the sensor so we must have control over them to execute the
> exact sequence.

Right, let's keep them mandatory. If we later have to make them optional for a 
board that pulls one of those signals up (assuming this can work at all) we'll 
revisit the bindings.

> >> +- vdddo-supply: Chip digital IO regulator.
> >> +- vdda-supply: Chip analog regulator.
> >> +- vddd-supply: Chip digital core regulator.
> >> +
> >> +The device node must contain one 'port' child node for its digital
> >> output
> >> +video port, in accordance with the video interface bindings defined in
> >> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> >> +
> >> +Example:
> >> +
> >> +   {
> >> +  ...
> >> +
> >> +  ov5645: ov5645@78 {
> >> +  compatible = "ovti,ov5645";
> >> +  reg = <0x78>;
> >> +
> >> +  enable-gpios = < 6 GPIO_ACTIVE_HIGH>;
> >> +  reset-gpios = < 20 GPIO_ACTIVE_LOW>;
> >> +  pinctrl-names = "default";
> >> +  pinctrl-0 = <_rear_default>;
> >> +
> >> +  clocks = < 200>;
> >> +  clock-names = "xclk";
> >> +  clock-frequency = <2388>;
> >> +
> >> +  vdddo-supply = <_dovdd_1v8>;
> >> +  vdda-supply = <_avdd_2v8>;
> >> +  vddd-supply = <_dvdd_1v2>;
> >> +
> >> +  port {
> >> +  ov5645_ep: endpoint {
> >> +  clock-lanes = <1>;
> >> +  data-lanes = <0 2>;
> >> +  remote-endpoint = <_ep>;
> >> +  };
> >> +  };
> >> +  };
> >> +  };

-- 
Regards,

Laurent Pinchart

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

[PATCH v2 1/1] doc-rst: v4l: Add documentation on CSI-2 bus configuration

[Sakari Ailus]

Document the interface between the CSI-2 transmitter and receiver drivers.

Signed-off-by: Sakari Ailus 
---
Hi Philipp,

Indeed the pixel rate is used by some driver as well.

How about this one instead?

The HTML page is available here (without CCS unfortunately):



since v1:
 
- Add PIXEL_RATE to the required controls.

- Document how pixel rate is calculated from the link frequency.

 Documentation/media/kapi/csi2.rst  | 59 ++
 Documentation/media/media_kapi.rst |  1 +
 2 files changed, 60 insertions(+)
 create mode 100644 Documentation/media/kapi/csi2.rst

diff --git a/Documentation/media/kapi/csi2.rst 
b/Documentation/media/kapi/csi2.rst
new file mode 100644
index 000..31f927d
--- /dev/null
+++ b/Documentation/media/kapi/csi2.rst
@@ -0,0 +1,59 @@
+MIPI CSI-2
+==
+
+CSI-2 is a data bus intended for transferring images from cameras to
+the host SoC. It is defined by the `MIPI alliance`_.
+
+.. _`MIPI alliance`: http://www.mipi.org/
+
+Transmitter drivers
+---
+
+CSI-2 transmitter, such as a sensor or a TV tuner, drivers need to
+provide the CSI-2 receiver with information on the CSI-2 bus
+configuration. These include the V4L2_CID_LINK_FREQ and
+V4L2_CID_PIXEL_RATE controls and
+(:c:type:`v4l2_subdev_video_ops`->s_stream() callback). These
+interface elements must be present on the sub-device represents the
+CSI-2 transmitter.
+
+The V4L2_CID_LINK_FREQ control is used to tell the receiver driver the
+frequency (and not the symbol rate) of the link. The
+V4L2_CID_PIXEL_RATE is may be used by the receiver to obtain the pixel
+rate the transmitter uses. The
+:c:type:`v4l2_subdev_video_ops`->s_stream() callback provides an
+ability to start and stop the stream.
+
+The value of the V4L2_CID_PIXEL_RATE is calculated as follows::
+
+   pixel_rate = link_freq * 2 * nr_of_lanes
+
+where
+
+.. list-table:: variables in pixel rate calculation
+   :header-rows: 1
+
+   * - variable or constant
+ - description
+   * - link_freq
+ - The value of the V4L2_CID_LINK_FREQ integer64 menu item.
+   * - nr_of_lanes
+ - Number of data lanes used on the CSI-2 link. This can
+   be obtained from the OF endpoint configuration.
+   * - 2
+ - Two bits are transferred per clock cycle per lane.
+
+The transmitter drivers must configure the CSI-2 transmitter to *LP-11
+mode* whenever the transmitter is powered on but not active. Some
+transmitters do this automatically but some have to be explicitly
+programmed to do so.
+
+Receiver drivers
+
+
+Before the receiver driver may enable the CSI-2 transmitter by using
+the :c:type:`v4l2_subdev_video_ops`->s_stream(), it must have powered
+the transmitter up by using the
+:c:type:`v4l2_subdev_core_ops`->s_power() callback. This may take
+place either indirectly by using :c:func:`v4l2_pipeline_pm_use` or
+directly.
diff --git a/Documentation/media/media_kapi.rst 
b/Documentation/media/media_kapi.rst
index f282ca2..bc06389 100644
--- a/Documentation/media/media_kapi.rst
+++ b/Documentation/media/media_kapi.rst
@@ -33,3 +33,4 @@ For more details see the file COPYING in the source 
distribution of Linux.
 kapi/rc-core
 kapi/mc-core
 kapi/cec-core
+kapi/csi2
-- 
2.7.4

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

Re: [PATCH v2 53/58] firewire: don't break long lines

[Mauro Carvalho Chehab]

Em Wed, 19 Oct 2016 08:03:01 +0900
Takashi Sakamoto  escreveu:

> From: Takashi Sakamoto 
> Date: Wed, 19 Oct 2016 07:53:35 +0900
> Subject: [PATCH] [media] firewire: use dev_dbg() instead of printk()
> 
> A structure for firedtv (struct firedtv) has a member for a pointer to
> struct device. In this case, we can use dev_dbg() for debug printing.
> This is more preferrable behaviour in device driver development.
> 
> Signed-off-by: Takashi Sakamoto 
> ---
>  drivers/media/firewire/firedtv-rc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/firewire/firedtv-rc.c
> b/drivers/media/firewire/firedtv-rc.c
> index f82d4a9..04dea2a 100644
> --- a/drivers/media/firewire/firedtv-rc.c
> +++ b/drivers/media/firewire/firedtv-rc.c
> @@ -184,8 +184,9 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned int 
> code)
>   else if (code >= 0x4540 && code <= 0x4542)
>   code = oldtable[code - 0x4521];
>   else {
> - printk(KERN_DEBUG "firedtv: invalid key code 0x%04x "
> -"from remote control\n", code);
> + dev_dbg(fdtv->device,
> + "invalid key code 0x%04x from remote control\n",
> + code);
>   return;
>   }

Looks good to me. Applied to my development tree:

https://git.linuxtv.org/mchehab/experimental.git/commit/?h=printk=e0de6d90145753bf40415d670471fcc536b2a26c

https://git.linuxtv.org/mchehab/experimental.git/commit/?h=printk=9532ba4af6a7619bb028ddd3b829e6f163917b79

Stefan,

Would the above be OK for you?

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

Re: [PATCH v2 53/58] firewire: don't break long lines

[Mauro Carvalho Chehab]

Em Wed, 19 Oct 2016 09:56:25 +0200
Stefan Richter  escreveu:

> On Oct 19 Takashi Sakamoto wrote:
> > --- a/drivers/media/firewire/firedtv-rc.c
> > +++ b/drivers/media/firewire/firedtv-rc.c
> > @@ -184,8 +184,9 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned
> > int code)
> > else if (code >= 0x4540 && code <= 0x4542)
> > code = oldtable[code - 0x4521];
> > else {
> > -   printk(KERN_DEBUG "firedtv: invalid key code 0x%04x "
> > -  "from remote control\n", code);
> > +   dev_dbg(fdtv->device,
> > +   "invalid key code 0x%04x from remote control\n",
> > +   code);
> > return;
> > }
> >   
> 
> Yes, dev_XYZ(fdtv->device, ...) is better here and is already used this
> way throughout the firedtv driver.  firedtv-rc.c somehow fell through the
> cracks when firedtv was made to use dev_XYZ().
> 
> (On an unrelated note, this reminds me that I still need to take care of
> Mauro's patches "Add a keymap for FireDTV board" and "firedtv: Port it to
> use rc_core" from May 28, 2012.)

Oh! I forgot about those a long time ago ;) Yeah, it would be great if
you could look into those patches when you have some time.

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

Re: [PATCH v2 1/3] ARM: dts: r8a7793: Enable VIN0-VIN2

[Geert Uytterhoeven]

On Tue, Oct 18, 2016 at 5:02 PM, Ulrich Hecht
 wrote:
> Signed-off-by: Ulrich Hecht 

Reviewed-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 53/58] firewire: don't break long lines

[Mauro Carvalho Chehab]

Em Wed, 19 Oct 2016 10:01:13 +0200
Stefan Richter  escreveu:

> On Oct 18 Mauro Carvalho Chehab wrote:
> [...]
> > The patch was generated via the script below, and manually
> > adjusted if needed.  
> [...]
> > Reviewed-by: Takashi Sakamoto 
> > Acked-by: Stefan Richter 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  drivers/media/firewire/firedtv-rc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)  
> [...]
> 
> Patch v1 also had a hunk in drivers/media/firewire/firedtv-avc.c.
> What happened to it?

There was some mistake when I manipulated it manually, after re-run
the script.

Btw, I liked more the Takashi's dev_dbg() patch for firedtv-rc.c.

So, I guess the better would be to apply his patch and the one
enclosed here, with just the firedtv-avc.c change.

Thanks,
Mauro

[PATCH] firewire: don't break long lines

Due to the 80-cols restrictions, and latter due to checkpatch
warnings, several strings were broken into multiple lines. This
is not considered a good practice anymore, as it makes harder
to grep for strings at the source code.

As we're right now fixing other drivers due to KERN_CONT, we need
to be able to identify what printk strings don't end with a "\n".
It is a way easier to detect those if we don't break long lines.

So, join those continuation lines.

The patch was generated via the script below, and manually
adjusted if needed.


use Text::Tabs;
while (<>) {
if ($next ne "") {
$c=$_;
if ($c =~ /^\s+\"(.*)/) {
$c2=$1;
$next =~ s/\"\n$//;
$n = expand($next);
$funpos = index($n, '(');
$pos = index($c2, '",');
if ($funpos && $pos > 0) {
$s1 = substr $c2, 0, $pos + 2;
$s2 = ' ' x ($funpos + 1) . substr $c2, $pos + 
2;
$s2 =~ s/^\s+//;

$s2 = ' ' x ($funpos + 1) . $s2 if ($s2 ne "");

print unexpand("$next$s1\n");
print unexpand("$s2\n") if ($s2 ne "");
} else {
print "$next$c2\n";
}
$next="";
next;
} else {
print $next;
}
$next="";
} else {
if (m/\"$/) {
if (!m/\\n\"$/) {
$next=$_;
next;
}
}
}
print $_;
}


Reviewed-by: Takashi Sakamoto 
Acked-by: Stefan Richter 
Signed-off-by: Mauro Carvalho Chehab 

diff --git a/drivers/media/firewire/firedtv-avc.c 
b/drivers/media/firewire/firedtv-avc.c
index 251a556112a9..5bde6c209cd7 100644
--- a/drivers/media/firewire/firedtv-avc.c
+++ b/drivers/media/firewire/firedtv-avc.c
@@ -1181,8 +1181,8 @@ int avc_ca_pmt(struct firedtv *fdtv, char *msg, int 
length)
if (es_info_length > 0) {
pmt_cmd_id = msg[read_pos++];
if (pmt_cmd_id != 1 && pmt_cmd_id != 4)
-   dev_err(fdtv->device, "invalid pmt_cmd_id %d "
-   "at stream level\n", pmt_cmd_id);
+   dev_err(fdtv->device, "invalid pmt_cmd_id %d at 
stream level\n",
+   pmt_cmd_id);
 
if (es_info_length > sizeof(c->operand) - 4 -
 write_pos) {
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/2] Add support for Omnivision OV5647

[Pavel Machek]

Hi!

> >> +struct regval_list {
> >> +  uint16_t addr;
> >> +  uint8_t data;
> >> +};
> > u8/u16?
> 
> This sensor uses 16 bits for addresses and 8 for data, so I think it makes 
> sense
> to keep it this way.

Yes, you can do it. But please use u8/u16 types (also elsewhere in the
driver), they are more common in ther kernel.

> Thanks for the feedback. I agree with most of your suggestions, and I 
> commented
> with the one I didn't agree.

You are welcome,
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 v3 2/2] Add support for Omnivision OV5647

[Ramiro Oliveira]

Hi!

On 10/18/2016 7:31 PM, Pavel Machek wrote:
> Hi!
>
>> +/*
>> + * A V4L2 driver for OmniVision OV5647 cameras.
>> + *
>> + * Based on Samsung S5K6AAFX SXGA 1/6" 1.3M CMOS Image Sensor driver
>> + * Copyright (C) 2011 Sylwester Nawrocki 
>> + *
>> + * Based on Omnivision OV7670 Camera Driver
>> + * Copyright (C) 2006-7 Jonathan Corbet 
>> + *
>> + * Copyright (C) 2016, Synopsys, Inc.
>> + *
>> + */
> If you do copyrights, you should also specify license.
>
>> +static bool debug;
>> +module_param(debug, bool, 0644);
>> +MODULE_PARM_DESC(debug, "Debug level (0-1)");
> Please remove, we have generic infrastructure for debugging.
>
>> +/*
>> + * The ov5647 sits on i2c with ID 0x6c
>> + */
>> +#define OV5647_I2C_ADDR 0x6c
>> +#define SENSOR_NAME "ov5647"
>> +
>> +#define OV5647_REG_CHIPID_H 0x300A
>> +#define OV5647_REG_CHIPID_L 0x300B
>> +
>> +#define REG_TERM 0xfffe
>> +#define VAL_TERM 0xfe
>> +#define REG_DLY  0x
>> +
>> +/*define the voltage level of control signal*/
> "/* ", " */".
>
>> +#define CSI_STBY_ON 1
>> +#define CSI_STBY_OFF0
>> +#define CSI_RST_ON  0
>> +#define CSI_RST_OFF 1
>> +#define CSI_PWR_ON  1
>> +#define CSI_PWR_OFF 0
>> +#define CSI_AF_PWR_ON   1
>> +#define CSI_AF_PWR_OFF  0
> ...
>> +enum power_seq_cmd {
>> +CSI_SUBDEV_PWR_OFF = 0x00,
>> +CSI_SUBDEV_PWR_ON = 0x01,
>> +};
> Pick one style for defines/enums?
>
>> +struct regval_list {
>> +uint16_t addr;
>> +uint8_t data;
>> +};
> u8/u16?

This sensor uses 16 bits for addresses and 8 for data, so I think it makes sense
to keep it this way.

>
>> +/**
>> +* @short I2C Write operation
>> +* @param[in] i2c_client I2C client
>> +* @param[in] reg register address
>> +* @param[in] val value to write
>> +* @return Error code
>> +*/
> " *"?
>
>> +static int ov5647_write(struct v4l2_subdev *sd, uint16_t reg, uint8_t val)
>> +{
>> +int ret;
>> +unsigned char data[3] = { reg >> 8, reg & 0xff, val};
> " }".
>
>> +static int ov5647_write_array(struct v4l2_subdev *subdev,
>> +struct regval_list *regs, int array_size)
>> +{
>> +int i = 0;
>> +int ret = 0;
>> +
>> +if (!regs)
>> +return -EINVAL;
>> +
>> +while (i < array_size) {
>> +if (regs->addr == REG_DLY)
>> +mdelay(regs->data);
>> +else
>> +ret = ov5647_write(subdev, regs->addr, regs->data);
> The "REG_DLY" is never used AFAICT? Remove?
>
>> +if (ret == -EIO)
>> +return ret;
>> +
> ov5647_write() can return errors other then EIO. Are they handled correctly?
>
>> +/**
>> + * @short Set SW standby
>> + * @param[in] subdev v4l2 subdev
>> + * @param[in] on_off standby on or off
>> + * @return Error code
>> + */
>> +static int sensor_s_sw_stby(struct v4l2_subdev *subdev, int on_off)
>> +{
>> +int ret;
>> +unsigned char rdval;
>> +
>> +ret = ov5647_read(subdev, 0x0100, );
>> +if (ret != 0)
>> +return ret;
>> +
>> +if (on_off == CSI_STBY_ON)
>> +ret = ov5647_write(subdev, 0x0100, rdval&0xfe);
>> +
>> +else
>> +ret = ov5647_write(subdev, 0x0100, rdval|0x01);
> I'd get rid of CSI_STBY_ON, and convert arg to bool, as you don't
> really handle other values. Plus, naming it "set_sw_standby()" would
> make core slightly more readable. Also kill the empty line before
> else.
>
>> +/**
>> +* @short Initialize sensor
>> +* @param[in] subdev v4l2 subdev
>> +* @param[in] val not used
>> +* @return Error code
>> +*/
>> +static int __sensor_init(struct v4l2_subdev *subdev)
>> +{
>> +int ret;
>> +uint8_t resetval;
>> +unsigned char rdval;
> u8 for both?
>
>> +ov5647_read(subdev, 0x0100, );
>> +if (!resetval&0x01) {
>> +v4l2_dbg(1, debug, subdev,
>> +"DEVICE WAS IN SOFTWARE STANDBY");
> No shouting please? If it is important maybe it should have higher
> priority?
>
>> +static int sensor_power(struct v4l2_subdev *subdev, int on)
>> +{
>> +int ret;
>> +struct ov5647 *ov5647 = to_state(subdev);
>> +
>> +ret = 0;
>> +mutex_lock(>lock);
>> +
>> +switch (on) {
>> +case CSI_SUBDEV_PWR_OFF:
> ...
>> +case CSI_SUBDEV_PWR_ON:
> ...
>> +default:
>> +return -EINVAL;
>> +}
>> +
>> +mutex_unlock(>lock);
> I'd really convert this to bool. Note how it returns with lock held?
>
>
>> +int ov5647_detect(struct v4l2_subdev *sd)
>> +{
>> +unsigned char v;
>> +int ret;
>> +
>> +ret = sensor_power(sd, 1);
>> +if (ret < 0)
>> +return ret;
>> +ret = ov5647_read(sd, OV5647_REG_CHIPID_H, );
>> +if (ret < 0)
>> +return ret;
>> +if (v != 0x56) /* OV manuf. id. */
>> +return -ENODEV;
>> +ret = ov5647_read(sd, OV5647_REG_CHIPID_L, );
>> +if (ret < 0)
>> +return ret;
>> +

Re: [PATCH v2 50/58] v4l2-core: don't break long lines

[Sakari Ailus]

Hi Mauro,

On Tue, Oct 18, 2016 at 06:46:02PM -0200, Mauro Carvalho Chehab wrote:
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index c52d94c018bb..26fe7aef1196 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -174,8 +174,7 @@ static void v4l_print_querycap(const void *arg, bool 
> write_only)
>  {
>   const struct v4l2_capability *p = arg;
>  
> - pr_cont("driver=%.*s, card=%.*s, bus=%.*s, version=0x%08x, "
> - "capabilities=0x%08x, device_caps=0x%08x\n",
> + pr_cont("driver=%.*s, card=%.*s, bus=%.*s, version=0x%08x, 
> capabilities=0x%08x, device_caps=0x%08x\n",
>   (int)sizeof(p->driver), p->driver,
>   (int)sizeof(p->card), p->card,
>   (int)sizeof(p->bus_info), p->bus_info,

I still wouldn't do this to v4l2-ioctl.c. It does not improve grappability
because of the format strings. Some are also really long such as the one a
few chunks below.

Other than that, this looks very nice now. Your script makes me wonder,
though, whether there should be a tool to automatically improve coding style
for cases such as this. I didn't realise so many strings were actually
split. I'm sure also the rest of the kernel would benefit from such a tool.
With the increased number of lines of code, the special cases that need to
be handled manually must decrease as well or it becomes unfeasible.

> @@ -186,8 +185,7 @@ static void v4l_print_enuminput(const void *arg, bool 
> write_only)
>  {
>   const struct v4l2_input *p = arg;
>  
> - pr_cont("index=%u, name=%.*s, type=%u, audioset=0x%x, tuner=%u, "
> - "std=0x%08Lx, status=0x%x, capabilities=0x%x\n",
> + pr_cont("index=%u, name=%.*s, type=%u, audioset=0x%x, tuner=%u, 
> std=0x%08Lx, status=0x%x, capabilities=0x%x\n",
>   p->index, (int)sizeof(p->name), p->name, p->type, p->audioset,
>   p->tuner, (unsigned long long)p->std, p->status,
>   p->capabilities);
> @@ -197,8 +195,7 @@ static void v4l_print_enumoutput(const void *arg, bool 
> write_only)
>  {
>   const struct v4l2_output *p = arg;
>  
> - pr_cont("index=%u, name=%.*s, type=%u, audioset=0x%x, "
> - "modulator=%u, std=0x%08Lx, capabilities=0x%x\n",
> + pr_cont("index=%u, name=%.*s, type=%u, audioset=0x%x, modulator=%u, 
> std=0x%08Lx, capabilities=0x%x\n",
>   p->index, (int)sizeof(p->name), p->name, p->type, p->audioset,
>   p->modulator, (unsigned long long)p->std, p->capabilities);
>  }
> @@ -256,11 +253,7 @@ static void v4l_print_format(const void *arg, bool 
> write_only)
>   case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>   case V4L2_BUF_TYPE_VIDEO_OUTPUT:
>   pix = >fmt.pix;
> - pr_cont(", width=%u, height=%u, "
> - "pixelformat=%c%c%c%c, field=%s, "
> - "bytesperline=%u, sizeimage=%u, colorspace=%d, "
> - "flags=0x%x, ycbcr_enc=%u, quantization=%u, "
> - "xfer_func=%u\n",
> + pr_cont(", width=%u, height=%u, pixelformat=%c%c%c%c, field=%s, 
> bytesperline=%u, sizeimage=%u, colorspace=%d, flags=0x%x, ycbcr_enc=%u, 
> quantization=%u, xfer_func=%u\n",
>   pix->width, pix->height,
>   (pix->pixelformat & 0xff),
>   (pix->pixelformat >>  8) & 0xff,
> @@ -274,10 +267,7 @@ static void v4l_print_format(const void *arg, bool 
> write_only)
>   case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>   case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>   mp = >fmt.pix_mp;
> - pr_cont(", width=%u, height=%u, "
> - "format=%c%c%c%c, field=%s, "
> - "colorspace=%d, num_planes=%u, flags=0x%x, "
> - "ycbcr_enc=%u, quantization=%u, xfer_func=%u\n",
> + pr_cont(", width=%u, height=%u, format=%c%c%c%c, field=%s, 
> colorspace=%d, num_planes=%u, flags=0x%x, ycbcr_enc=%u, quantization=%u, 
> xfer_func=%u\n",
>   mp->width, mp->height,
>   (mp->pixelformat & 0xff),
>   (mp->pixelformat >>  8) & 0xff,
> @@ -306,8 +296,7 @@ static void v4l_print_format(const void *arg, bool 
> write_only)
>   case V4L2_BUF_TYPE_VBI_CAPTURE:
>   case V4L2_BUF_TYPE_VBI_OUTPUT:
>   vbi = >fmt.vbi;
> - pr_cont(", sampling_rate=%u, offset=%u, samples_per_line=%u, "
> - "sample_format=%c%c%c%c, start=%u,%u, count=%u,%u\n",
> + pr_cont(", sampling_rate=%u, offset=%u, samples_per_line=%u, 
> sample_format=%c%c%c%c, start=%u,%u, count=%u,%u\n",
>   vbi->sampling_rate, vbi->offset,
>   vbi->samples_per_line,
>   (vbi->sample_format & 0xff),
> @@ -343,9 +332,7 @@ static void v4l_print_framebuffer(const void *arg, bool 
> write_only)
>  {
>   const struct 

Re: [PATCH v2 50/58] v4l2-core: don't break long lines

[Mauro Carvalho Chehab]

Em Wed, 19 Oct 2016 10:09:16 +0300
Sakari Ailus  escreveu:

> Hi Mauro,
> 
> On Tue, Oct 18, 2016 at 06:46:02PM -0200, Mauro Carvalho Chehab wrote:
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c 
> > b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index c52d94c018bb..26fe7aef1196 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -174,8 +174,7 @@ static void v4l_print_querycap(const void *arg, bool 
> > write_only)
> >  {
> > const struct v4l2_capability *p = arg;
> >  
> > -   pr_cont("driver=%.*s, card=%.*s, bus=%.*s, version=0x%08x, "
> > -   "capabilities=0x%08x, device_caps=0x%08x\n",
> > +   pr_cont("driver=%.*s, card=%.*s, bus=%.*s, version=0x%08x, 
> > capabilities=0x%08x, device_caps=0x%08x\n",
> > (int)sizeof(p->driver), p->driver,
> > (int)sizeof(p->card), p->card,
> > (int)sizeof(p->bus_info), p->bus_info,  
> 
> I still wouldn't do this to v4l2-ioctl.c. It does not improve grappability
> because of the format strings. 

The main reason that made me do this patch series is to identify the lack
of KERN_CONT at the media subsystem.

The grep I'm using to identify missing KERN_CONT lines actually tests
for a string line that doesn't end with "\n", like:

$ git grep '("' drivers/media/|grep -v KERN_|grep -v '\\n'|grep -v 
MODULE

That's said, the format strings don't hurt grep:

$ git grep -E "driver=.*bus=.*device_caps="
drivers/media/v4l2-core/v4l2-ioctl.c:   pr_cont("driver=%.*s, 
card=%.*s, bus=%.*s, version=0x%08x, capabilities=0x%08x, device_caps=0x%08x\n",

> Some are also really long such as the one a
> few chunks below.

Yeah, but it gives a bad coding style example. I prefer to have the core
subsystem as close as possible of the coding style I would like to see
at the drivers code I review. Btw, at least on my 1920x1050 monitor,
if I open a console full screen, only one line of v4l2-ioctl.c is bigger
than the terminal column size.

> Other than that, this looks very nice now. Your script makes me wonder,
> though, whether there should be a tool to automatically improve coding style
> for cases such as this. I didn't realise so many strings were actually
> split. I'm sure also the rest of the kernel would benefit from such a tool.
> With the increased number of lines of code, the special cases that need to
> be handled manually must decrease as well or it becomes unfeasible.

Yeah, I was also thinking that there would be a way less places than
what it was hit by this script.

The script on this patch is generic enough to be used to fix such cases
on other subsystems, although it needs some polish to cover a few corner
cases. I suspect it shouldn't be hard to integrate it to checkpatch.pl
to be used with --fix.

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

Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Jan Kara]

On Thu 13-10-16 01:20:18, Lorenzo Stoakes wrote:
> This patch removes the write parameter from __access_remote_vm() and replaces 
> it
> with a gup_flags parameter as use of this function previously _implied_
> FOLL_FORCE, whereas after this patch callers explicitly pass this flag.
> 
> We make this explicit as use of FOLL_FORCE can result in surprising behaviour
> (and hence bugs) within the mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes 

So I'm not convinced this (and the following two patches) is actually
helping much. By grepping for FOLL_FORCE we will easily see that any caller
of access_remote_vm() gets that semantics and can thus continue search
accordingly (it is much simpler than searching for all get_user_pages()
users and extracting from parameter lists what they actually pass as
'force' argument). Sure it makes somewhat more visible to callers of
access_remote_vm() that they get FOLL_FORCE semantics but OTOH it also
opens a space for issues where a caller of access_remote_vm() actually
wants FOLL_FORCE (and currently all of them want it) and just mistakenly
does not set it. All in all I'd prefer to keep access_remote_vm() and
friends as is...

Honza

> ---
>  mm/memory.c | 23 +++
>  mm/nommu.c  |  9 ++---
>  2 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 20a9adb..79ebed3 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3869,14 +3869,11 @@ EXPORT_SYMBOL_GPL(generic_access_phys);
>   * given task for page fault accounting.
>   */
>  static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
> - unsigned long addr, void *buf, int len, int write)
> + unsigned long addr, void *buf, int len, unsigned int gup_flags)
>  {
>   struct vm_area_struct *vma;
>   void *old_buf = buf;
> - unsigned int flags = FOLL_FORCE;
> -
> - if (write)
> - flags |= FOLL_WRITE;
> + int write = gup_flags & FOLL_WRITE;
>  
>   down_read(>mmap_sem);
>   /* ignore errors, just check how much was successfully transferred */
> @@ -3886,7 +3883,7 @@ static int __access_remote_vm(struct task_struct *tsk, 
> struct mm_struct *mm,
>   struct page *page = NULL;
>  
>   ret = get_user_pages_remote(tsk, mm, addr, 1,
> - flags, , );
> + gup_flags, , );
>   if (ret <= 0) {
>  #ifndef CONFIG_HAVE_IOREMAP_PROT
>   break;
> @@ -3945,7 +3942,12 @@ static int __access_remote_vm(struct task_struct *tsk, 
> struct mm_struct *mm,
>  int access_remote_vm(struct mm_struct *mm, unsigned long addr,
>   void *buf, int len, int write)
>  {
> - return __access_remote_vm(NULL, mm, addr, buf, len, write);
> + unsigned int flags = FOLL_FORCE;
> +
> + if (write)
> + flags |= FOLL_WRITE;
> +
> + return __access_remote_vm(NULL, mm, addr, buf, len, flags);
>  }
>  
>  /*
> @@ -3958,12 +3960,17 @@ int access_process_vm(struct task_struct *tsk, 
> unsigned long addr,
>  {
>   struct mm_struct *mm;
>   int ret;
> + unsigned int flags = FOLL_FORCE;
>  
>   mm = get_task_mm(tsk);
>   if (!mm)
>   return 0;
>  
> - ret = __access_remote_vm(tsk, mm, addr, buf, len, write);
> + if (write)
> + flags |= FOLL_WRITE;
> +
> + ret = __access_remote_vm(tsk, mm, addr, buf, len, flags);
> +
>   mmput(mm);
>  
>   return ret;
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 70cb844..bde7df3 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -1809,9 +1809,10 @@ void filemap_map_pages(struct fault_env *fe,
>  EXPORT_SYMBOL(filemap_map_pages);
>  
>  static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
> - unsigned long addr, void *buf, int len, int write)
> + unsigned long addr, void *buf, int len, unsigned int gup_flags)
>  {
>   struct vm_area_struct *vma;
> + int write = gup_flags & FOLL_WRITE;
>  
>   down_read(>mmap_sem);
>  
> @@ -1853,7 +1854,8 @@ static int __access_remote_vm(struct task_struct *tsk, 
> struct mm_struct *mm,
>  int access_remote_vm(struct mm_struct *mm, unsigned long addr,
>   void *buf, int len, int write)
>  {
> - return __access_remote_vm(NULL, mm, addr, buf, len, write);
> + return __access_remote_vm(NULL, mm, addr, buf, len,
> + write ? FOLL_WRITE : 0);
>  }
>  
>  /*
> @@ -1871,7 +1873,8 @@ int access_process_vm(struct task_struct *tsk, unsigned 
> long addr, void *buf, in
>   if (!mm)
>   return 0;
>  
> - len = __access_remote_vm(tsk, mm, addr, buf, len, write);
> + len = __access_remote_vm(tsk, mm, addr, buf, len,
> + write ? FOLL_WRITE : 0);
>  
>   mmput(mm);
>   return len;
> -- 
> 2.10.0
> 
-- 
Jan Kara 
SUSE Labs, CR

Re: [PATCH v2 1/1] doc-rst: v4l: Add documentation on CSI-2 bus configuration

[Philipp Zabel]

Am Mittwoch, den 19.10.2016, 17:27 +0300 schrieb Sakari Ailus:
> On 10/19/16 17:24, Philipp Zabel wrote:
> > Am Mittwoch, den 19.10.2016, 15:59 +0300 schrieb Sakari Ailus:
> >> Document the interface between the CSI-2 transmitter and receiver drivers.
> >>
> >> Signed-off-by: Sakari Ailus 
> >> ---
> >> Hi Philipp,
> >>
> >> Indeed the pixel rate is used by some driver as well.
> >>
> >> How about this one instead?
> >>
> >> The HTML page is available here (without CCS unfortunately):
> >>
> >> 
> >>
> >> since v1:
> >>  
> >> - Add PIXEL_RATE to the required controls.
> >>
> >> - Document how pixel rate is calculated from the link frequency.
> >>
> >>  Documentation/media/kapi/csi2.rst  | 59 
> >> ++
> >>  Documentation/media/media_kapi.rst |  1 +
> >>  2 files changed, 60 insertions(+)
> >>  create mode 100644 Documentation/media/kapi/csi2.rst
> >>
> >> diff --git a/Documentation/media/kapi/csi2.rst 
> >> b/Documentation/media/kapi/csi2.rst
> >> new file mode 100644
> >> index 000..31f927d
> >> --- /dev/null
> >> +++ b/Documentation/media/kapi/csi2.rst
> >> @@ -0,0 +1,59 @@
> >> +MIPI CSI-2
> >> +==
> >> +
> >> +CSI-2 is a data bus intended for transferring images from cameras to
> >> +the host SoC. It is defined by the `MIPI alliance`_.
> >> +
> >> +.. _`MIPI alliance`: http://www.mipi.org/
> >> +
> >> +Transmitter drivers
> >> +---
> >> +
> >> +CSI-2 transmitter, such as a sensor or a TV tuner, drivers need to
> >> +provide the CSI-2 receiver with information on the CSI-2 bus
> >> +configuration. These include the V4L2_CID_LINK_FREQ and
> >> +V4L2_CID_PIXEL_RATE controls and
> >> +(:c:type:`v4l2_subdev_video_ops`->s_stream() callback). These
> >> +interface elements must be present on the sub-device represents the
> >> +CSI-2 transmitter.
> >> +
> >> +The V4L2_CID_LINK_FREQ control is used to tell the receiver driver the
> >> +frequency (and not the symbol rate) of the link. The
> >> +V4L2_CID_PIXEL_RATE is may be used by the receiver to obtain the pixel
> >> +rate the transmitter uses. The
> >> +:c:type:`v4l2_subdev_video_ops`->s_stream() callback provides an
> >> +ability to start and stop the stream.
> >> +
> >> +The value of the V4L2_CID_PIXEL_RATE is calculated as follows::
> >> +
> >> +  pixel_rate = link_freq * 2 * nr_of_lanes
> > 
> > This is the total bps, which must be divided by the bits per pixel
> > depending on the selected MEDIA_BUS_FMT, for example
> > /16 for MEDIA_BUS_FMT_UYVY8_1X16, or /24 for MEDIA_BUS_FMT_RGB888_1X24,
> > to obtain pixel_rate.
> 
> Uh, indeed. I'll change this.
> 
> > 
> >> +where
> >> +
> >> +.. list-table:: variables in pixel rate calculation
> >> +   :header-rows: 1
> >> +
> >> +   * - variable or constant
> >> + - description
> >> +   * - link_freq
> >> + - The value of the V4L2_CID_LINK_FREQ integer64 menu item.
> >> +   * - nr_of_lanes
> >> + - Number of data lanes used on the CSI-2 link. This can
> >> +   be obtained from the OF endpoint configuration.
> > 
> > I suppose the number of lanes should be calculated as
> > nr_of_lanes = DIV_ROUND_UP(pixel_rate * bpp, link_freq * 2)
> > in the receiver driver? Not all lanes configured in the device tree have
> > to be used, depending on the configured link frequencies and bus format.
> 
> Do we have any user for that yet?
> 
> I know there's hardware where this would be necessary in order to
> support all image sizes and formats for instance, but there's no driver yet.

The TC358743 driver is currently hardcoded to 297 MHz link frequency and
scales via the number of data lanes in use (1-4). Should the receiver
just ask it directly about the number of lanes in use via the
g_mbus_config video op?

regards
Philipp

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

Re: [PATCH v6 1/2] media: i2c/ov5645: add the device tree binding document

[Todor Tomov]

Hi Laurent,

Thank you for the review.

On 10/19/2016 11:49 AM, Laurent Pinchart wrote:
> Hi Todor,
> 
> On Friday 14 Oct 2016 15:01:08 Todor Tomov wrote:
>> On 09/08/2016 03:22 PM, Laurent Pinchart wrote:
>>> On Thursday 08 Sep 2016 12:13:54 Todor Tomov wrote:
 Add the document for ov5645 device tree binding.

 Signed-off-by: Todor Tomov 
 ---

  .../devicetree/bindings/media/i2c/ov5645.txt   | 52 
  1 file changed, 52 insertions(+)
  create mode 100644
  Documentation/devicetree/bindings/media/i2c/ov5645.txt

 diff --git a/Documentation/devicetree/bindings/media/i2c/ov5645.txt
 b/Documentation/devicetree/bindings/media/i2c/ov5645.txt new file mode
 100644
 index 000..bcf6dba
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/media/i2c/ov5645.txt
 @@ -0,0 +1,52 @@
 +* Omnivision 1/4-Inch 5Mp CMOS Digital Image Sensor
 +
 +The Omnivision OV5645 is a 1/4-Inch CMOS active pixel digital image
 sensor with
 +an active array size of 2592H x 1944V. It is programmable through a
 serial I2C
 +interface.
 +
 +Required Properties:
 +- compatible: Value should be "ovti,ov5645".
 +- clocks: Reference to the xclk clock.
 +- clock-names: Should be "xclk".
 +- clock-frequency: Frequency of the xclk clock.
 +- enable-gpios: Chip enable GPIO. Polarity is GPIO_ACTIVE_HIGH.
> 
> By the way, isn't the pin called pwdnb and isn't it active low ?

Yes, the pin is called "pwdnb" and is active low (must be up for power to be 
up).
I have changed the name to "enable" as it is more generally used - this change
was suggested by Rob Herring. As the logic switches with this change of the name
I have stated it is active high which ends up in the same condition (enable
must be up for the power to be up). I think this is correct, isn't it?

> 
 +- reset-gpios: Chip reset GPIO. Polarity is GPIO_ACTIVE_LOW.
>>>
>>> Shouldn't the enable and reset GPIOs be optional ?
>>
>> I don't think so. The operations on the GPIOs are part of the power up
>> sequence of the sensor so we must have control over them to execute the
>> exact sequence.
> 
> Right, let's keep them mandatory. If we later have to make them optional for 
> a 
> board that pulls one of those signals up (assuming this can work at all) 
> we'll 
> revisit the bindings.

Ok.

> 
 +- vdddo-supply: Chip digital IO regulator.
 +- vdda-supply: Chip analog regulator.
 +- vddd-supply: Chip digital core regulator.
 +
 +The device node must contain one 'port' child node for its digital
 output
 +video port, in accordance with the video interface bindings defined in
 +Documentation/devicetree/bindings/media/video-interfaces.txt.
 +
 +Example:
 +
 +   {
 +  ...
 +
 +  ov5645: ov5645@78 {
 +  compatible = "ovti,ov5645";
 +  reg = <0x78>;
 +
 +  enable-gpios = < 6 GPIO_ACTIVE_HIGH>;
 +  reset-gpios = < 20 GPIO_ACTIVE_LOW>;
 +  pinctrl-names = "default";
 +  pinctrl-0 = <_rear_default>;
 +
 +  clocks = < 200>;
 +  clock-names = "xclk";
 +  clock-frequency = <2388>;
 +
 +  vdddo-supply = <_dovdd_1v8>;
 +  vdda-supply = <_avdd_2v8>;
 +  vddd-supply = <_dvdd_1v2>;
 +
 +  port {
 +  ov5645_ep: endpoint {
 +  clock-lanes = <1>;
 +  data-lanes = <0 2>;
 +  remote-endpoint = <_ep>;
 +  };
 +  };
 +  };
 +  };
> 

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

Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Lorenzo Stoakes]

On Wed, Oct 19, 2016 at 10:52:05AM +0200, Michal Hocko wrote:
> yes this is the desirable and expected behavior.
>
> > wonder if this is desirable behaviour or whether this ought to be limited to
> > ptrace system calls. Regardless, by making the flag more visible it makes it
> > easier to see that this is happening.
>
> mem_open already enforces PTRACE_MODE_ATTACH

Ah I missed this, that makes a lot of sense, thanks!

I still wonder whether other invocations of access_remote_vm() in fs/proc/base.c
(the principle caller of this function) need FOLL_FORCE, for example the various
calls that simply read data from other processes, so I think the point stands
about keeping this explicit.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] doc-rst: v4l: Add documentation on CSI-2 bus configuration

[Philipp Zabel]

Am Dienstag, den 18.10.2016, 18:02 +0300 schrieb Sakari Ailus:
> Document the interface between the CSI-2 transmitter and receiver drivers.
> 
> Signed-off-by: Sakari Ailus 
> ---
> Hi folks,
> 
> We've got multiple CSI-2 tranmitter and receiver drivers currently in the
> kernel. Some receivers require information on dynamic bus parameters and
> some of the transmitter drivers implement that. It's time to document what
> is expected of both so we will not end up with multiple non-interoperable
> implementations.
> 
> Regards,
> Sakari
> 
>  Documentation/media/kapi/csi2.rst  | 35 +++
>  Documentation/media/media_kapi.rst |  1 +
>  2 files changed, 36 insertions(+)
>  create mode 100644 Documentation/media/kapi/csi2.rst
> 
> diff --git a/Documentation/media/kapi/csi2.rst 
> b/Documentation/media/kapi/csi2.rst
> new file mode 100644
> index 000..856a509
> --- /dev/null
> +++ b/Documentation/media/kapi/csi2.rst
> @@ -0,0 +1,35 @@
> +MIPI CSI-2
> +==
> +
> +CSI-2 is a data bus intended for transferring images from cameras to
> +the host SoC. It is defined by the `MIPI alliance`_.
> +
> +.. _`MIPI alliance`: http://www.mipi.org/
> +
> +Transmitter drivers
> +---
> +
> +CSI-2 transmitter, such as a sensor or a TV tuner, drivers need to
> +provide the CSI-2 receiver with information on the CSI-2 bus
> +configuration. These include the V4L2_CID_LINK_FREQ control and
> +(:c:type:`v4l2_subdev_video_ops`->s_stream() callback). Both must be
> +present on the sub-device represents the CSI-2 transmitter. The
> +V4L2_CID_LINK_FREQ control is used to tell the receiver driver the
> +frequency (and not the symbol rate) of the link and the
> +:c:type:`v4l2_subdev_video_ops`->s_stream() callback provides an
> +ability to start and stop the stream.
> +
> +The transmitter drivers must configure the CSI-2 transmitter to *LP-11
> +mode* whenever the transmitter is powered on but not active. Some
> +transmitters do this automatically but some have to be explicitly
> +programmed to do so.
> +
> +Receiver drivers
> +
> +
> +Before the receiver driver may enable the CSI-2 transmitter by using
> +the :c:type:`v4l2_subdev_video_ops`->s_stream(), it must have powered
> +the transmitter up by using the
> +:c:type:`v4l2_subdev_core_ops`->s_power() callback. This may take
> +place either indirectly by using :c:func:`v4l2_pipeline_pm_use` or
> +directly.
> diff --git a/Documentation/media/media_kapi.rst 
> b/Documentation/media/media_kapi.rst
> index f282ca2..bc06389 100644
> --- a/Documentation/media/media_kapi.rst
> +++ b/Documentation/media/media_kapi.rst
> @@ -33,3 +33,4 @@ For more details see the file COPYING in the source 
> distribution of Linux.
>  kapi/rc-core
>  kapi/mc-core
>  kapi/cec-core
> +kapi/csi2

Reviewed-by: Philipp Zabel 

so far. Should this document also include a suggestion on how exactly to
calculate the number of lanes to use from the pixel rate and link
frequency?

regards
Philipp


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

Re: [PATCH v2 53/58] firewire: don't break long lines

[Stefan Richter]

On Oct 19 Takashi Sakamoto wrote:
> --- a/drivers/media/firewire/firedtv-rc.c
> +++ b/drivers/media/firewire/firedtv-rc.c
> @@ -184,8 +184,9 @@ void fdtv_handle_rc(struct firedtv *fdtv, unsigned
> int code)
>   else if (code >= 0x4540 && code <= 0x4542)
>   code = oldtable[code - 0x4521];
>   else {
> - printk(KERN_DEBUG "firedtv: invalid key code 0x%04x "
> -"from remote control\n", code);
> + dev_dbg(fdtv->device,
> + "invalid key code 0x%04x from remote control\n",
> + code);
>   return;
>   }
> 

Yes, dev_XYZ(fdtv->device, ...) is better here and is already used this
way throughout the firedtv driver.  firedtv-rc.c somehow fell through the
cracks when firedtv was made to use dev_XYZ().

(On an unrelated note, this reminds me that I still need to take care of
Mauro's patches "Add a keymap for FireDTV board" and "firedtv: Port it to
use rc_core" from May 28, 2012.)
-- 
Stefan Richter
-==- =-=- =--==
http://arcgraph.de/sr/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/1] doc-rst: v4l: Add documentation on CSI-2 bus configuration

[Sakari Ailus]

Document the interface between the CSI-2 transmitter and receiver drivers.

Signed-off-by: Sakari Ailus 
---
since v2:

- Add bits_per_sample variable to the formula. It should be correct now.

 Documentation/media/kapi/csi2.rst  | 61 ++
 Documentation/media/media_kapi.rst |  1 +
 2 files changed, 62 insertions(+)
 create mode 100644 Documentation/media/kapi/csi2.rst

diff --git a/Documentation/media/kapi/csi2.rst 
b/Documentation/media/kapi/csi2.rst
new file mode 100644
index 000..2004db0
--- /dev/null
+++ b/Documentation/media/kapi/csi2.rst
@@ -0,0 +1,61 @@
+MIPI CSI-2
+==
+
+CSI-2 is a data bus intended for transferring images from cameras to
+the host SoC. It is defined by the `MIPI alliance`_.
+
+.. _`MIPI alliance`: http://www.mipi.org/
+
+Transmitter drivers
+---
+
+CSI-2 transmitter, such as a sensor or a TV tuner, drivers need to
+provide the CSI-2 receiver with information on the CSI-2 bus
+configuration. These include the V4L2_CID_LINK_FREQ and
+V4L2_CID_PIXEL_RATE controls and
+(:c:type:`v4l2_subdev_video_ops`->s_stream() callback). These
+interface elements must be present on the sub-device represents the
+CSI-2 transmitter.
+
+The V4L2_CID_LINK_FREQ control is used to tell the receiver driver the
+frequency (and not the symbol rate) of the link. The
+V4L2_CID_PIXEL_RATE is may be used by the receiver to obtain the pixel
+rate the transmitter uses. The
+:c:type:`v4l2_subdev_video_ops`->s_stream() callback provides an
+ability to start and stop the stream.
+
+The value of the V4L2_CID_PIXEL_RATE is calculated as follows::
+
+   pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample
+
+where
+
+.. list-table:: variables in pixel rate calculation
+   :header-rows: 1
+
+   * - variable or constant
+ - description
+   * - link_freq
+ - The value of the V4L2_CID_LINK_FREQ integer64 menu item.
+   * - nr_of_lanes
+ - Number of data lanes used on the CSI-2 link. This can
+   be obtained from the OF endpoint configuration.
+   * - 2
+ - Two bits are transferred per clock cycle per lane.
+   * - bits_per_sample
+ - Number of bits per sample.
+
+The transmitter drivers must configure the CSI-2 transmitter to *LP-11
+mode* whenever the transmitter is powered on but not active. Some
+transmitters do this automatically but some have to be explicitly
+programmed to do so.
+
+Receiver drivers
+
+
+Before the receiver driver may enable the CSI-2 transmitter by using
+the :c:type:`v4l2_subdev_video_ops`->s_stream(), it must have powered
+the transmitter up by using the
+:c:type:`v4l2_subdev_core_ops`->s_power() callback. This may take
+place either indirectly by using :c:func:`v4l2_pipeline_pm_use` or
+directly.
diff --git a/Documentation/media/media_kapi.rst 
b/Documentation/media/media_kapi.rst
index f282ca2..bc06389 100644
--- a/Documentation/media/media_kapi.rst
+++ b/Documentation/media/media_kapi.rst
@@ -33,3 +33,4 @@ For more details see the file COPYING in the source 
distribution of Linux.
 kapi/rc-core
 kapi/mc-core
 kapi/cec-core
+kapi/csi2
-- 
2.7.4

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

Re: [PATCH 3/5] [media] winbond-cir: Move assignments for three variables in wbcir_shutdown()

[David Härdeman]

October 14, 2016 1:43 PM, "SF Markus Elfring"  
wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 10:40:12 +0200
> 
> Move the setting for the local variables "mask", "match" and "rc6_csl"
> behind the source code for a condition check by this function
> at the beginning.
 
Again, I can't see what the point is?

> Signed-off-by: Markus Elfring 
> ---
> drivers/media/rc/winbond-cir.c | 8 
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index fd997f0..9d05e17 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -702,17 +702,17 @@ wbcir_shutdown(struct pnp_dev *device)
> bool do_wake = true;
> u8 match[11];
> u8 mask[11];
> - u8 rc6_csl = 0;
> + u8 rc6_csl;
> int i;
> 
> - memset(match, 0, sizeof(match));
> - memset(mask, 0, sizeof(mask));
> -
> if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev)) {
> do_wake = false;
> goto finish;
> }
> 
> + rc6_csl = 0;
> + memset(match, 0, sizeof(match));
> + memset(mask, 0, sizeof(mask));
> switch (protocol) {
> case IR_PROTOCOL_RC5:
> if (wake_sc > 0xFFF) {
> -- 
> 2.10.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/1] doc-rst: v4l: Add documentation on CSI-2 bus configuration

[Philipp Zabel]

Am Mittwoch, den 19.10.2016, 15:59 +0300 schrieb Sakari Ailus:
> Document the interface between the CSI-2 transmitter and receiver drivers.
> 
> Signed-off-by: Sakari Ailus 
> ---
> Hi Philipp,
> 
> Indeed the pixel rate is used by some driver as well.
> 
> How about this one instead?
> 
> The HTML page is available here (without CCS unfortunately):
> 
> 
> 
> since v1:
>  
> - Add PIXEL_RATE to the required controls.
> 
> - Document how pixel rate is calculated from the link frequency.
> 
>  Documentation/media/kapi/csi2.rst  | 59 
> ++
>  Documentation/media/media_kapi.rst |  1 +
>  2 files changed, 60 insertions(+)
>  create mode 100644 Documentation/media/kapi/csi2.rst
> 
> diff --git a/Documentation/media/kapi/csi2.rst 
> b/Documentation/media/kapi/csi2.rst
> new file mode 100644
> index 000..31f927d
> --- /dev/null
> +++ b/Documentation/media/kapi/csi2.rst
> @@ -0,0 +1,59 @@
> +MIPI CSI-2
> +==
> +
> +CSI-2 is a data bus intended for transferring images from cameras to
> +the host SoC. It is defined by the `MIPI alliance`_.
> +
> +.. _`MIPI alliance`: http://www.mipi.org/
> +
> +Transmitter drivers
> +---
> +
> +CSI-2 transmitter, such as a sensor or a TV tuner, drivers need to
> +provide the CSI-2 receiver with information on the CSI-2 bus
> +configuration. These include the V4L2_CID_LINK_FREQ and
> +V4L2_CID_PIXEL_RATE controls and
> +(:c:type:`v4l2_subdev_video_ops`->s_stream() callback). These
> +interface elements must be present on the sub-device represents the
> +CSI-2 transmitter.
> +
> +The V4L2_CID_LINK_FREQ control is used to tell the receiver driver the
> +frequency (and not the symbol rate) of the link. The
> +V4L2_CID_PIXEL_RATE is may be used by the receiver to obtain the pixel
> +rate the transmitter uses. The
> +:c:type:`v4l2_subdev_video_ops`->s_stream() callback provides an
> +ability to start and stop the stream.
> +
> +The value of the V4L2_CID_PIXEL_RATE is calculated as follows::
> +
> + pixel_rate = link_freq * 2 * nr_of_lanes

This is the total bps, which must be divided by the bits per pixel
depending on the selected MEDIA_BUS_FMT, for example
/16 for MEDIA_BUS_FMT_UYVY8_1X16, or /24 for MEDIA_BUS_FMT_RGB888_1X24,
to obtain pixel_rate.

> +where
> +
> +.. list-table:: variables in pixel rate calculation
> +   :header-rows: 1
> +
> +   * - variable or constant
> + - description
> +   * - link_freq
> + - The value of the V4L2_CID_LINK_FREQ integer64 menu item.
> +   * - nr_of_lanes
> + - Number of data lanes used on the CSI-2 link. This can
> +   be obtained from the OF endpoint configuration.

I suppose the number of lanes should be calculated as
nr_of_lanes = DIV_ROUND_UP(pixel_rate * bpp, link_freq * 2)
in the receiver driver? Not all lanes configured in the device tree have
to be used, depending on the configured link frequencies and bus format.

> +   * - 2
> + - Two bits are transferred per clock cycle per lane.
> +
> +The transmitter drivers must configure the CSI-2 transmitter to *LP-11
> +mode* whenever the transmitter is powered on but not active. Some
> +transmitters do this automatically but some have to be explicitly
> +programmed to do so.
> +
> +Receiver drivers
> +
> +
> +Before the receiver driver may enable the CSI-2 transmitter by using
> +the :c:type:`v4l2_subdev_video_ops`->s_stream(), it must have powered
> +the transmitter up by using the
> +:c:type:`v4l2_subdev_core_ops`->s_power() callback. This may take
> +place either indirectly by using :c:func:`v4l2_pipeline_pm_use` or
> +directly.
> diff --git a/Documentation/media/media_kapi.rst 
> b/Documentation/media/media_kapi.rst
> index f282ca2..bc06389 100644
> --- a/Documentation/media/media_kapi.rst
> +++ b/Documentation/media/media_kapi.rst
> @@ -33,3 +33,4 @@ For more details see the file COPYING in the source 
> distribution of Linux.
>  kapi/rc-core
>  kapi/mc-core
>  kapi/cec-core
> +kapi/csi2

regards
Philipp

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

Re: [PATCH v2 1/1] doc-rst: v4l: Add documentation on CSI-2 bus configuration

[Sakari Ailus]

On 10/19/16 17:24, Philipp Zabel wrote:
> Am Mittwoch, den 19.10.2016, 15:59 +0300 schrieb Sakari Ailus:
>> Document the interface between the CSI-2 transmitter and receiver drivers.
>>
>> Signed-off-by: Sakari Ailus 
>> ---
>> Hi Philipp,
>>
>> Indeed the pixel rate is used by some driver as well.
>>
>> How about this one instead?
>>
>> The HTML page is available here (without CCS unfortunately):
>>
>> 
>>
>> since v1:
>>  
>> - Add PIXEL_RATE to the required controls.
>>
>> - Document how pixel rate is calculated from the link frequency.
>>
>>  Documentation/media/kapi/csi2.rst  | 59 
>> ++
>>  Documentation/media/media_kapi.rst |  1 +
>>  2 files changed, 60 insertions(+)
>>  create mode 100644 Documentation/media/kapi/csi2.rst
>>
>> diff --git a/Documentation/media/kapi/csi2.rst 
>> b/Documentation/media/kapi/csi2.rst
>> new file mode 100644
>> index 000..31f927d
>> --- /dev/null
>> +++ b/Documentation/media/kapi/csi2.rst
>> @@ -0,0 +1,59 @@
>> +MIPI CSI-2
>> +==
>> +
>> +CSI-2 is a data bus intended for transferring images from cameras to
>> +the host SoC. It is defined by the `MIPI alliance`_.
>> +
>> +.. _`MIPI alliance`: http://www.mipi.org/
>> +
>> +Transmitter drivers
>> +---
>> +
>> +CSI-2 transmitter, such as a sensor or a TV tuner, drivers need to
>> +provide the CSI-2 receiver with information on the CSI-2 bus
>> +configuration. These include the V4L2_CID_LINK_FREQ and
>> +V4L2_CID_PIXEL_RATE controls and
>> +(:c:type:`v4l2_subdev_video_ops`->s_stream() callback). These
>> +interface elements must be present on the sub-device represents the
>> +CSI-2 transmitter.
>> +
>> +The V4L2_CID_LINK_FREQ control is used to tell the receiver driver the
>> +frequency (and not the symbol rate) of the link. The
>> +V4L2_CID_PIXEL_RATE is may be used by the receiver to obtain the pixel
>> +rate the transmitter uses. The
>> +:c:type:`v4l2_subdev_video_ops`->s_stream() callback provides an
>> +ability to start and stop the stream.
>> +
>> +The value of the V4L2_CID_PIXEL_RATE is calculated as follows::
>> +
>> +pixel_rate = link_freq * 2 * nr_of_lanes
> 
> This is the total bps, which must be divided by the bits per pixel
> depending on the selected MEDIA_BUS_FMT, for example
> /16 for MEDIA_BUS_FMT_UYVY8_1X16, or /24 for MEDIA_BUS_FMT_RGB888_1X24,
> to obtain pixel_rate.

Uh, indeed. I'll change this.

> 
>> +where
>> +
>> +.. list-table:: variables in pixel rate calculation
>> +   :header-rows: 1
>> +
>> +   * - variable or constant
>> + - description
>> +   * - link_freq
>> + - The value of the V4L2_CID_LINK_FREQ integer64 menu item.
>> +   * - nr_of_lanes
>> + - Number of data lanes used on the CSI-2 link. This can
>> +   be obtained from the OF endpoint configuration.
> 
> I suppose the number of lanes should be calculated as
>   nr_of_lanes = DIV_ROUND_UP(pixel_rate * bpp, link_freq * 2)
> in the receiver driver? Not all lanes configured in the device tree have
> to be used, depending on the configured link frequencies and bus format.

Do we have any user for that yet?

I know there's hardware where this would be necessary in order to
support all image sizes and formats for instance, but there's no driver yet.

If we don't need to expose this to the user --- I don't think we do ---
we could use frame descriptors to do that.

> 
>> +   * - 2
>> + - Two bits are transferred per clock cycle per lane.
>> +
>> +The transmitter drivers must configure the CSI-2 transmitter to *LP-11
>> +mode* whenever the transmitter is powered on but not active. Some
>> +transmitters do this automatically but some have to be explicitly
>> +programmed to do so.
>> +
>> +Receiver drivers
>> +
>> +
>> +Before the receiver driver may enable the CSI-2 transmitter by using
>> +the :c:type:`v4l2_subdev_video_ops`->s_stream(), it must have powered
>> +the transmitter up by using the
>> +:c:type:`v4l2_subdev_core_ops`->s_power() callback. This may take
>> +place either indirectly by using :c:func:`v4l2_pipeline_pm_use` or
>> +directly.
>> diff --git a/Documentation/media/media_kapi.rst 
>> b/Documentation/media/media_kapi.rst
>> index f282ca2..bc06389 100644
>> --- a/Documentation/media/media_kapi.rst
>> +++ b/Documentation/media/media_kapi.rst
>> @@ -33,3 +33,4 @@ For more details see the file COPYING in the source 
>> distribution of Linux.
>>  kapi/rc-core
>>  kapi/mc-core
>>  kapi/cec-core
>> +kapi/csi2
> 
> regards
> Philipp
> 


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

Re: [media] winbond-cir: Move a variable assignment in wbcir_tx()

[David Härdeman]

October 19, 2016 3:32 PM, "SF Markus Elfring"  
wrote:
>>> Move the assignment for the local variable "data" behind the source code
>>> for a memory allocation by this function.
>> 
>> Sorry, I can't see what the point is?
> 
> * How do you think about to avoid a variable assignment in case
> that this memory allocation failed anyhow?

There is no memory allocation that can fail at this point.

> * Do you care for data access locality?

Not unless you can show measurable performance improvements?
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 6/7] atmel-isi: remove dependency of the soc-camera framework

[Hans Verkuil]

On 10/19/2016 09:36 AM, Wu, Songjun wrote:
> 
> 
> On 10/18/2016 18:58, Hans Verkuil wrote:
>> On 10/18/16 11:21, Wu, Songjun wrote:
>>> Hi Hans,
>>>
>>> Do you have any issue on this patch?
>>
>> ENOTIME :-(
>>
>>> Could I give you some help? :)
>>
>> I would certainly help if you can make the requested change to this patch.
>>
>> Let me know if you want to do that, because in that case I'll rebase my
>> tree
>> to the latest media_tree master.
>>
> Yes, I would like to make the requested change to this patch. :)
> It seems the patch is not based on the latest media_tree master.
> Will you rebase this patch to the latest media_tree, or let me move it 
> and make the requested change based on the media_tree?

I've rebased my branch:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=sama5d3-2

Regards,

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

Re: [PATCH v6 2/2] media: Add a driver for the ov5645 camera sensor.

[Laurent Pinchart]

Hi Todor,

(CC'ing Mark Brown for a question on regulators)

On Friday 14 Oct 2016 14:57:01 Todor Tomov wrote:
> Hi Laurent,
> 
> Thank you for the time spent to do this thorough review of the patch!
> 
> Below I have removed some of the comments where I agree and I'll fix.
> I have left the places where I have something relevant to say or ask.
> 
> On 09/08/2016 03:22 PM, Laurent Pinchart wrote:
> > On Thursday 08 Sep 2016 12:13:55 Todor Tomov wrote:
> >> The ov5645 sensor from Omnivision supports up to 2592x1944
> >> and CSI2 interface.
> >> 
> >> The driver adds support for the following modes:
> >> - 1280x960
> >> - 1920x1080
> >> - 2592x1944
> >> 
> >> Output format is packed 8bit UYVY.
> >> 
> >> Signed-off-by: Todor Tomov 
> >> ---
> >> 
> >>  drivers/media/i2c/Kconfig  |   12 +
> >>  drivers/media/i2c/Makefile |1 +
> >>  drivers/media/i2c/ov5645.c | 1372 ++
> >>  3 files changed, 1385 insertions(+)
> >>  create mode 100644 drivers/media/i2c/ov5645.c
> > 
> > [snip]
> > 
> >> diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> >> new file mode 100644
> >> index 000..5e5c37e
> >> --- /dev/null
> >> +++ b/drivers/media/i2c/ov5645.c
> >> @@ -0,0 +1,1372 @@

[snip]

> >> +  { 0x3103, 0x11 },
> >> +  { 0x3008, 0x82 },
> >> +  { 0x3008, 0x42 },
> >> +  { 0x3103, 0x03 },
> >> +  { 0x3503, 0x07 },
> > 
> > [snip]
> > 
> >> +  { 0x3503, 0x00 },
> > 
> > Can't you get rid of the first write to 0x3503 ?
> 
> No, this is a startup sequence from the vendor so I'm following it as it is.

0x3503 controls the AEC/AGC mode, I wonder if that's really needed. I'm OK 
keeping it as-is for now.

> [snip]
> 
> >> +static int ov5645_regulators_enable(struct ov5645 *ov5645)
> >> +{
> >> +  int ret;
> >> +
> >> +  ret = regulator_enable(ov5645->io_regulator);
> >> +  if (ret < 0) {
> >> +  dev_err(ov5645->dev, "set io voltage failed\n");
> >> +  return ret;
> >> +  }
> >> +
> >> +  ret = regulator_enable(ov5645->core_regulator);
> >> +  if (ret) {
> >> +  dev_err(ov5645->dev, "set core voltage failed\n");
> >> +  goto err_disable_io;
> >> +  }
> >> +
> >> +  ret = regulator_enable(ov5645->analog_regulator);
> >> +  if (ret) {
> >> +  dev_err(ov5645->dev, "set analog voltage failed\n");
> >> +  goto err_disable_core;
> >> +  }
> > 
> > How about using the regulator bulk API ? This would simplify the enable
> > and disable functions.
> 
> The driver must enable the regulators in this order. I can see in the
> implementation of the bulk api that they are enabled again in order
> but I don't see stated anywhere that it is guaranteed to follow the
> same order in future. I'd prefer to keep it explicit as it is now.

I believe it should be an API guarantee, otherwise many drivers using the bulk 
API would break. Mark, could you please comment on that ?

> [snip]
> 
> >> +static int ov5645_set_power_on(struct ov5645 *ov5645)
> >> +{
> >> +  int ret;
> >> +
> >> +  clk_set_rate(ov5645->xclk, ov5645->xclk_freq);
> > 
> > Is this needed every time you power the sensor on or could you do it just
> > once at probe time ?
> 
> I'll move it at probe time.
> 
> >> +  ret = clk_prepare_enable(ov5645->xclk);
> >> +  if (ret < 0) {
> >> +  dev_err(ov5645->dev, "clk prepare enable failed\n");
> >> +  return ret;
> >> +  }
> > 
> > Is it safe to start the clock before the regulators ? Driving an input of
> > an unpowered chip can lead to latch-up issues.
> 
> Correct, power should be enabled first. I'll fix this.
> 
> >> +  ret = ov5645_regulators_enable(ov5645);
> >> +  if (ret < 0) {
> >> +  clk_disable_unprepare(ov5645->xclk);
> >> +  return ret;
> >> +  }
> >> +
> >> +  usleep_range(5000, 15000);
> >> +  gpiod_set_value_cansleep(ov5645->enable_gpio, 1);
> >> +
> >> +  usleep_range(1000, 2000);
> >> +  gpiod_set_value_cansleep(ov5645->rst_gpio, 0);
> >> +
> >> +  msleep(20);
> >> +
> >> +  return ret;
> > 
> > You can return 0.
> 
> Ok.
> 
> [snip]
> 
> >> +static int ov5645_set_hflip(struct ov5645 *ov5645, s32 value)
> >> +{
> >> +  u8 val;
> >> +  int ret;
> >> +
> >> +  ret = ov5645_read_reg(ov5645, OV5645_TIMING_TC_REG21, );
> >> +  if (ret < 0)
> >> +  return ret;
> >> +
> >> +  if (value == 0)
> >> +  val &= ~(OV5645_SENSOR_MIRROR);
> >> +  else
> >> +  val |= (OV5645_SENSOR_MIRROR);
> >> +
> >> +  return ov5645_write_reg(ov5645, OV5645_TIMING_TC_REG21, val);
> > 
> > You could cache this register too.
> 
> Ok.
> 
> >> +}
> >> +
> >> +static int ov5645_set_vflip(struct ov5645 *ov5645, s32 value)
> >> +{
> >> +  u8 val;
> >> +  int ret;
> >> +
> >> +  ret = ov5645_read_reg(ov5645, OV5645_TIMING_TC_REG20, );
> >> +  if (ret < 0)
> >> +  return ret;
> >> +
> >> +  if (value == 0)
> >> +  val |= (OV5645_SENSOR_VFLIP | OV5645_ISP_VFLIP);
> >> +  else
> >> +  val &= ~(OV5645_SENSOR_VFLIP | OV5645_ISP_VFLIP);
> >> +
> >> +  

Re: [PATCH 08/10] mm: replace __access_remote_vm() write parameter with gup_flags

[Michal Hocko]

On Wed 19-10-16 09:40:45, Lorenzo Stoakes wrote:
> On Wed, Oct 19, 2016 at 10:13:52AM +0200, Michal Hocko wrote:
> > On Wed 19-10-16 09:59:03, Jan Kara wrote:
> > > On Thu 13-10-16 01:20:18, Lorenzo Stoakes wrote:
> > > > This patch removes the write parameter from __access_remote_vm() and 
> > > > replaces it
> > > > with a gup_flags parameter as use of this function previously _implied_
> > > > FOLL_FORCE, whereas after this patch callers explicitly pass this flag.
> > > >
> > > > We make this explicit as use of FOLL_FORCE can result in surprising 
> > > > behaviour
> > > > (and hence bugs) within the mm subsystem.
> > > >
> > > > Signed-off-by: Lorenzo Stoakes 
> > >
> > > So I'm not convinced this (and the following two patches) is actually
> > > helping much. By grepping for FOLL_FORCE we will easily see that any 
> > > caller
> > > of access_remote_vm() gets that semantics and can thus continue search
> >
> > I am really wondering. Is there anything inherent that would require
> > FOLL_FORCE for access_remote_vm? I mean FOLL_FORCE is a really
> > non-trivial thing. It doesn't obey vma permissions so we should really
> > minimize its usage. Do all of those users really need FOLL_FORCE?
> 
> I wonder about this also, for example by accessing /proc/self/mem you trigger
> access_remote_vm() and consequently get_user_pages_remote() meaning FOLL_FORCE
> is implied and you can use /proc/self/mem to override any VMA permissions. I

yes this is the desirable and expected behavior. 

> wonder if this is desirable behaviour or whether this ought to be limited to
> ptrace system calls. Regardless, by making the flag more visible it makes it
> easier to see that this is happening.

mem_open already enforces PTRACE_MODE_ATTACH

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

Re: [PATCH 06/10] mm: replace get_user_pages() write/force parameters with gup_flags

[Jan Kara]

On Thu 13-10-16 01:20:16, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_user_pages() and
> replaces them with a gup_flags parameter to make the use of FOLL_FORCE 
> explicit
> in callers as use of this flag can result in surprising behaviour (and hence
> bugs) within the mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes 

The patch looks good. You can add:

Reviewed-by: Jan Kara 

Honza

> ---
>  arch/cris/arch-v32/drivers/cryptocop.c |  4 +---
>  arch/ia64/kernel/err_inject.c  |  2 +-
>  arch/x86/mm/mpx.c  |  5 ++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  7 +--
>  drivers/gpu/drm/radeon/radeon_ttm.c|  3 ++-
>  drivers/gpu/drm/via/via_dmablit.c  |  4 ++--
>  drivers/infiniband/core/umem.c |  6 +-
>  drivers/infiniband/hw/mthca/mthca_memfree.c|  2 +-
>  drivers/infiniband/hw/qib/qib_user_pages.c |  3 ++-
>  drivers/infiniband/hw/usnic/usnic_uiom.c   |  5 -
>  drivers/media/v4l2-core/videobuf-dma-sg.c  |  7 +--
>  drivers/misc/mic/scif/scif_rma.c   |  3 +--
>  drivers/misc/sgi-gru/grufault.c|  2 +-
>  drivers/platform/goldfish/goldfish_pipe.c  |  3 ++-
>  drivers/rapidio/devices/rio_mport_cdev.c   |  3 ++-
>  .../vc04_services/interface/vchiq_arm/vchiq_2835_arm.c |  3 +--
>  .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |  3 +--
>  drivers/virt/fsl_hypervisor.c  |  4 ++--
>  include/linux/mm.h |  2 +-
>  mm/gup.c   | 12 +++-
>  mm/mempolicy.c |  2 +-
>  mm/nommu.c | 18 
> --
>  22 files changed, 49 insertions(+), 54 deletions(-)
> 
> diff --git a/arch/cris/arch-v32/drivers/cryptocop.c 
> b/arch/cris/arch-v32/drivers/cryptocop.c
> index b5698c8..099e170 100644
> --- a/arch/cris/arch-v32/drivers/cryptocop.c
> +++ b/arch/cris/arch-v32/drivers/cryptocop.c
> @@ -2722,7 +2722,6 @@ static int cryptocop_ioctl_process(struct inode *inode, 
> struct file *filp, unsig
>   err = get_user_pages((unsigned long int)(oper.indata + prev_ix),
>noinpages,
>0,  /* read access only for in data */
> -  0, /* no force */
>inpages,
>NULL);
>  
> @@ -2736,8 +2735,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
> struct file *filp, unsig
>   if (oper.do_cipher){
>   err = get_user_pages((unsigned long int)oper.cipher_outdata,
>nooutpages,
> -  1, /* write access for out data */
> -  0, /* no force */
> +  FOLL_WRITE, /* write access for out data */
>outpages,
>NULL);
>   up_read(>mm->mmap_sem);
> diff --git a/arch/ia64/kernel/err_inject.c b/arch/ia64/kernel/err_inject.c
> index 09f8457..5ed0ea9 100644
> --- a/arch/ia64/kernel/err_inject.c
> +++ b/arch/ia64/kernel/err_inject.c
> @@ -142,7 +142,7 @@ store_virtual_to_phys(struct device *dev, struct 
> device_attribute *attr,
>   u64 virt_addr=simple_strtoull(buf, NULL, 16);
>   int ret;
>  
> - ret = get_user_pages(virt_addr, 1, VM_READ, 0, NULL, NULL);
> + ret = get_user_pages(virt_addr, 1, FOLL_WRITE, NULL, NULL);
>   if (ret<=0) {
>  #ifdef ERR_INJ_DEBUG
>   printk("Virtual address %lx is not existing.\n",virt_addr);
> diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
> index 8047687..e4f8009 100644
> --- a/arch/x86/mm/mpx.c
> +++ b/arch/x86/mm/mpx.c
> @@ -544,10 +544,9 @@ static int mpx_resolve_fault(long __user *addr, int 
> write)
>  {
>   long gup_ret;
>   int nr_pages = 1;
> - int force = 0;
>  
> - gup_ret = get_user_pages((unsigned long)addr, nr_pages, write,
> - force, NULL, NULL);
> + gup_ret = get_user_pages((unsigned long)addr, nr_pages,
> + write ? FOLL_WRITE : 0, NULL, NULL);
>   /*
>* get_user_pages() returns number of pages gotten.
>* 0 means we failed to fault in and get anything,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 887483b..dcaf691 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -555,10 +555,13 @@ struct amdgpu_ttm_tt {
>  int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct page **pages)
>  {
>   

Re: [PATCH 07/10] mm: replace get_user_pages_remote() write/force parameters with gup_flags

[Jan Kara]

On Thu 13-10-16 01:20:17, Lorenzo Stoakes wrote:
> This patch removes the write and force parameters from get_user_pages_remote()
> and replaces them with a gup_flags parameter to make the use of FOLL_FORCE
> explicit in callers as use of this flag can result in surprising behaviour 
> (and
> hence bugs) within the mm subsystem.
> 
> Signed-off-by: Lorenzo Stoakes 

Looks good. You can add:

Reviewed-by: Jan Kara 

Honza

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c   |  7 +--
>  drivers/gpu/drm/i915/i915_gem_userptr.c |  6 +-
>  drivers/infiniband/core/umem_odp.c  |  7 +--
>  fs/exec.c   |  9 +++--
>  include/linux/mm.h  |  2 +-
>  kernel/events/uprobes.c |  6 --
>  mm/gup.c| 22 +++---
>  mm/memory.c |  6 +-
>  security/tomoyo/domain.c|  2 +-
>  9 files changed, 40 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c 
> b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index 5ce3603..0370b84 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -748,19 +748,22 @@ static struct page **etnaviv_gem_userptr_do_get_pages(
>   int ret = 0, pinned, npages = etnaviv_obj->base.size >> PAGE_SHIFT;
>   struct page **pvec;
>   uintptr_t ptr;
> + unsigned int flags = 0;
>  
>   pvec = drm_malloc_ab(npages, sizeof(struct page *));
>   if (!pvec)
>   return ERR_PTR(-ENOMEM);
>  
> + if (!etnaviv_obj->userptr.ro)
> + flags |= FOLL_WRITE;
> +
>   pinned = 0;
>   ptr = etnaviv_obj->userptr.ptr;
>  
>   down_read(>mmap_sem);
>   while (pinned < npages) {
>   ret = get_user_pages_remote(task, mm, ptr, npages - pinned,
> - !etnaviv_obj->userptr.ro, 0,
> - pvec + pinned, NULL);
> + flags, pvec + pinned, NULL);
>   if (ret < 0)
>   break;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c 
> b/drivers/gpu/drm/i915/i915_gem_userptr.c
> index e537930..c6f780f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> +++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> @@ -508,6 +508,10 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
> *_work)
>   pvec = drm_malloc_gfp(npages, sizeof(struct page *), GFP_TEMPORARY);
>   if (pvec != NULL) {
>   struct mm_struct *mm = obj->userptr.mm->mm;
> + unsigned int flags = 0;
> +
> + if (!obj->userptr.read_only)
> + flags |= FOLL_WRITE;
>  
>   ret = -EFAULT;
>   if (atomic_inc_not_zero(>mm_users)) {
> @@ -517,7 +521,7 @@ __i915_gem_userptr_get_pages_worker(struct work_struct 
> *_work)
>   (work->task, mm,
>obj->userptr.ptr + pinned * PAGE_SIZE,
>npages - pinned,
> -  !obj->userptr.read_only, 0,
> +  flags,
>pvec + pinned, NULL);
>   if (ret < 0)
>   break;
> diff --git a/drivers/infiniband/core/umem_odp.c 
> b/drivers/infiniband/core/umem_odp.c
> index 75077a0..1f0fe32 100644
> --- a/drivers/infiniband/core/umem_odp.c
> +++ b/drivers/infiniband/core/umem_odp.c
> @@ -527,6 +527,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 
> user_virt, u64 bcnt,
>   u64 off;
>   int j, k, ret = 0, start_idx, npages = 0;
>   u64 base_virt_addr;
> + unsigned int flags = 0;
>  
>   if (access_mask == 0)
>   return -EINVAL;
> @@ -556,6 +557,9 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 
> user_virt, u64 bcnt,
>   goto out_put_task;
>   }
>  
> + if (access_mask & ODP_WRITE_ALLOWED_BIT)
> + flags |= FOLL_WRITE;
> +
>   start_idx = (user_virt - ib_umem_start(umem)) >> PAGE_SHIFT;
>   k = start_idx;
>  
> @@ -574,8 +578,7 @@ int ib_umem_odp_map_dma_pages(struct ib_umem *umem, u64 
> user_virt, u64 bcnt,
>*/
>   npages = get_user_pages_remote(owning_process, owning_mm,
>   user_virt, gup_num_pages,
> - access_mask & ODP_WRITE_ALLOWED_BIT,
> - 0, local_page_list, NULL);
> + flags, local_page_list, NULL);
>   up_read(_mm->mmap_sem);
>  
>   if (npages < 0)
> diff --git a/fs/exec.c b/fs/exec.c
> index 6fcfb3f..4e497b9 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -191,6 +191,7 @@ static 

Re: [RFC PATCH 6/7] atmel-isi: remove dependency of the soc-camera framework

[Wu, Songjun]

On 10/18/2016 18:58, Hans Verkuil wrote:

On 10/18/16 11:21, Wu, Songjun wrote:

Hi Hans,

Do you have any issue on this patch?


ENOTIME :-(


Could I give you some help? :)


I would certainly help if you can make the requested change to this patch.

Let me know if you want to do that, because in that case I'll rebase my
tree
to the latest media_tree master.


Yes, I would like to make the requested change to this patch. :)
It seems the patch is not based on the latest media_tree master.
Will you rebase this patch to the latest media_tree, or let me move it 
and make the requested change based on the media_tree?



Regards,

Hans



On 9/23/2016 14:05, Wu, Songjun wrote:



On 9/21/2016 15:04, Hans Verkuil wrote:

On 08/18/2016 07:53 AM, Wu, Songjun wrote:

Hi Hans,

Thank you for the patch.

On 8/17/2016 14:29, Hans Verkuil wrote:

From: Hans Verkuil 

This patch converts the atmel-isi driver from a soc-camera driver to
a driver
that is stand-alone.

Signed-off-by: Hans Verkuil 
---
 drivers/media/platform/soc_camera/Kconfig |3 +-
 drivers/media/platform/soc_camera/atmel-isi.c | 1216
+++--
 2 files changed, 721 insertions(+), 498 deletions(-)

diff --git a/drivers/media/platform/soc_camera/Kconfig
b/drivers/media/platform/soc_camera/Kconfig
index 39f6641..f74e358 100644
--- a/drivers/media/platform/soc_camera/Kconfig
+++ b/drivers/media/platform/soc_camera/Kconfig
@@ -54,9 +54,8 @@ config VIDEO_SH_MOBILE_CEU

 config VIDEO_ATMEL_ISI
 tristate "ATMEL Image Sensor Interface (ISI) support"
-depends on VIDEO_DEV && SOC_CAMERA
+depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF && HAS_DMA
 depends on ARCH_AT91 || COMPILE_TEST
-depends on HAS_DMA
 select VIDEOBUF2_DMA_CONTIG
 ---help---
   This module makes the ATMEL Image Sensor Interface available
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
b/drivers/media/platform/soc_camera/atmel-isi.c
index 30211f6..9947acb 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c




+
 static int atmel_isi_probe(struct platform_device *pdev)
 {
 int irq;
 struct atmel_isi *isi;
+struct vb2_queue *q;
 struct resource *regs;
 int ret, i;
-struct soc_camera_host *soc_host;

 isi = devm_kzalloc(>dev, sizeof(struct atmel_isi),
GFP_KERNEL);
 if (!isi) {
@@ -1044,20 +1216,65 @@ static int atmel_isi_probe(struct
platform_device *pdev)
 return ret;

 isi->active = NULL;
-spin_lock_init(>lock);
+isi->dev = >dev;
+mutex_init(>lock);
+spin_lock_init(>irqlock);
 INIT_LIST_HEAD(>video_buffer_list);
 INIT_LIST_HEAD(>dma_desc_head);

+q = >queue;
+
+/* Initialize the top-level structure */
+ret = v4l2_device_register(>dev, >v4l2_dev);
+if (ret)
+return ret;
+
+isi->vdev = video_device_alloc();
+if (isi->vdev == NULL) {
+ret = -ENOMEM;
+goto err_vdev_alloc;
+}

If video device is unregistered, the ISI driver must be reloaded when
registering a new video device.
So '*vdev' can be replaced by 'vdev', or move the code above to
isi_graph_notify_complete.


I'm afraid I don't understand what you mean. Can you clarify?



If the sensor driver is defined as a module, e.g. ov7670.ko. 'insmod
ov7670.ko' and 'rmmod ov7670', video_device_release will be called, and
'*vdev' will be freed, when 'insmod ov7670.ko' is run again, the error
will will occur, if the code above is moved to
isi_graph_notify_complete, there will be no error.


Regards,

Hans




+
+/* video node */
+isi->vdev->fops = _fops;
+isi->vdev->v4l2_dev = >v4l2_dev;
+isi->vdev->queue = >queue;
+strlcpy(isi->vdev->name, KBUILD_MODNAME,
sizeof(isi->vdev->name));
+isi->vdev->release = video_device_release;
+isi->vdev->ioctl_ops = _ioctl_ops;
+isi->vdev->lock = >lock;
+isi->vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE |
V4L2_CAP_STREAMING |
+V4L2_CAP_READWRITE;
+video_set_drvdata(isi->vdev, isi);
+
+/* buffer queue */
+q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+q->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
+q->lock = >lock;
+q->drv_priv = isi;
+q->buf_struct_size = sizeof(struct frame_buffer);
+q->ops = _video_qops;
+q->mem_ops = _dma_contig_memops;
+q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+q->min_buffers_needed = 2;
+q->dev = >dev;
+
+ret = vb2_queue_init(q);
+if (ret < 0) {
+dev_err(>dev, "failed to initialize VB2 queue\n");
+goto err_vb2_queue;
+}


Regards,
Songjun Wu


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


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

Re: [RFC PATCH 6/7] atmel-isi: remove dependency of the soc-camera framework

[Wu, Songjun]

On 10/19/2016 15:46, Hans Verkuil wrote:

On 10/19/2016 09:36 AM, Wu, Songjun wrote:



On 10/18/2016 18:58, Hans Verkuil wrote:

On 10/18/16 11:21, Wu, Songjun wrote:

Hi Hans,

Do you have any issue on this patch?


ENOTIME :-(


Could I give you some help? :)


I would certainly help if you can make the requested change to this patch.

Let me know if you want to do that, because in that case I'll rebase my
tree
to the latest media_tree master.


Yes, I would like to make the requested change to this patch. :)
It seems the patch is not based on the latest media_tree master.
Will you rebase this patch to the latest media_tree, or let me move it
and make the requested change based on the media_tree?


I've rebased my branch:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=sama5d3-2


Thank you very much.
I will make the requested change based on your branch.


Regards,

Hans


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

Re: [PATCH 04/10] mm: replace get_user_pages_locked() write/force parameters with gup_flags

[Jan Kara]

On Tue 18-10-16 14:56:09, Lorenzo Stoakes wrote:
> On Tue, Oct 18, 2016 at 02:54:25PM +0200, Jan Kara wrote:
> > > @@ -1282,7 +1282,7 @@ long get_user_pages(unsigned long start, unsigned 
> > > long nr_pages,
> > >   int write, int force, struct page **pages,
> > >   struct vm_area_struct **vmas);
> > >  long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
> > > - int write, int force, struct page **pages, int *locked);
> > > + unsigned int gup_flags, struct page **pages, int *locked);
> >
> > Hum, the prototype is inconsistent with e.g. __get_user_pages_unlocked()
> > where gup_flags come after **pages argument. Actually it makes more sense
> > to have it before **pages so that input arguments come first and output
> > arguments second but I don't care that much. But it definitely should be
> > consistent...
> 
> It was difficult to decide quite how to arrange parameters as there was
> inconsitency with regards to parameter ordering already - for example
> __get_user_pages() places its flags argument before pages whereas, as you 
> note,
> __get_user_pages_unlocked() puts them afterwards.
> 
> I ended up compromising by trying to match the existing ordering of the 
> function
> as much as I could by replacing write, force pairs with gup_flags in the same
> location (with the exception of get_user_pages_unlocked() which I felt should
> match __get_user_pages_unlocked() in signature) or if there was already a
> gup_flags parameter as in the case of __get_user_pages_unlocked() I simply
> removed the write, force pair and left the flags as the last parameter.
> 
> I am happy to rearrange parameters as needed, however I am not sure if it'd be
> worthwhile for me to do so (I am keen to try to avoid adding too much noise 
> here
> :)
> 
> If we were to rearrange parameters for consistency I'd suggest adjusting
> __get_user_pages_unlocked() to put gup_flags before pages and do the same with
> get_user_pages_unlocked(), let me know what you think.

Yeah, ok. If the inconsistency is already there, just leave it for now.

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