[PATCH v2 06/11] Staging: android: remove unnecessary blank lines

[Mateusz Nowotyński]

Removes unnecessary blank lines in android/ion/ion_cma_heap.c

Signed-off-by: Mateusz Nowotyński 
---
 drivers/staging/android/ion/ion_cma_heap.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
b/drivers/staging/android/ion/ion_cma_heap.c
index a0949bc..b06f4ce 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -31,7 +31,6 @@ struct ion_cma_heap {
 
 #define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap)
 
-
 /* ION CMA heap operations functions */
 static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
unsigned long len,
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 09/11] Staging: android: fix sizeof style issue

[Mateusz Nowotyński]

Converts sizeof(type) to sizeof(variable) in android/ion/ion_system_heap.c

Signed-off-by: Mateusz Nowotyński 
---
 drivers/staging/android/ion/ion_system_heap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index 2c5bfbf..4dc5d7a 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -152,7 +152,7 @@ static int ion_system_heap_allocate(struct ion_heap *heap,
max_order = compound_order(page);
i++;
}
-   table = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
+   table = kmalloc(sizeof(*table), GFP_KERNEL);
if (!table)
goto free_pages;
 
@@ -381,7 +381,7 @@ static int ion_system_contig_heap_allocate(struct ion_heap 
*heap,
for (i = len >> PAGE_SHIFT; i < (1 << order); i++)
__free_page(page + i);
 
-   table = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
+   table = kmalloc(sizeof(*table), GFP_KERNEL);
if (!table) {
ret = -ENOMEM;
goto free_pages;
@@ -431,7 +431,7 @@ static struct ion_heap 
*__ion_system_contig_heap_create(void)
 {
struct ion_heap *heap;
 
-   heap = kzalloc(sizeof(struct ion_heap), GFP_KERNEL);
+   heap = kzalloc(sizeof(*heap), GFP_KERNEL);
if (!heap)
return ERR_PTR(-ENOMEM);
heap->ops = _ops;
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 11/11] Staging: android: use BIT macro

[Mateusz Nowotyński]

Use BIT macro instead of left shifting in android/ion/ion.h

Signed-off-by: Mateusz Nowotyński 
---
 drivers/staging/android/ion/ion.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion.h 
b/drivers/staging/android/ion/ion.h
index 4f04f65..6cc720b 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -149,7 +149,7 @@ struct ion_heap_ops {
 /**
  * heap flags - flags between the heaps and core ion code
  */
-#define ION_HEAP_FLAG_DEFER_FREE (1 << 0)
+#define ION_HEAP_FLAG_DEFER_FREE BIT(0)
 
 /**
  * private flags - flags internal to ion
@@ -160,7 +160,7 @@ struct ion_heap_ops {
  * any buffer storage that came from the system allocator will be
  * returned to the system allocator.
  */
-#define ION_PRIV_FLAG_SHRINKER_FREE (1 << 0)
+#define ION_PRIV_FLAG_SHRINKER_FREE BIT(0)
 
 /**
  * struct ion_heap - represents a heap in the system
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 08/11] Staging: android: Remove unnecessary blank lines

[Mateusz Nowotyński]

Removes unnecessary blank lines in android/ion/ion_system_heap.c

Signed-off-by: Mateusz Nowotyński 
---
 drivers/staging/android/ion/ion_system_heap.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion_system_heap.c 
b/drivers/staging/android/ion/ion_system_heap.c
index c50f2d9..2c5bfbf 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -98,7 +98,6 @@ static void free_buffer_page(struct ion_system_heap *heap,
ion_page_pool_free(pool, page);
 }
 
-
 static struct page *alloc_largest_available(struct ion_system_heap *heap,
struct ion_buffer *buffer,
unsigned long size,
@@ -256,7 +255,6 @@ static struct ion_heap_ops system_heap_ops = {
 static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file 
*s,
  void *unused)
 {
-
struct ion_system_heap *sys_heap = container_of(heap,
struct ion_system_heap,
heap);
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 07/11] Staging: android: fix sizeof style issue

[Mateusz Nowotyński]

Converts sizeof(type) to sizeof(variable) in android/ion/ion_cma_heap.c

Signed-off-by: Mateusz Nowotyński 
---
 drivers/staging/android/ion/ion_cma_heap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_cma_heap.c 
b/drivers/staging/android/ion/ion_cma_heap.c
index b06f4ce..bb2c144 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -45,7 +45,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct 
ion_buffer *buffer,
if (!pages)
return -ENOMEM;
 
-   table = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
+   table = kmalloc(sizeof(*table), GFP_KERNEL);
if (!table)
goto err;
 
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 10/11] Staging: android: Fix code alignment issue

[Mateusz Nowotyński]

Fixes code alignment issue in ion/ion.h

Signed-off-by: Mateusz Nowotyński 
---
 drivers/staging/android/ion/ion.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion.h 
b/drivers/staging/android/ion/ion.h
index ace8416..4f04f65 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -240,8 +240,8 @@ int ion_heap_buffer_zero(struct ion_buffer *buffer);
 int ion_heap_pages_zero(struct page *page, size_t size, pgprot_t pgprot);
 
 int ion_alloc(size_t len,
-   unsigned int heap_id_mask,
-   unsigned int flags);
+ unsigned int heap_id_mask,
+ unsigned int flags);
 
 /**
  * ion_heap_init_shrinker
@@ -305,7 +305,7 @@ size_t ion_heap_freelist_drain(struct ion_heap *heap, 
size_t size);
  * flag.
  */
 size_t ion_heap_freelist_shrink(struct ion_heap *heap,
-   size_t size);
+   size_t size);
 
 /**
  * ion_heap_freelist_size - returns the size of the freelist in bytes
@@ -366,7 +366,7 @@ void ion_page_pool_free(struct ion_page_pool *pool, struct 
page *page);
  * returns the number of items freed in pages
  */
 int ion_page_pool_shrink(struct ion_page_pool *pool, gfp_t gfp_mask,
- int nr_to_scan);
+int nr_to_scan);
 
 long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
 
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 05/11] Staging: android: Fix code alignment issue

[Mateusz Nowotyński]

Fixes code alignment issue in ion/ion_carveout_heap.c

Signed-off-by: Mateusz Nowotyński 
---
 drivers/staging/android/ion/ion_carveout_heap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ion/ion_carveout_heap.c 
b/drivers/staging/android/ion/ion_carveout_heap.c
index 5fdc1f3..fee7650 100644
--- a/drivers/staging/android/ion/ion_carveout_heap.c
+++ b/drivers/staging/android/ion/ion_carveout_heap.c
@@ -33,7 +33,7 @@ struct ion_carveout_heap {
 };
 
 static phys_addr_t ion_carveout_allocate(struct ion_heap *heap,
-unsigned long size)
+unsigned long size)
 {
struct ion_carveout_heap *carveout_heap =
container_of(heap, struct ion_carveout_heap, heap);
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 01/11] Staging: android: Fix style issue in ion-ioctl.c

[Mateusz Nowotyński]

This is patch fixing code alignment style issue in
android/ion/ion-ioctl.c file

Signed-off-by: Mateusz Nowotyński 
---
 drivers/staging/android/ion/ion-ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion-ioctl.c 
b/drivers/staging/android/ion/ion-ioctl.c
index 76427e4..d9f8b14 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -83,8 +83,8 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned 
long arg)
int fd;
 
fd = ion_alloc(data.allocation.len,
-   data.allocation.heap_id_mask,
-   data.allocation.flags);
+  data.allocation.heap_id_mask,
+  data.allocation.flags);
if (fd < 0)
return fd;
 
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 02/11] Staging: android: NULL comparasion in ion.c file

[Mateusz Nowotyński]

Fixes NULL comparison coding style issues in
android/ion/ion.c file

Signed-off-by: Mateusz Nowotyński 
---
 drivers/staging/android/ion/ion.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 03d3a4f..73f90e6 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -103,7 +103,7 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap 
*heap,
goto err2;
}
 
-   if (buffer->sg_table == NULL) {
+   if (!buffer->sg_table) {
WARN_ONCE(1, "This heap needs to set the sgtable");
ret = -EINVAL;
goto err1;
@@ -163,7 +163,7 @@ static void *ion_buffer_kmap_get(struct ion_buffer *buffer)
return buffer->vaddr;
}
vaddr = buffer->heap->ops->map_kernel(buffer->heap, buffer);
-   if (WARN_ONCE(vaddr == NULL,
+   if (WARN_ONCE(!vaddr,
  "heap->ops->map_kernel should return ERR_PTR on error"))
return ERR_PTR(-EINVAL);
if (IS_ERR(vaddr))
@@ -435,7 +435,7 @@ int ion_alloc(size_t len, unsigned int heap_id_mask, 
unsigned int flags)
}
up_read(>lock);
 
-   if (buffer == NULL)
+   if (!buffer)
return -ENODEV;
 
if (IS_ERR(buffer))
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 03/11] Staging: android: Fix alignment issue in ion.c

[Mateusz Nowotyński]

Fixes code alignment style issues in android/ion/ion.c

Signed-off-by: Mateusz Nowotyński 
---
 drivers/staging/android/ion/ion.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 73f90e6..e10c696 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -221,7 +221,7 @@ struct ion_dma_buf_attachment {
 };
 
 static int ion_dma_buf_attach(struct dma_buf *dmabuf, struct device *dev,
-   struct dma_buf_attachment *attachment)
+ struct dma_buf_attachment *attachment)
 {
struct ion_dma_buf_attachment *a;
struct sg_table *table;
@@ -358,7 +358,7 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf 
*dmabuf,
mutex_lock(>lock);
list_for_each_entry(a, >attachments, list) {
dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
-   DMA_BIDIRECTIONAL);
+   DMA_BIDIRECTIONAL);
}
mutex_unlock(>lock);
 
@@ -380,7 +380,7 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf 
*dmabuf,
mutex_lock(>lock);
list_for_each_entry(a, >attachments, list) {
dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents,
-   DMA_BIDIRECTIONAL);
+  DMA_BIDIRECTIONAL);
}
mutex_unlock(>lock);
 
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 04/11] Staging: android: remove unnecessary blank lines

[Mateusz Nowotyński]

Removes unnecessary blank lines in android/ion/ion.c

Signed-off-by: Mateusz Nowotyński 
---
 drivers/staging/android/ion/ion.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index e10c696..11acbe2 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -264,7 +264,6 @@ static void ion_dma_buf_detatch(struct dma_buf *dmabuf,
kfree(a);
 }
 
-
 static struct sg_table *ion_map_dma_buf(struct dma_buf_attachment *attachment,
enum dma_data_direction direction)
 {
@@ -354,7 +353,6 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf 
*dmabuf,
mutex_unlock(>lock);
}
 
-
mutex_lock(>lock);
list_for_each_entry(a, >attachments, list) {
dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
-- 
2.10.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 16/34] [media] add Omnivision OV5640 sensor driver

[Steve Longerbeam]

Hi Sakari,


On 05/29/2017 08:55 AM, Sakari Ailus wrote:

Hi Steve,

A few comments below.

On Wed, May 24, 2017 at 05:29:31PM -0700, Steve Longerbeam wrote:

This driver is based on ov5640_mipi.c from Freescale imx_3.10.17_1.0.0_beta
branch, modified heavily to bring forward to latest interfaces and code
cleanup.

Signed-off-by: Steve Longerbeam
---
  drivers/media/i2c/Kconfig  |9 +
  drivers/media/i2c/Makefile |1 +
  drivers/media/i2c/ov5640.c | 2224 
  3 files changed, 2234 insertions(+)
  create mode 100644 drivers/media/i2c/ov5640.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index fd181c9..ff082a7 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -539,6 +539,15 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
  
+config VIDEO_OV5640

+   tristate "OmniVision OV5640 sensor support"
+   depends on OF
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the Omnivision
+ OV5640 camera sensor with a MIPI CSI-2 interface.
+
  config VIDEO_OV5645
tristate "OmniVision OV5645 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 62323ec..dc6b0c4 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
  obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
  obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
  obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
  obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
  obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
new file mode 100644
index 000..2a032bc
--- /dev/null
+++ b/drivers/media/i2c/ov5640.c
@@ -0,0 +1,2224 @@
+/*
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 

Could you rebase this on the V4L2 fwnode patchset here, please?




Once the fwnode patchset hits mediatree, then yes it can be
converted along with all the others under media/i2c.





+
+static int ov5640_write_reg16(struct ov5640_dev *sensor, u16 reg, u16 val)
+{
+   int ret;
+
+   ret = ov5640_write_reg(sensor, reg, val >> 8);
+   if (ret)
+   return ret;
+
+   return ov5640_write_reg(sensor, reg + 1, val & 0xff);

Does the sensor datasheet suggest doing this?


Why would the datasheet suggest or not suggest such things?
Coding details like this don't belong in the datasheet.


  Making the write in two
transactions will make it non-atomic that could be an issue in some corner
cases.


It's called everywhere under the same device mutex.





+
+static int ov5640_set_gain(struct ov5640_dev *sensor, int auto_gain)
+{
+   struct ov5640_ctrls *ctrls = >ctrls;
+
+   if (ctrls->auto_gain->is_new) {
+   ov5640_mod_reg(sensor, OV5640_REG_AEC_PK_MANUAL,
+  BIT(1), ctrls->auto_gain->val ? 0 : BIT(1));

You're generally silently ignoring all I²C access errors. Is that
intentional?


Yeah, this driver is much cleaned up from the original, but there are
still some issues like this. The register access errors are really only
being paid attention to during s_power() when loading the initial
register set, which is enough at least to catch a non-existent chip
or basic i2c bus or other hardware issues. But I should work on
catching all access errors. This is something I did in an earlier rev
but I used a questionable short-cut to make it easier to implement.
I'll just have to catch every case one by one.






+
+static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
+   struct ov5640_dev *sensor = to_ov5640_dev(sd);
+   int ret = 0;
+
+   mutex_lock(>lock);

Could you use the same lock for the controls as you use for the rest? Just
setting handler->lock after handler init does the trick.


Can you please rephrase, I don't follow. "same lock for the controls as
you use for the rest" - there's only one device lock owned by this driver
and I am already using that same 

Re: [PATCH 01/12] staging: ccree: correct coding style violations

[Joe Perches]

On Mon, 2017-05-29 at 20:11 +0300, Gilad Ben-Yossef wrote:
> On Mon, May 29, 2017 at 7:57 PM, Joe Perches  wrote:
> > On Mon, 2017-05-29 at 16:37 +0200, Greg Kroah-Hartman wrote:
> > > On Sun, May 28, 2017 at 05:40:26PM +0300, Gilad Ben-Yossef wrote:
> > > > cc_crypto_ctx.h had multiple coding style violations reported by
> > > > checkpatch. Fix them all.
> > > 
> > > Sorry, no.  You need to do only one-thing-per-patch, and "fix all coding
> > > style issues is not "one thing".  I wouldn't take this kind of patch
> > > from anyone else, so why should I take it from you?  :)
> > 
> > Because he's the named MAINTAINER of the subsystem
> > and you are acting as an upstream conduit.
> > 
> 
> LOL. Thank you Joe, but I have opted to upstream via staging to get the 
> guidance
> and review of Greg and other developers kind enough to offer it, and I'd be a
> fool not to listen to them.

For reviews of technical merit, true.

For reviews of commit log wording of whitespace
changes, where git diff -w shows no difference,
less true.

This patch seems almost entirely whitespace except
one bit reformatting a comment block.

Breaking those down into changes like:
added space after commas
added spaces around bit shifts
added spaces around arithmetic
is simply excessive.

The only comment I would have given would be to
change the patch context that added line comment
initiators to use the /** kernel-doc style.

And maybe a style change of

#define CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE (CC_MULTI2_SYSTEM_KEY_SIZE + \
  CC_MULTI2_DATA_KEY_SIZE)

to 

#define CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE \
(CC_MULTI2_SYSTEM_KEY_SIZE + CC_MULTI2_DATA_KEY_SIZE)

as it's sometimes easier to scan arithmetic on a single line.

btw: this #define seems misleading as it's used in both .min_keysize
and .max_keysize with "+ 1" and key[CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE]
is also used.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] Staging: gdm724x: Change spaces to tabs

[Mart Lubbers]

This patch fixes the following checkpatch.pl warning in gdm_usb.c:
WARNING: suspect code indent for conditional statements (8, 12)

Signed-off-by: Mart Lubbers 
---
 drivers/staging/gdm724x/gdm_usb.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/gdm724x/gdm_usb.c 
b/drivers/staging/gdm724x/gdm_usb.c
index 15a7e81ec2d2..87cd1f827455 100644
--- a/drivers/staging/gdm724x/gdm_usb.c
+++ b/drivers/staging/gdm724x/gdm_usb.c
@@ -738,11 +738,11 @@ static int gdm_usb_sdu_send(void *priv_dev, void *data, 
int len,
sdu->cmd_evt = gdm_cpu_to_dev16(>gdm_ed, LTE_TX_SDU);
if (nic_type == NIC_TYPE_ARP) {
send_len = len + SDU_PARAM_LEN;
-   memcpy(sdu->data, data, len);
+   memcpy(sdu->data, data, len);
} else {
-   send_len = len - ETH_HLEN;
-   send_len += SDU_PARAM_LEN;
-   memcpy(sdu->data, data + ETH_HLEN, len - ETH_HLEN);
+   send_len = len - ETH_HLEN;
+   send_len += SDU_PARAM_LEN;
+   memcpy(sdu->data, data + ETH_HLEN, len - ETH_HLEN);
}
 
sdu->len = gdm_cpu_to_dev16(>gdm_ed, send_len);
-- 
2.11.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 00/34] i.MX Media Driver

[Steve Longerbeam]

Hi Sakari,


On 05/29/2017 08:36 AM, Sakari Ailus wrote:

Hi Hans,

On Mon, May 29, 2017 at 03:46:08PM +0200, Hans Verkuil wrote:

Hi Steve,

On 05/25/2017 02:29 AM, Steve Longerbeam wrote:

In version 7:

- video-mux: switched to Philipp's latest video-mux driver and updated
   bindings docs, that makes use of the mmio-mux framework.

- mmio-mux: includes Philipp's temporary patch that adds mmio-mux support
   to video-mux driver, until mux framework is merged.

- mmio-mux: updates to device tree from Philipp that define the i.MX6 mux
   devices and modifies the video-mux device to become a consumer of the
   video mmio-mux.

- minor updates to Documentation/media/v4l-drivers/imx.rst.

- ov5640: do nothing if entity stream count is greater than 1 in
   ov5640_s_stream().

- Previous versions of this driver had not tested the ability to enable
   multiple independent streams, for instance enabling multiple output
   pads from the imx6-mipi-csi2 subdevice, or enabling both prpenc and
   prpvf outputs. Marek Vasut tested this support and reported issues
   with it.

   v4l2_pipeline_inherit_controls() used the media graph walk APIs, but
   that walks both sink and source pads, so if there are multiple paths
   enabled to video capture devices, controls would be added to the wrong
   video capture device, and no controls added to the other enabled
   capture devices.

   These issues have been fixed. Control inheritance works correctly now
   even with multiple enabled capture paths, and (for example)
   simultaneous capture from prpenc and prpvf works also, and each with
   independent scaling, CSC, and controls. For example prpenc can be
   capturing with a 90 degree rotation, while prpvf is capturing with
   vertical flip.

   So the v4l2_pipeline_inherit_controls() patch has been dropped. The
   new version of control inheritance could be made generically available,
   but it would be more involved to incorporate it into v4l2-core.

- A new function imx_media_fill_default_mbus_fields() is added to setup
   colorimetry at sink pads, and these are propagated to source pads.

- Ensure that the current sink and source rectangles meet alignment
   restrictions before applying a new rotation control setting in
   prp-enc/vf subdevices.

- Chain the s_stream() subdev calls instead of implementing a custom
   stream on/off function that attempts to call a fixed set of subdevices
   in a pipeline in the correct order. This also simplifies imx6-mipi-csi2
   subdevice, since the correct MIPI CSI-2 startup sequence can be
   enforced completely in s_stream(), and s_power() is no longer
   required. This also paves the way for more arbitrary OF graphs
   external to the i.MX6.

- Converted the v4l2_subdev and media_entity ops structures to const.

What is the status as of v7?

 From what I can tell patch 2/34 needs an Ack from Rob Herring, patches
4-14 are out of scope for the media subsystem, patches 20-25 and 27-34
are all staging (so fine to be merged from my point of view).

I'm not sure if patch 26 (defconfig) should be applied while the imx
driver is in staging. I would suggest that this patch is moved to the end
of the series.

That leaves patches 15-19. I replied to patch 15 with a comment, patches
16-18 look good to me, although patches 17 and 18 should be combined to one
patch since patch 17 won't compile otherwise. Any idea when the multiplexer is
expected to be merged? (just curious)

I would really like to get this merged for 4.13, so did I miss anything?
 From what I can tell it is really just an Ack for patch 2/34.

The patchset re-initialises the control handler in the video node in its
media device link_setup callback, among other things. I don't think that
this as such should prevent including the driver in staging, but at the same
time my concern is what kind of guarantees can we give in terms of V4L2
framework support on things like this using the current API. I'd say "none".


Re-initializing a control handler in itself works fine (maybe we should
add a v4l2_ctrl_handler_reinit() ?).

I agree that calling free/init/add at this time (in link_setup and 
link_notify)

is a new way to use those APIs. The effect is that a video node's controls
can change post driver load.

There's nothing in Documentation/kapi/v4l2-controls.rst that states that
a video node's control list can't change at runtime. But as I said 
free/init/add

works just fine and can be called any time after a subdevice or video device
is registered.



If there's a need for this (there should not be, as the controls are exposed
to the user space through the sub-device nodes as the other drivers do), the
framework APIs need to be extended.


Right, this gets back to the media framework usability arguments. At least
myself, Philipp, and Russell feel that automatic inheritance of a configured
pipeline's controls to a video device adds to the usability.



I think it'd be good to add a TODO file (such as the one in OMAP4ISS) as
part of 

Re: [PATCH 01/12] staging: ccree: correct coding style violations

[Gilad Ben-Yossef]

On Mon, May 29, 2017 at 8:36 PM, Joe Perches  wrote:
> On Mon, 2017-05-29 at 20:11 +0300, Gilad Ben-Yossef wrote:
>> On Mon, May 29, 2017 at 7:57 PM, Joe Perches  wrote:
>> > On Mon, 2017-05-29 at 16:37 +0200, Greg Kroah-Hartman wrote:
>> > > On Sun, May 28, 2017 at 05:40:26PM +0300, Gilad Ben-Yossef wrote:
>> > > > cc_crypto_ctx.h had multiple coding style violations reported by
>> > > > checkpatch. Fix them all.
>> > >
>> > > Sorry, no.  You need to do only one-thing-per-patch, and "fix all coding
>> > > style issues is not "one thing".  I wouldn't take this kind of patch
>> > > from anyone else, so why should I take it from you?  :)
>> >
>> > Because he's the named MAINTAINER of the subsystem
>> > and you are acting as an upstream conduit.
>> >
>>
>> LOL. Thank you Joe, but I have opted to upstream via staging to get the 
>> guidance
>> and review of Greg and other developers kind enough to offer it, and I'd be a
>> fool not to listen to them.
>
> For reviews of technical merit, true.
>
> For reviews of commit log wording of whitespace
> changes, where git diff -w shows no difference,
> less true.
>
> This patch seems almost entirely whitespace except
> one bit reformatting a comment block.
>
> Breaking those down into changes like:
> added space after commas
> added spaces around bit shifts
> added spaces around arithmetic
> is simply excessive.

I'll admit that this was my line of reasoning as well for including it
in a single patch but I if Greg finds it easier to review broken
down I'm fine with that. Something tells me he sees a lot of patches... :-)

> The only comment I would have given would be to
> change the patch context that added line comment
> initiators to use the /** kernel-doc style.
>
> And maybe a style change of
>
> #define CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE (CC_MULTI2_SYSTEM_KEY_SIZE + \
>   CC_MULTI2_DATA_KEY_SIZE)
>
> to
>
> #define CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE \
> (CC_MULTI2_SYSTEM_KEY_SIZE + CC_MULTI2_DATA_KEY_SIZE)
>
> as it's sometimes easier to scan arithmetic on a single line.
>

Thanks for the feedback. I will include it in the revised series.

> btw: this #define seems misleading as it's used in both .min_keysize
> and .max_keysize with "+ 1" and key[CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE]
> is also used.
>

 I'll look into that. There are still areas in this driver I've
inherited that I find... intriguing :-)

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 04/12] staging: ccree: cleanup lli access macro

[Gilad Ben-Yossef]

On Mon, May 29, 2017 at 5:41 PM, Greg Kroah-Hartman
 wrote:
> On Sun, May 28, 2017 at 05:40:29PM +0300, Gilad Ben-Yossef wrote:
>> The Linked List Item descriptors were being accessed via
>> a baroque set of defines and macro. Re-factor for structs
>> and inline function for readability and sanity.
>>
>> Signed-off-by: Gilad Ben-Yossef 
>> ---
>>  drivers/staging/ccree/cc_lli_defs.h| 65 
>> +++---
>>  drivers/staging/ccree/ssi_buffer_mgr.c | 45 +--
>>  2 files changed, 44 insertions(+), 66 deletions(-)
>>
>> diff --git a/drivers/staging/ccree/cc_lli_defs.h 
>> b/drivers/staging/ccree/cc_lli_defs.h
>> index 857b94f..c6b2917 100644
>> --- a/drivers/staging/ccree/cc_lli_defs.h
>> +++ b/drivers/staging/ccree/cc_lli_defs.h
>> @@ -28,36 +28,43 @@
>>
>>  #define CC_MAX_MLLI_ENTRY_SIZE 0x1
>>
>> -#define LLI_SET_ADDR(__lli_p, __addr) do {   \
>> - u32 *lli_p = (u32 *)__lli_p;\
>> - typeof(__addr) addr = __addr;   \
>> - \
>> - BITFIELD_SET(lli_p[LLI_WORD0_OFFSET],   \
>> - LLI_LADDR_BIT_OFFSET,   \
>> - LLI_LADDR_BIT_SIZE, (addr & U32_MAX));  \
>> - \
>> - BITFIELD_SET(lli_p[LLI_WORD1_OFFSET],   \
>> - LLI_HADDR_BIT_OFFSET,   \
>> - LLI_HADDR_BIT_SIZE, MSB64(addr));   \
>> - } while (0)
>> -
>> -#define LLI_SET_SIZE(lli_p, size)\
>> - BITFIELD_SET(((u32 *)(lli_p))[LLI_WORD1_OFFSET],\
>> - LLI_SIZE_BIT_OFFSET, LLI_SIZE_BIT_SIZE, size)
>> +#define LLI_MAX_NUM_OF_DATA_ENTRIES 128
>> +#define LLI_MAX_NUM_OF_ASSOC_DATA_ENTRIES 4
>> +#define MLLI_TABLE_MIN_ALIGNMENT 4 /* 32 bit alignment */
>> +#define MAX_NUM_OF_BUFFERS_IN_MLLI 4
>> +#define MAX_NUM_OF_TOTAL_MLLI_ENTRIES (2 * LLI_MAX_NUM_OF_DATA_ENTRIES + \
>> +LLI_MAX_NUM_OF_ASSOC_DATA_ENTRIES)
>> +
>> +struct cc_lli_entry {
>> +#ifndef __LITTLE_ENDIAN__
>> + u32 addr_lsb;
>> + u16 size;
>> + u16 addr_msb;
>> +#else /* __BIG_ENDIAN__ */
>> + u16 addr_msb;
>> + u16 size;
>> + u32 addr_lsb;
>> +#endif
>
> How is the bits within the different variables also not affected by the
> endian issues?  Just moving them around in the 64bits seems really
> strange.

I actually did not overlook this issue. My reasoning was that the new code
is doing exactly what the old code was doing. Either the callers was doing
the right thing, or the HW is magically taking care of it[1] or the
new code is as
broken as the old in a bug to bug compatibility sort of way.

And if it's broken, fixing it in this patch would actually break the
"do one thing"
rule... :-)

[1] I know this sounds strange but the control register description has language
that sounds like it does automagicall swabbing under certain circumstances I am
yet to understand and since I don't have a big endian test system at
my disposable
I chose to leave it as is for now.

>
> Why not just use the "normal" ways to address data in an endian-neutral
> way instead of making your own macros up here?  (i.e. shift and mask.)
>

I personally find it more readable it this way and there are other
example in the
kernel source of doing this) but I will gladly switch if you think it
is preferred.

Thanks,
Gilad

> thanks,
>
> greg k-h



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 00/34] i.MX Media Driver

[Hans Verkuil]

On 05/29/2017 07:23 PM, Steve Longerbeam wrote:

Hi Hans, thanks for the reply...


On 05/29/2017 06:46 AM, Hans Verkuil wrote:

Hi Steve,

On 05/25/2017 02:29 AM, Steve Longerbeam wrote:

In version 7:




What is the status as of v7?

 From what I can tell patch 2/34 needs an Ack from Rob Herring,



Yes still missing that Ack. I think the issue is likely the Synopsys DW
mipi csi-2 bindings. Someone earlier noted that there is another driver
under devel for this Synopsys core, with another set of bindings.
But it was determined that in fact this is a different device with a
different register set.

  From what I remember of dealing with Synopsys cores in the past,
these cores are highly configurable using their coreBuilder tools. So
while the other device might stem from the same initial core from
Synopsys, it was probably built with different design parameters
compared to the core that exists in the i.MX6. So in essence it is a
different device.



patches
4-14 are out of scope for the media subsystem,


Ok. I did submit patches 4-14 to the right set of folks. Should I just
drop this set in the next submission if they have not changed?


No, please keep them. Just make a note in the cover letter that they go
through a different tree. I like seeing the whole set :-)


patches 20-25 and 27-34
are all staging (so fine to be merged from my point of view).

I'm not sure if patch 26 (defconfig) should be applied while the imx
driver is in staging. I would suggest that this patch is moved to the end
of the series.


Ok.



That leaves patches 15-19. I replied to patch 15 with a comment, patches
16-18 look good to me, although patches 17 and 18 should be combined
to one
patch since patch 17 won't compile otherwise. Any idea when the
multiplexer is
expected to be merged? (just curious)


Philipp replied separately.



I would really like to get this merged for 4.13, so did I miss anything?
 From what I can tell it is really just an Ack for patch 2/34.


Agreed.


Can you split off the TODO file in its own patch? It was buried in the
big patch and I missed it because of that.

Take a look at Sakari's comment from today about another TODO item that
probably should be in the TODO file.

Regards,

Hans

Regards,

Hans
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 00/34] i.MX Media Driver

[Steve Longerbeam]

Hi Hans, thanks for the reply...


On 05/29/2017 06:46 AM, Hans Verkuil wrote:

Hi Steve,

On 05/25/2017 02:29 AM, Steve Longerbeam wrote:

In version 7:




What is the status as of v7?

From what I can tell patch 2/34 needs an Ack from Rob Herring,



Yes still missing that Ack. I think the issue is likely the Synopsys DW
mipi csi-2 bindings. Someone earlier noted that there is another driver
under devel for this Synopsys core, with another set of bindings.
But it was determined that in fact this is a different device with a
different register set.

From what I remember of dealing with Synopsys cores in the past,
these cores are highly configurable using their coreBuilder tools. So
while the other device might stem from the same initial core from
Synopsys, it was probably built with different design parameters
compared to the core that exists in the i.MX6. So in essence it is a
different device.



patches
4-14 are out of scope for the media subsystem,


Ok. I did submit patches 4-14 to the right set of folks. Should I just
drop this set in the next submission if they have not changed?


patches 20-25 and 27-34
are all staging (so fine to be merged from my point of view).

I'm not sure if patch 26 (defconfig) should be applied while the imx
driver is in staging. I would suggest that this patch is moved to the end
of the series.


Ok.



That leaves patches 15-19. I replied to patch 15 with a comment, patches
16-18 look good to me, although patches 17 and 18 should be combined 
to one
patch since patch 17 won't compile otherwise. Any idea when the 
multiplexer is

expected to be merged? (just curious)


Philipp replied separately.



I would really like to get this merged for 4.13, so did I miss anything?
From what I can tell it is really just an Ack for patch 2/34.


Agreed.

Steve








Marek Vasut (1):
   media: imx: Drop warning upon multiple S_STREAM disable calls

Philipp Zabel (9):
   dt-bindings: Add bindings for video-multiplexer device
   ARM: dts: imx6qdl: add multiplexer controls
   ARM: dts: imx6qdl: Add video multiplexers, mipi_csi, and their
 connections
   add mux and video interface bridge entity functions
   platform: add video-multiplexer subdevice driver
   platform: video-mux: include temporary mmio-mux support
   media: imx: csi: increase burst size for YUV formats
   media: imx: csi: add frame skipping support
   media: imx: csi: add sink selection rectangles

Russell King (3):
   media: imx: csi: add support for bayer formats
   media: imx: csi: add frame size/interval enumeration
   media: imx: capture: add frame sizes/interval enumeration

Steve Longerbeam (21):
   [media] dt-bindings: Add bindings for i.MX media driver
   [media] dt/bindings: Add bindings for OV5640
   ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node
   ARM: dts: imx6qdl: add capture-subsystem device
   ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround
   ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors
   ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
   ARM: dts: imx6-sabreauto: create i2cmux for i2c3
   ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b
   ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture
   ARM: dts: imx6-sabreauto: add the ADV7180 video decoder
   [media] add Omnivision OV5640 sensor driver
   media: Add userspace header file for i.MX
   media: Add i.MX media core driver
   media: imx: Add Capture Device Interface
   media: imx: Add CSI subdev driver
   media: imx: Add VDIC subdev driver
   media: imx: Add IC subdev drivers
   media: imx: Add MIPI CSI-2 Receiver subdev driver
   ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers
   media: imx: set and propagate default field, colorimetry

  .../devicetree/bindings/media/i2c/ov5640.txt   |   45 +
  Documentation/devicetree/bindings/media/imx.txt|   74 +
  .../devicetree/bindings/media/video-mux.txt|   60 +
  Documentation/media/uapi/mediactl/media-types.rst  |   22 +
  Documentation/media/v4l-drivers/imx.rst|  590 ++
  arch/arm/boot/dts/imx6dl-sabrelite.dts |5 +
  arch/arm/boot/dts/imx6dl-sabresd.dts   |5 +
  arch/arm/boot/dts/imx6dl.dtsi  |  189 ++
  arch/arm/boot/dts/imx6q-sabrelite.dts  |5 +
  arch/arm/boot/dts/imx6q-sabresd.dts|5 +
  arch/arm/boot/dts/imx6q.dtsi   |  125 ++
  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi   |  144 +-
  arch/arm/boot/dts/imx6qdl-sabrelite.dtsi   |  152 +-
  arch/arm/boot/dts/imx6qdl-sabresd.dtsi |  114 +-
  arch/arm/boot/dts/imx6qdl.dtsi |   20 +-
  arch/arm/configs/imx_v6_v7_defconfig   |   11 +
  drivers/media/i2c/Kconfig  |9 +
  drivers/media/i2c/Makefile |1 +
  drivers/media/i2c/ov5640.c | 2224 


  

Re: [PATCH 02/12] staging: ccree: move to kernel bitfields/bitops

[Gilad Ben-Yossef]

On Mon, May 29, 2017 at 5:38 PM, Greg Kroah-Hartman
 wrote:
> On Sun, May 28, 2017 at 05:40:27PM +0300, Gilad Ben-Yossef wrote:
>> ccree had a lot of boilerplate code for dealing with bitops
>> and bitfield register access. Move it over to the generic kernel
>> infrastructure used for doing the same.
>>
>> Signed-off-by: Gilad Ben-Yossef 
>> ---
>>  drivers/staging/ccree/cc_bitops.h|  11 +-
>>  drivers/staging/ccree/cc_hw_queue_defs.h | 679 ---
>>  drivers/staging/ccree/ssi_aead.c | 901 
>> +++
>>  drivers/staging/ccree/ssi_cipher.c   | 224 
>>  drivers/staging/ccree/ssi_driver.h   |   4 +-
>>  drivers/staging/ccree/ssi_fips_ll.c  | 704 
>>  drivers/staging/ccree/ssi_hash.c | 832 ++--
>>  drivers/staging/ccree/ssi_ivgen.c|  77 ++-
>>  drivers/staging/ccree/ssi_request_mgr.c  |  25 +-
>>  drivers/staging/ccree/ssi_sram_mgr.c |   8 +-
>>  10 files changed, 1765 insertions(+), 1700 deletions(-)
>>
>> diff --git a/drivers/staging/ccree/cc_bitops.h 
>> b/drivers/staging/ccree/cc_bitops.h
>> index ee5238a..a12a65c 100644
>> --- a/drivers/staging/ccree/cc_bitops.h
>> +++ b/drivers/staging/ccree/cc_bitops.h
>> @@ -21,9 +21,14 @@
>>  #ifndef _CC_BITOPS_H_
>>  #define _CC_BITOPS_H_
>>
>> -#define BITMASK(mask_size) (((mask_size) < 32) ? \
>> - ((1UL << (mask_size)) - 1) : 0xUL)
>> -#define BITMASK_AT(mask_size, mask_offset) (BITMASK(mask_size) << 
>> (mask_offset))
>> +#include 
>> +#include 
>> +
>> +#define BITMASK(mask_size) (((mask_size) < 32) ? \
>> + ((1UL << (mask_size)) - 1) : 0xUL)
>> +
>> +#define BITMASK_AT(mask_size, mask_offset) \
>> + (BITMASK(mask_size) << (mask_offset))
>
> Why do you still needed these macros with this change?
>

My description of the patch is at fault. This patch gets rid of one
group of these
custom bit field ops users - and so does each one in the rest of the
series until
the patch that deletes them.

Getting rid of the custom bitfield ops was the reason why I've done
it, but of course
as you point it it isn't what the patch is doing. I will fix it.

>>
>>  #define BITFIELD_GET(word, bit_offset, bit_size) \
>>   (((word) >> (bit_offset)) & BITMASK(bit_size))
>> diff --git a/drivers/staging/ccree/cc_hw_queue_defs.h 
>> b/drivers/staging/ccree/cc_hw_queue_defs.h
>> index 7138176..af10850 100644
>> --- a/drivers/staging/ccree/cc_hw_queue_defs.h
>> +++ b/drivers/staging/ccree/cc_hw_queue_defs.h
>> @@ -22,25 +22,69 @@
>>  #include "cc_regs.h"
>>  #include "dx_crys_kernel.h"
>>
>> -/**
>> -*DEFINITIONS
>> -**/
>> -
>> -/* Dma AXI Secure bit */
>> -#define  AXI_SECURE  0
>> -#define AXI_NOT_SECURE   1
>> +/*
>> + *   DEFINITIONS
>> + 
>> */
>
> That has nothing to do with changing bitops :(
>
>>
>>  #define HW_DESC_SIZE_WORDS   6
>> -#define HW_QUEUE_SLOTS_MAX  15 /* Max. available slots in HW 
>> queue */
>> +#define HW_QUEUE_SLOTS_MAX  15 /* Max. available HW queue slots 
>> */
>
> Neither does this :(
>
> I'm stopping reviewing this here, please be more careful...

Thanks. I will fix and resend.

Gilad
-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/12] staging: ccree: correct coding style violations

[Gilad Ben-Yossef]

On Mon, May 29, 2017 at 7:57 PM, Joe Perches  wrote:
> On Mon, 2017-05-29 at 16:37 +0200, Greg Kroah-Hartman wrote:
>> On Sun, May 28, 2017 at 05:40:26PM +0300, Gilad Ben-Yossef wrote:
>> > cc_crypto_ctx.h had multiple coding style violations reported by
>> > checkpatch. Fix them all.
>>
>> Sorry, no.  You need to do only one-thing-per-patch, and "fix all coding
>> style issues is not "one thing".  I wouldn't take this kind of patch
>> from anyone else, so why should I take it from you?  :)
>
> Because he's the named MAINTAINER of the subsystem
> and you are acting as an upstream conduit.
>

LOL. Thank you Joe, but I have opted to upstream via staging to get the guidance
and review of Greg and other developers kind enough to offer it, and I'd be a
fool not to listen to them.

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/12] staging: ccree: correct coding style violations

[Joe Perches]

On Mon, 2017-05-29 at 16:37 +0200, Greg Kroah-Hartman wrote:
> On Sun, May 28, 2017 at 05:40:26PM +0300, Gilad Ben-Yossef wrote:
> > cc_crypto_ctx.h had multiple coding style violations reported by
> > checkpatch. Fix them all.
> 
> Sorry, no.  You need to do only one-thing-per-patch, and "fix all coding
> style issues is not "one thing".  I wouldn't take this kind of patch
> from anyone else, so why should I take it from you?  :)

Because he's the named MAINTAINER of the subsystem
and you are acting as an upstream conduit.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 00/34] i.MX Media Driver

[Hans Verkuil]

On 05/29/2017 05:36 PM, Sakari Ailus wrote:

Hi Hans,

On Mon, May 29, 2017 at 03:46:08PM +0200, Hans Verkuil wrote:

Hi Steve,

On 05/25/2017 02:29 AM, Steve Longerbeam wrote:

In version 7:

- video-mux: switched to Philipp's latest video-mux driver and updated
   bindings docs, that makes use of the mmio-mux framework.

- mmio-mux: includes Philipp's temporary patch that adds mmio-mux support
   to video-mux driver, until mux framework is merged.

- mmio-mux: updates to device tree from Philipp that define the i.MX6 mux
   devices and modifies the video-mux device to become a consumer of the
   video mmio-mux.

- minor updates to Documentation/media/v4l-drivers/imx.rst.

- ov5640: do nothing if entity stream count is greater than 1 in
   ov5640_s_stream().

- Previous versions of this driver had not tested the ability to enable
   multiple independent streams, for instance enabling multiple output
   pads from the imx6-mipi-csi2 subdevice, or enabling both prpenc and
   prpvf outputs. Marek Vasut tested this support and reported issues
   with it.

   v4l2_pipeline_inherit_controls() used the media graph walk APIs, but
   that walks both sink and source pads, so if there are multiple paths
   enabled to video capture devices, controls would be added to the wrong
   video capture device, and no controls added to the other enabled
   capture devices.

   These issues have been fixed. Control inheritance works correctly now
   even with multiple enabled capture paths, and (for example)
   simultaneous capture from prpenc and prpvf works also, and each with
   independent scaling, CSC, and controls. For example prpenc can be
   capturing with a 90 degree rotation, while prpvf is capturing with
   vertical flip.

   So the v4l2_pipeline_inherit_controls() patch has been dropped. The
   new version of control inheritance could be made generically available,
   but it would be more involved to incorporate it into v4l2-core.

- A new function imx_media_fill_default_mbus_fields() is added to setup
   colorimetry at sink pads, and these are propagated to source pads.

- Ensure that the current sink and source rectangles meet alignment
   restrictions before applying a new rotation control setting in
   prp-enc/vf subdevices.

- Chain the s_stream() subdev calls instead of implementing a custom
   stream on/off function that attempts to call a fixed set of subdevices
   in a pipeline in the correct order. This also simplifies imx6-mipi-csi2
   subdevice, since the correct MIPI CSI-2 startup sequence can be
   enforced completely in s_stream(), and s_power() is no longer
   required. This also paves the way for more arbitrary OF graphs
   external to the i.MX6.

- Converted the v4l2_subdev and media_entity ops structures to const.


What is the status as of v7?

 From what I can tell patch 2/34 needs an Ack from Rob Herring, patches
4-14 are out of scope for the media subsystem, patches 20-25 and 27-34
are all staging (so fine to be merged from my point of view).

I'm not sure if patch 26 (defconfig) should be applied while the imx
driver is in staging. I would suggest that this patch is moved to the end
of the series.

That leaves patches 15-19. I replied to patch 15 with a comment, patches
16-18 look good to me, although patches 17 and 18 should be combined to one
patch since patch 17 won't compile otherwise. Any idea when the multiplexer is
expected to be merged? (just curious)

I would really like to get this merged for 4.13, so did I miss anything?
 From what I can tell it is really just an Ack for patch 2/34.


The patchset re-initialises the control handler in the video node in its
media device link_setup callback, among other things. I don't think that
this as such should prevent including the driver in staging, but at the same
time my concern is what kind of guarantees can we give in terms of V4L2
framework support on things like this using the current API. I'd say "none".

If there's a need for this (there should not be, as the controls are exposed
to the user space through the sub-device nodes as the other drivers do), the
framework APIs need to be extended.

I think it'd be good to add a TODO file (such as the one in OMAP4ISS) as
part of the patchset that would include a list of things left to do so
they're not forgotten.



A TODO file, that's a very good point. Yes, we need that.

Regards,

Hans
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 16/34] [media] add Omnivision OV5640 sensor driver

[Sakari Ailus]

Hi Steve,

A few comments below.

On Wed, May 24, 2017 at 05:29:31PM -0700, Steve Longerbeam wrote:
> This driver is based on ov5640_mipi.c from Freescale imx_3.10.17_1.0.0_beta
> branch, modified heavily to bring forward to latest interfaces and code
> cleanup.
> 
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/media/i2c/Kconfig  |9 +
>  drivers/media/i2c/Makefile |1 +
>  drivers/media/i2c/ov5640.c | 2224 
> 
>  3 files changed, 2234 insertions(+)
>  create mode 100644 drivers/media/i2c/ov5640.c
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index fd181c9..ff082a7 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -539,6 +539,15 @@ config VIDEO_OV2659
> To compile this driver as a module, choose M here: the
> module will be called ov2659.
>  
> +config VIDEO_OV5640
> + tristate "OmniVision OV5640 sensor support"
> + depends on OF
> + depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
> + depends on MEDIA_CAMERA_SUPPORT
> + ---help---
> +   This is a Video4Linux2 sensor-level driver for the Omnivision
> +   OV5640 camera sensor with a MIPI CSI-2 interface.
> +
>  config VIDEO_OV5645
>   tristate "OmniVision OV5645 sensor support"
>   depends on OF
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 62323ec..dc6b0c4 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -58,6 +58,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
>  obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
>  obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
>  obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
> +obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
>  obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
>  obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> new file mode 100644
> index 000..2a032bc
> --- /dev/null
> +++ b/drivers/media/i2c/ov5640.c
> @@ -0,0 +1,2224 @@
> +/*
> + * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
> + * Copyright (C) 2014-2017 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Could you rebase this on the V4L2 fwnode patchset here, please?



> +#include 
> +
> +/* min/typical/max system clock (xclk) frequencies */
> +#define OV5640_XCLK_MIN  600
> +#define OV5640_XCLK_MAX 2400
> +
> +/*
> + * FIXME: there is no subdev API to set the MIPI CSI-2
> + * virtual channel yet, so this is hardcoded for now.
> + */
> +#define OV5640_MIPI_VC   1
> +
> +#define OV5640_DEFAULT_SLAVE_ID 0x3c
> +
> +#define OV5640_REG_CHIP_ID   0x300a
> +#define OV5640_REG_PAD_OUTPUT00  0x3019
> +#define OV5640_REG_SC_PLL_CTRL0  0x3034
> +#define OV5640_REG_SC_PLL_CTRL1  0x3035
> +#define OV5640_REG_SC_PLL_CTRL2  0x3036
> +#define OV5640_REG_SC_PLL_CTRL3  0x3037
> +#define OV5640_REG_SLAVE_ID  0x3100
> +#define OV5640_REG_SYS_ROOT_DIVIDER  0x3108
> +#define OV5640_REG_AWB_R_GAIN0x3400
> +#define OV5640_REG_AWB_G_GAIN0x3402
> +#define OV5640_REG_AWB_B_GAIN0x3404
> +#define OV5640_REG_AWB_MANUAL_CTRL   0x3406
> +#define OV5640_REG_AEC_PK_EXPOSURE_HI0x3500
> +#define OV5640_REG_AEC_PK_EXPOSURE_MED   0x3501
> +#define OV5640_REG_AEC_PK_EXPOSURE_LO0x3502
> +#define OV5640_REG_AEC_PK_MANUAL 0x3503
> +#define OV5640_REG_AEC_PK_REAL_GAIN  0x350a
> +#define OV5640_REG_AEC_PK_VTS0x350c
> +#define OV5640_REG_TIMING_HTS0x380c
> +#define OV5640_REG_TIMING_VTS0x380e
> +#define OV5640_REG_TIMING_TC_REG21   0x3821
> +#define OV5640_REG_AEC_CTRL000x3a00
> +#define OV5640_REG_AEC_B50_STEP  0x3a08
> +#define OV5640_REG_AEC_B60_STEP  0x3a0a
> +#define OV5640_REG_AEC_CTRL0D0x3a0d
> +#define OV5640_REG_AEC_CTRL0E0x3a0e
> +#define OV5640_REG_AEC_CTRL0F0x3a0f
> +#define OV5640_REG_AEC_CTRL100x3a10
> +#define OV5640_REG_AEC_CTRL110x3a11
> +#define OV5640_REG_AEC_CTRL1B0x3a1b
> +#define OV5640_REG_AEC_CTRL1E0x3a1e
> +#define OV5640_REG_AEC_CTRL1F0x3a1f
> +#define 

Re: [PATCH] staging/lustre/lov: remove set_fs() call from lov_getstripe()

[Oleg Drokin]

On May 29, 2017, at 10:28 AM, Greg Kroah-Hartman wrote:

> On Fri, May 26, 2017 at 11:40:33PM -0400, Oleg Drokin wrote:
>> lov_getstripe() calls set_fs(KERNEL_DS) so that it can handle a struct
>> lov_user_md pointer from user- or kernel-space.  This changes the
>> behavior of copy_from_user() on SPARC and may result in a misaligned
>> access exception which in turn oopses the kernel.  In fact the
>> relevant argument to lov_getstripe() is never called with a
>> kernel-space pointer and so changing the address limits is unnecessary
>> and so we remove the calls to save, set, and restore the address
>> limits.
>> 
>> Signed-off-by: John L. Hammond 
>> Reviewed-on: http://review.whamcloud.com/6150
>> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3221
>> Reviewed-by: Andreas Dilger 
>> Reviewed-by: Li Wei 
>> Signed-off-by: Oleg Drokin 
>> ---
>> drivers/staging/lustre/lustre/lov/lov_pack.c | 9 -
>> 1 file changed, 9 deletions(-)
> 
> So is this the patch that you want applied to the staging tree(s) as
> well?  If so, please let me know, otherwise I have no clue…

Yes, this is it.
Thanks!

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 00/34] i.MX Media Driver

[Sakari Ailus]

Hi Hans,

On Mon, May 29, 2017 at 03:46:08PM +0200, Hans Verkuil wrote:
> Hi Steve,
> 
> On 05/25/2017 02:29 AM, Steve Longerbeam wrote:
> >In version 7:
> >
> >- video-mux: switched to Philipp's latest video-mux driver and updated
> >   bindings docs, that makes use of the mmio-mux framework.
> >
> >- mmio-mux: includes Philipp's temporary patch that adds mmio-mux support
> >   to video-mux driver, until mux framework is merged.
> >
> >- mmio-mux: updates to device tree from Philipp that define the i.MX6 mux
> >   devices and modifies the video-mux device to become a consumer of the
> >   video mmio-mux.
> >
> >- minor updates to Documentation/media/v4l-drivers/imx.rst.
> >
> >- ov5640: do nothing if entity stream count is greater than 1 in
> >   ov5640_s_stream().
> >
> >- Previous versions of this driver had not tested the ability to enable
> >   multiple independent streams, for instance enabling multiple output
> >   pads from the imx6-mipi-csi2 subdevice, or enabling both prpenc and
> >   prpvf outputs. Marek Vasut tested this support and reported issues
> >   with it.
> >
> >   v4l2_pipeline_inherit_controls() used the media graph walk APIs, but
> >   that walks both sink and source pads, so if there are multiple paths
> >   enabled to video capture devices, controls would be added to the wrong
> >   video capture device, and no controls added to the other enabled
> >   capture devices.
> >
> >   These issues have been fixed. Control inheritance works correctly now
> >   even with multiple enabled capture paths, and (for example)
> >   simultaneous capture from prpenc and prpvf works also, and each with
> >   independent scaling, CSC, and controls. For example prpenc can be
> >   capturing with a 90 degree rotation, while prpvf is capturing with
> >   vertical flip.
> >
> >   So the v4l2_pipeline_inherit_controls() patch has been dropped. The
> >   new version of control inheritance could be made generically available,
> >   but it would be more involved to incorporate it into v4l2-core.
> >
> >- A new function imx_media_fill_default_mbus_fields() is added to setup
> >   colorimetry at sink pads, and these are propagated to source pads.
> >
> >- Ensure that the current sink and source rectangles meet alignment
> >   restrictions before applying a new rotation control setting in
> >   prp-enc/vf subdevices.
> >
> >- Chain the s_stream() subdev calls instead of implementing a custom
> >   stream on/off function that attempts to call a fixed set of subdevices
> >   in a pipeline in the correct order. This also simplifies imx6-mipi-csi2
> >   subdevice, since the correct MIPI CSI-2 startup sequence can be
> >   enforced completely in s_stream(), and s_power() is no longer
> >   required. This also paves the way for more arbitrary OF graphs
> >   external to the i.MX6.
> >
> >- Converted the v4l2_subdev and media_entity ops structures to const.
> 
> What is the status as of v7?
> 
> From what I can tell patch 2/34 needs an Ack from Rob Herring, patches
> 4-14 are out of scope for the media subsystem, patches 20-25 and 27-34
> are all staging (so fine to be merged from my point of view).
> 
> I'm not sure if patch 26 (defconfig) should be applied while the imx
> driver is in staging. I would suggest that this patch is moved to the end
> of the series.
> 
> That leaves patches 15-19. I replied to patch 15 with a comment, patches
> 16-18 look good to me, although patches 17 and 18 should be combined to one
> patch since patch 17 won't compile otherwise. Any idea when the multiplexer is
> expected to be merged? (just curious)
> 
> I would really like to get this merged for 4.13, so did I miss anything?
> From what I can tell it is really just an Ack for patch 2/34.

The patchset re-initialises the control handler in the video node in its
media device link_setup callback, among other things. I don't think that
this as such should prevent including the driver in staging, but at the same
time my concern is what kind of guarantees can we give in terms of V4L2
framework support on things like this using the current API. I'd say "none".

If there's a need for this (there should not be, as the controls are exposed
to the user space through the sub-device nodes as the other drivers do), the
framework APIs need to be extended.

I think it'd be good to add a TODO file (such as the one in OMAP4ISS) as
part of the patchset that would include a list of things left to do so
they're not forgotten.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 00/34] i.MX Media Driver

[Hans Verkuil]

On 05/29/2017 04:21 PM, Hans Verkuil wrote:

On 05/29/2017 04:15 PM, Philipp Zabel wrote:

On Mon, 2017-05-29 at 15:46 +0200, Hans Verkuil wrote:

Hi Steve,

On 05/25/2017 02:29 AM, Steve Longerbeam wrote:

In version 7:

- video-mux: switched to Philipp's latest video-mux driver and updated
 bindings docs, that makes use of the mmio-mux framework.

- mmio-mux: includes Philipp's temporary patch that adds mmio-mux support
 to video-mux driver, until mux framework is merged.

- mmio-mux: updates to device tree from Philipp that define the i.MX6 mux
 devices and modifies the video-mux device to become a consumer of the
 video mmio-mux.

- minor updates to Documentation/media/v4l-drivers/imx.rst.

- ov5640: do nothing if entity stream count is greater than 1 in
 ov5640_s_stream().

- Previous versions of this driver had not tested the ability to enable
 multiple independent streams, for instance enabling multiple output
 pads from the imx6-mipi-csi2 subdevice, or enabling both prpenc and
 prpvf outputs. Marek Vasut tested this support and reported issues
 with it.

 v4l2_pipeline_inherit_controls() used the media graph walk APIs, but
 that walks both sink and source pads, so if there are multiple paths
 enabled to video capture devices, controls would be added to the wrong
 video capture device, and no controls added to the other enabled
 capture devices.

 These issues have been fixed. Control inheritance works correctly now
 even with multiple enabled capture paths, and (for example)
 simultaneous capture from prpenc and prpvf works also, and each with
 independent scaling, CSC, and controls. For example prpenc can be
 capturing with a 90 degree rotation, while prpvf is capturing with
 vertical flip.

 So the v4l2_pipeline_inherit_controls() patch has been dropped. The
 new version of control inheritance could be made generically available,
 but it would be more involved to incorporate it into v4l2-core.

- A new function imx_media_fill_default_mbus_fields() is added to setup
 colorimetry at sink pads, and these are propagated to source pads.

- Ensure that the current sink and source rectangles meet alignment
 restrictions before applying a new rotation control setting in
 prp-enc/vf subdevices.

- Chain the s_stream() subdev calls instead of implementing a custom
 stream on/off function that attempts to call a fixed set of subdevices
 in a pipeline in the correct order. This also simplifies imx6-mipi-csi2
 subdevice, since the correct MIPI CSI-2 startup sequence can be
 enforced completely in s_stream(), and s_power() is no longer
 required. This also paves the way for more arbitrary OF graphs
 external to the i.MX6.

- Converted the v4l2_subdev and media_entity ops structures to const.


What is the status as of v7?

   From what I can tell patch 2/34 needs an Ack from Rob Herring, patches
4-14 are out of scope for the media subsystem, patches 20-25 and 27-34
are all staging (so fine to be merged from my point of view).

I'm not sure if patch 26 (defconfig) should be applied while the imx
driver is in staging. I would suggest that this patch is moved to the end
of the series.

That leaves patches 15-19. I replied to patch 15 with a comment, patches
16-18 look good to me, although patches 17 and 18 should be combined to one
patch since patch 17 won't compile otherwise.


Is this a problem? It won't break any builds as patch 17 depends on
CONFIG_MULTIPLEXER, which doesn't exist yet. I'm fine with merging the
two patches, though.


You are right, but it is weird. I think I would prefer to have these two
merged and the #ifdef CONFIG_MULTIPLEXER bits removed. Just a note in the
commit log that this should be converted to the multiplexer when that gets
merged would be enough.

Dead code in drivers/media should be avoided because that's what this
driver currently has.


Thanks for those updates! That really leaves just an Ack for patch 2/34.

Sooo close!

Regards,

Hans
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/11] Staging: android: Fix style issue in ion-ioctl.c

[Greg KH]

On Sun, May 28, 2017 at 09:56:15PM +0200, Mateusz Nowotyński wrote:
> This is patch fixing code alignment style issue in
> android/ion/ion-ioctl.c file
> Signed-off-by: Mateusz Nowotyński 

I need a blank line before the signed-off-by: line please.

Can you fix this up and resend all of these?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 12/27] Drivers: ccree: ssi_fips_ll.c - align block comments

[Greg KH]

On Wed, May 24, 2017 at 04:43:52PM +1200, Derek Robson wrote:
> Fixed block comment alignment, Style fix only
> Found using checkpatch
> 
> Signed-off-by: Derek Robson 
> ---
>  drivers/staging/ccree/ssi_fips_ll.c | 21 -
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/ccree/ssi_fips_ll.c 
> b/drivers/staging/ccree/ssi_fips_ll.c
> index 7c7c922f0788..cb487d747ee8 100644
> --- a/drivers/staging/ccree/ssi_fips_ll.c
> +++ b/drivers/staging/ccree/ssi_fips_ll.c
> @@ -15,9 +15,9 @@
>   */
>  
>  /**
> -This file defines the driver FIPS Low Level implmentaion functions,
> -that executes the KAT.
> -***/
> + * This file defines the driver FIPS Low Level implmentaion functions,
> + * that executes the KAT.
> + ***/
>  #include 
>  
>  #include "ssi_driver.h"
> @@ -816,7 +816,8 @@ ssi_hmac_fips_run_test(struct ssi_drvdata *drvdata,
>  dma_addr_t digest_bytes_len_dma_addr)
>  {
>   /* The implemented flow is not the same as the one implemented in 
> ssi_hash.c (setkey + digest flows).
> -In this flow, there is no need to store and reload some of the 
> intermidiate results. */
> +  * In this flow, there is no need to store and reload some of the 
> intermidiate results.
> +  */
>  
>   /* max number of descriptors used for the flow */
>   #define FIPS_HMAC_MAX_SEQ_LEN 12
> @@ -948,9 +949,9 @@ ssi_hmac_fips_run_test(struct ssi_drvdata *drvdata,
>   idx++;
>  
>   /* at this point:
> -tmp_digest = H(o_key_pad)
> -k0 = H(i_key_pad || m)
> -*/
> +  * tmp_digest = H(o_key_pad)
> +  * k0 = H(i_key_pad || m)
> +  */
>  
>   /* Loading hash opad xor key state */
>   HW_DESC_INIT([idx]);
> @@ -1413,8 +1414,10 @@ ssi_gcm_fips_run_test(struct ssi_drvdata *drvdata,
>   idx++;
>  
>   /* Configure Hash Engine to work with GHASH.
> -Since it was not possible to extend HASH submodes to add GHASH,
> -The following command is necessary in order to select GHASH 
> (according to HW designers)*/
> +  * Since it was not possible to extend HASH submodes to add GHASH,
> +  * The following command is necessary in order to 

You added a coding style issue that was not previously here in this
patch, which isn't ok :(

I've stopped here with this series, please fix up the reset of this
series (hint, this wasn't the only place you messed this up) and resend.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 02/12] staging: ccree: move to kernel bitfields/bitops

[Greg Kroah-Hartman]

On Sun, May 28, 2017 at 05:40:27PM +0300, Gilad Ben-Yossef wrote:
> ccree had a lot of boilerplate code for dealing with bitops
> and bitfield register access. Move it over to the generic kernel
> infrastructure used for doing the same.
> 
> Signed-off-by: Gilad Ben-Yossef 
> ---
>  drivers/staging/ccree/cc_bitops.h|  11 +-
>  drivers/staging/ccree/cc_hw_queue_defs.h | 679 ---
>  drivers/staging/ccree/ssi_aead.c | 901 
> +++
>  drivers/staging/ccree/ssi_cipher.c   | 224 
>  drivers/staging/ccree/ssi_driver.h   |   4 +-
>  drivers/staging/ccree/ssi_fips_ll.c  | 704 
>  drivers/staging/ccree/ssi_hash.c | 832 ++--
>  drivers/staging/ccree/ssi_ivgen.c|  77 ++-
>  drivers/staging/ccree/ssi_request_mgr.c  |  25 +-
>  drivers/staging/ccree/ssi_sram_mgr.c |   8 +-
>  10 files changed, 1765 insertions(+), 1700 deletions(-)
> 
> diff --git a/drivers/staging/ccree/cc_bitops.h 
> b/drivers/staging/ccree/cc_bitops.h
> index ee5238a..a12a65c 100644
> --- a/drivers/staging/ccree/cc_bitops.h
> +++ b/drivers/staging/ccree/cc_bitops.h
> @@ -21,9 +21,14 @@
>  #ifndef _CC_BITOPS_H_
>  #define _CC_BITOPS_H_
>  
> -#define BITMASK(mask_size) (((mask_size) < 32) ? \
> - ((1UL << (mask_size)) - 1) : 0xUL)
> -#define BITMASK_AT(mask_size, mask_offset) (BITMASK(mask_size) << 
> (mask_offset))
> +#include 
> +#include 
> +
> +#define BITMASK(mask_size) (((mask_size) < 32) ? \
> + ((1UL << (mask_size)) - 1) : 0xUL)
> +
> +#define BITMASK_AT(mask_size, mask_offset) \
> + (BITMASK(mask_size) << (mask_offset))

Why do you still needed these macros with this change?

>  
>  #define BITFIELD_GET(word, bit_offset, bit_size) \
>   (((word) >> (bit_offset)) & BITMASK(bit_size))
> diff --git a/drivers/staging/ccree/cc_hw_queue_defs.h 
> b/drivers/staging/ccree/cc_hw_queue_defs.h
> index 7138176..af10850 100644
> --- a/drivers/staging/ccree/cc_hw_queue_defs.h
> +++ b/drivers/staging/ccree/cc_hw_queue_defs.h
> @@ -22,25 +22,69 @@
>  #include "cc_regs.h"
>  #include "dx_crys_kernel.h"
>  
> -/**
> -*DEFINITIONS
> -**/
> -
> -/* Dma AXI Secure bit */
> -#define  AXI_SECURE  0
> -#define AXI_NOT_SECURE   1
> +/*
> + *   DEFINITIONS
> + 
> */

That has nothing to do with changing bitops :(

>  
>  #define HW_DESC_SIZE_WORDS   6
> -#define HW_QUEUE_SLOTS_MAX  15 /* Max. available slots in HW 
> queue */
> +#define HW_QUEUE_SLOTS_MAX  15 /* Max. available HW queue slots 
> */

Neither does this :(

I'm stopping reviewing this here, please be more careful...

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 04/12] staging: ccree: cleanup lli access macro

[Greg Kroah-Hartman]

On Sun, May 28, 2017 at 05:40:29PM +0300, Gilad Ben-Yossef wrote:
> The Linked List Item descriptors were being accessed via
> a baroque set of defines and macro. Re-factor for structs
> and inline function for readability and sanity.
> 
> Signed-off-by: Gilad Ben-Yossef 
> ---
>  drivers/staging/ccree/cc_lli_defs.h| 65 
> +++---
>  drivers/staging/ccree/ssi_buffer_mgr.c | 45 +--
>  2 files changed, 44 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/staging/ccree/cc_lli_defs.h 
> b/drivers/staging/ccree/cc_lli_defs.h
> index 857b94f..c6b2917 100644
> --- a/drivers/staging/ccree/cc_lli_defs.h
> +++ b/drivers/staging/ccree/cc_lli_defs.h
> @@ -28,36 +28,43 @@
>  
>  #define CC_MAX_MLLI_ENTRY_SIZE 0x1
>  
> -#define LLI_SET_ADDR(__lli_p, __addr) do {   \
> - u32 *lli_p = (u32 *)__lli_p;\
> - typeof(__addr) addr = __addr;   \
> - \
> - BITFIELD_SET(lli_p[LLI_WORD0_OFFSET],   \
> - LLI_LADDR_BIT_OFFSET,   \
> - LLI_LADDR_BIT_SIZE, (addr & U32_MAX));  \
> - \
> - BITFIELD_SET(lli_p[LLI_WORD1_OFFSET],   \
> - LLI_HADDR_BIT_OFFSET,   \
> - LLI_HADDR_BIT_SIZE, MSB64(addr));   \
> - } while (0)
> -
> -#define LLI_SET_SIZE(lli_p, size)\
> - BITFIELD_SET(((u32 *)(lli_p))[LLI_WORD1_OFFSET],\
> - LLI_SIZE_BIT_OFFSET, LLI_SIZE_BIT_SIZE, size)
> +#define LLI_MAX_NUM_OF_DATA_ENTRIES 128
> +#define LLI_MAX_NUM_OF_ASSOC_DATA_ENTRIES 4
> +#define MLLI_TABLE_MIN_ALIGNMENT 4 /* 32 bit alignment */
> +#define MAX_NUM_OF_BUFFERS_IN_MLLI 4
> +#define MAX_NUM_OF_TOTAL_MLLI_ENTRIES (2 * LLI_MAX_NUM_OF_DATA_ENTRIES + \
> +LLI_MAX_NUM_OF_ASSOC_DATA_ENTRIES)
> +
> +struct cc_lli_entry {
> +#ifndef __LITTLE_ENDIAN__
> + u32 addr_lsb;
> + u16 size;
> + u16 addr_msb;
> +#else /* __BIG_ENDIAN__ */
> + u16 addr_msb;
> + u16 size;
> + u32 addr_lsb;
> +#endif

How is the bits within the different variables also not affected by the
endian issues?  Just moving them around in the 64bits seems really
strange.

Why not just use the "normal" ways to address data in an endian-neutral
way instead of making your own macros up here?  (i.e. shift and mask.)

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ks7010: define ether_hdr.h_proto to be big-endian

[Greg KH]

On Sun, May 21, 2017 at 10:15:11AM +0100, d...@acm.org wrote:
> From: Richard Porter 
> 
> Fixes sparse warnings:
> drivers/staging/ks7010/ks_hostif.c:339:21: warning: cast to restricted __be16
> drivers/staging/ks7010/ks_hostif.c:430:21: warning: cast to restricted __be16
> drivers/staging/ks7010/ks_hostif.c:1226:21: warning: cast to restricted __be16
> 
> Signed-off-by: Richard Porter 
> ---
>  drivers/staging/ks7010/eap_packet.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/ks7010/eap_packet.h 
> b/drivers/staging/ks7010/eap_packet.h
> index b2d25ef..eec4aa4 100644
> --- a/drivers/staging/ks7010/eap_packet.h
> +++ b/drivers/staging/ks7010/eap_packet.h
> @@ -18,7 +18,7 @@ struct ether_hdr {
>   unsigned char h_source_snap;
>   unsigned char h_command;
>   unsigned char h_vendor_id[3];
> - unsigned short h_proto; /* packet type ID field */
> + __be16 h_proto; /* packet type ID field */

How do you know this field is really this endian?  Is this on the wire?
On the device itself?  How have you verified this is correct?

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 01/12] staging: ccree: correct coding style violations

[Greg Kroah-Hartman]

On Sun, May 28, 2017 at 05:40:26PM +0300, Gilad Ben-Yossef wrote:
> cc_crypto_ctx.h had multiple coding style violations reported by
> checkpatch. Fix them all.

Sorry, no.  You need to do only one-thing-per-patch, and "fix all coding
style issues is not "one thing".  I wouldn't take this kind of patch
from anyone else, so why should I take it from you?  :)

thnaks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging/lustre/lov: remove set_fs() call from lov_getstripe()

[Greg Kroah-Hartman]

On Fri, May 26, 2017 at 11:40:33PM -0400, Oleg Drokin wrote:
> lov_getstripe() calls set_fs(KERNEL_DS) so that it can handle a struct
> lov_user_md pointer from user- or kernel-space.  This changes the
> behavior of copy_from_user() on SPARC and may result in a misaligned
> access exception which in turn oopses the kernel.  In fact the
> relevant argument to lov_getstripe() is never called with a
> kernel-space pointer and so changing the address limits is unnecessary
> and so we remove the calls to save, set, and restore the address
> limits.
> 
> Signed-off-by: John L. Hammond 
> Reviewed-on: http://review.whamcloud.com/6150
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3221
> Reviewed-by: Andreas Dilger 
> Reviewed-by: Li Wei 
> Signed-off-by: Oleg Drokin 
> ---
>  drivers/staging/lustre/lustre/lov/lov_pack.c | 9 -
>  1 file changed, 9 deletions(-)

So is this the patch that you want applied to the staging tree(s) as
well?  If so, please let me know, otherwise I have no clue...

Come on, you know better than this...

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: fsl-dpaa2/eth: Fix address translations

[Laurentiu Tudor]

On 05/25/2017 03:31 PM, Ruxandra Ioana Radulescu wrote:
>> -Original Message-
>> From: Laurentiu Tudor
>> Sent: Wednesday, May 24, 2017 3:34 PM
>> To: Ruxandra Ioana Radulescu ;
>> gre...@linuxfoundation.org
>> Cc: de...@driverdev.osuosl.org; linux-ker...@vger.kernel.org;
>> ag...@suse.de; a...@arndb.de; linux-arm-ker...@lists.infradead.org;
>> io...@lists.linux-foundation.org; Bogdan Purcareata
>> ; stuyo...@gmail.com; Nipun Gupta
>> 
>> Subject: Re: [PATCH 1/2] staging: fsl-dpaa2/eth: Fix address translations
>>
>> Hi Ioana,
>>
>> Debatable nit inline.
>>
>> On 05/24/2017 03:13 PM, Ioana Radulescu wrote:
>>> Use the correct mechanisms for translating a DMA-mapped IOVA
>>> address into a virtual one. Without this fix, once SMMU is
>>> enabled on Layerscape platforms, the Ethernet driver throws
>>> IOMMU translation faults.
>>>
>>> Signed-off-by: Nipun Gupta 
>>> Signed-off-by: Ioana Radulescu 
>>> ---
>>>drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 25
>> +++--
>>>drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h |  1 +
>>>2 files changed, 20 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
>> b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
>>> index 6f9eed66c64d..3fee0d6f17e0 100644
>>> --- a/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
>>> +++ b/drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c
>>> @@ -37,6 +37,7 @@
>>>#include 
>>>#include 
>>>#include 
>>> +#include 
>>>
>>>#include "../../fsl-mc/include/mc.h"
>>>#include "../../fsl-mc/include/mc-sys.h"
>>> @@ -54,6 +55,16 @@ MODULE_DESCRIPTION("Freescale DPAA2 Ethernet
>> Driver");
>>>
>>>const char dpaa2_eth_drv_version[] = "0.1";
>>>
>>> +static void *dpaa2_iova_to_virt(struct iommu_domain *domain,
>>
>> if you pass a "struct dpaa2_eth_priv *priv" instead of "iommu_domain"
>> you can move the priv->iommu_domain reference in the function and
>> slightly simplify the call sites.
>
> Fair point, but I'd prefer keeping this function independent of the
> Ethernet driver's private data structure. This way, if other (future)
> DPAA2 drivers will need a similar function, we can just move it
> to a common area instead of duplicating the code.

Understood. Fine by me then.

---
Best Regards, Laurentiu
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 00/34] i.MX Media Driver

[Hans Verkuil]

On 05/29/2017 04:15 PM, Philipp Zabel wrote:

On Mon, 2017-05-29 at 15:46 +0200, Hans Verkuil wrote:

Hi Steve,

On 05/25/2017 02:29 AM, Steve Longerbeam wrote:

In version 7:

- video-mux: switched to Philipp's latest video-mux driver and updated
bindings docs, that makes use of the mmio-mux framework.

- mmio-mux: includes Philipp's temporary patch that adds mmio-mux support
to video-mux driver, until mux framework is merged.

- mmio-mux: updates to device tree from Philipp that define the i.MX6 mux
devices and modifies the video-mux device to become a consumer of the
video mmio-mux.

- minor updates to Documentation/media/v4l-drivers/imx.rst.

- ov5640: do nothing if entity stream count is greater than 1 in
ov5640_s_stream().

- Previous versions of this driver had not tested the ability to enable
multiple independent streams, for instance enabling multiple output
pads from the imx6-mipi-csi2 subdevice, or enabling both prpenc and
prpvf outputs. Marek Vasut tested this support and reported issues
with it.

v4l2_pipeline_inherit_controls() used the media graph walk APIs, but
that walks both sink and source pads, so if there are multiple paths
enabled to video capture devices, controls would be added to the wrong
video capture device, and no controls added to the other enabled
capture devices.

These issues have been fixed. Control inheritance works correctly now
even with multiple enabled capture paths, and (for example)
simultaneous capture from prpenc and prpvf works also, and each with
independent scaling, CSC, and controls. For example prpenc can be
capturing with a 90 degree rotation, while prpvf is capturing with
vertical flip.

So the v4l2_pipeline_inherit_controls() patch has been dropped. The
new version of control inheritance could be made generically available,
but it would be more involved to incorporate it into v4l2-core.

- A new function imx_media_fill_default_mbus_fields() is added to setup
colorimetry at sink pads, and these are propagated to source pads.

- Ensure that the current sink and source rectangles meet alignment
restrictions before applying a new rotation control setting in
prp-enc/vf subdevices.

- Chain the s_stream() subdev calls instead of implementing a custom
stream on/off function that attempts to call a fixed set of subdevices
in a pipeline in the correct order. This also simplifies imx6-mipi-csi2
subdevice, since the correct MIPI CSI-2 startup sequence can be
enforced completely in s_stream(), and s_power() is no longer
required. This also paves the way for more arbitrary OF graphs
external to the i.MX6.

- Converted the v4l2_subdev and media_entity ops structures to const.


What is the status as of v7?

  From what I can tell patch 2/34 needs an Ack from Rob Herring, patches
4-14 are out of scope for the media subsystem, patches 20-25 and 27-34
are all staging (so fine to be merged from my point of view).

I'm not sure if patch 26 (defconfig) should be applied while the imx
driver is in staging. I would suggest that this patch is moved to the end
of the series.

That leaves patches 15-19. I replied to patch 15 with a comment, patches
16-18 look good to me, although patches 17 and 18 should be combined to one
patch since patch 17 won't compile otherwise.


Is this a problem? It won't break any builds as patch 17 depends on
CONFIG_MULTIPLEXER, which doesn't exist yet. I'm fine with merging the
two patches, though.


You are right, but it is weird. I think I would prefer to have these two
merged and the #ifdef CONFIG_MULTIPLEXER bits removed. Just a note in the
commit log that this should be converted to the multiplexer when that gets
merged would be enough.

Dead code in drivers/media should be avoided because that's what this
driver currently has.

Regards,

Hans




Any idea when the multiplexer is expected to be merged? (just curious)


I have no idea. v15 of the multiplexer framework patchset was posted on
2017-05-14, is still waiting for comments.


I would really like to get this merged for 4.13, so did I miss anything?


Seconded, and not that I can tell.


  From what I can tell it is really just an Ack for patch 2/34.


regards
Philipp



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 00/34] i.MX Media Driver

[Philipp Zabel]

On Mon, 2017-05-29 at 15:46 +0200, Hans Verkuil wrote:
> Hi Steve,
> 
> On 05/25/2017 02:29 AM, Steve Longerbeam wrote:
> > In version 7:
> > 
> > - video-mux: switched to Philipp's latest video-mux driver and updated
> >bindings docs, that makes use of the mmio-mux framework.
> > 
> > - mmio-mux: includes Philipp's temporary patch that adds mmio-mux support
> >to video-mux driver, until mux framework is merged.
> > 
> > - mmio-mux: updates to device tree from Philipp that define the i.MX6 mux
> >devices and modifies the video-mux device to become a consumer of the
> >video mmio-mux.
> > 
> > - minor updates to Documentation/media/v4l-drivers/imx.rst.
> > 
> > - ov5640: do nothing if entity stream count is greater than 1 in
> >ov5640_s_stream().
> > 
> > - Previous versions of this driver had not tested the ability to enable
> >multiple independent streams, for instance enabling multiple output
> >pads from the imx6-mipi-csi2 subdevice, or enabling both prpenc and
> >prpvf outputs. Marek Vasut tested this support and reported issues
> >with it.
> > 
> >v4l2_pipeline_inherit_controls() used the media graph walk APIs, but
> >that walks both sink and source pads, so if there are multiple paths
> >enabled to video capture devices, controls would be added to the wrong
> >video capture device, and no controls added to the other enabled
> >capture devices.
> > 
> >These issues have been fixed. Control inheritance works correctly now
> >even with multiple enabled capture paths, and (for example)
> >simultaneous capture from prpenc and prpvf works also, and each with
> >independent scaling, CSC, and controls. For example prpenc can be
> >capturing with a 90 degree rotation, while prpvf is capturing with
> >vertical flip.
> > 
> >So the v4l2_pipeline_inherit_controls() patch has been dropped. The
> >new version of control inheritance could be made generically available,
> >but it would be more involved to incorporate it into v4l2-core.
> > 
> > - A new function imx_media_fill_default_mbus_fields() is added to setup
> >colorimetry at sink pads, and these are propagated to source pads.
> > 
> > - Ensure that the current sink and source rectangles meet alignment
> >restrictions before applying a new rotation control setting in
> >prp-enc/vf subdevices.
> > 
> > - Chain the s_stream() subdev calls instead of implementing a custom
> >stream on/off function that attempts to call a fixed set of subdevices
> >in a pipeline in the correct order. This also simplifies imx6-mipi-csi2
> >subdevice, since the correct MIPI CSI-2 startup sequence can be
> >enforced completely in s_stream(), and s_power() is no longer
> >required. This also paves the way for more arbitrary OF graphs
> >external to the i.MX6.
> > 
> > - Converted the v4l2_subdev and media_entity ops structures to const.
> 
> What is the status as of v7?
>
>  From what I can tell patch 2/34 needs an Ack from Rob Herring, patches
> 4-14 are out of scope for the media subsystem, patches 20-25 and 27-34
> are all staging (so fine to be merged from my point of view).
> 
> I'm not sure if patch 26 (defconfig) should be applied while the imx
> driver is in staging. I would suggest that this patch is moved to the end
> of the series.
> 
> That leaves patches 15-19. I replied to patch 15 with a comment, patches
> 16-18 look good to me, although patches 17 and 18 should be combined to one
> patch since patch 17 won't compile otherwise.

Is this a problem? It won't break any builds as patch 17 depends on
CONFIG_MULTIPLEXER, which doesn't exist yet. I'm fine with merging the
two patches, though.

> Any idea when the multiplexer is expected to be merged? (just curious)

I have no idea. v15 of the multiplexer framework patchset was posted on
2017-05-14, is still waiting for comments.

> I would really like to get this merged for 4.13, so did I miss anything?

Seconded, and not that I can tell.

>  From what I can tell it is really just an Ack for patch 2/34.

regards
Philipp

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 15/34] add mux and video interface bridge entity functions

[Philipp Zabel]

On Mon, 2017-05-29 at 15:37 +0200, Hans Verkuil wrote:
> On 05/25/2017 02:29 AM, Steve Longerbeam wrote:
> > From: Philipp Zabel 
> > 
> > Signed-off-by: Philipp Zabel 
> > 
> > - renamed MEDIA_ENT_F_MUX to MEDIA_ENT_F_VID_MUX
> > 
> > Signed-off-by: Steve Longerbeam 
> > ---
> >   Documentation/media/uapi/mediactl/media-types.rst | 22 
> > ++
> >   include/uapi/linux/media.h|  6 ++
> >   2 files changed, 28 insertions(+)
> > 
> > diff --git a/Documentation/media/uapi/mediactl/media-types.rst 
> > b/Documentation/media/uapi/mediactl/media-types.rst
> > index 2a5164a..47ee003 100644
> > --- a/Documentation/media/uapi/mediactl/media-types.rst
> > +++ b/Documentation/media/uapi/mediactl/media-types.rst
> > @@ -299,6 +299,28 @@ Types and flags used to represent the media graph 
> > elements
> >   received on its sink pad and outputs the statistics data on
> >   its source pad.
> >   
> > +-  ..  row 29
> > +
> > +   ..  _MEDIA-ENT-F-VID-MUX:
> > +
> > +   -  ``MEDIA_ENT_F_VID_MUX``
> > +
> > +   - Video multiplexer. An entity capable of multiplexing must have at
> > + least two sink pads and one source pad, and must pass the video
> > + frame(s) received from the active sink pad to the source pad. 
> > Video
> > + frame(s) from the inactive sink pads are discarded.
> > +
> > +-  ..  row 30
> > +
> > +   ..  _MEDIA-ENT-F-VID-IF-BRIDGE:
> > +
> > +   -  ``MEDIA_ENT_F_VID_IF_BRIDGE``
> > +
> > +   - Video interface bridge. A video interface bridge entity must have 
> > at
> > + least one sink pad and one source pad. It receives video frame(s) 
> > on
> > + its sink pad in one bus format (HDMI, eDP, MIPI CSI-2, ...) and
> > + converts them and outputs them on its source pad in another bus 
> > format
> > + (eDP, MIPI CSI-2, parallel, ...).
> 
> I'm unhappy with the term 'bus format'. It's too close to 'mediabus format'.
> How about calling it "bus protocol"?

How about:

   "It receives video frames on its sink pad from an input video bus
of one type (HDMI, eDP, MIPI CSI-2, ...), and outputs them on its
source pad to an output video bus of another type (eDP, MIPI
CSI-2, parallel, ...)."

regards
Philipp

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 15/34] add mux and video interface bridge entity functions

[Hans Verkuil]

On 05/29/2017 03:51 PM, Philipp Zabel wrote:

On Mon, 2017-05-29 at 15:37 +0200, Hans Verkuil wrote:

On 05/25/2017 02:29 AM, Steve Longerbeam wrote:

From: Philipp Zabel 

Signed-off-by: Philipp Zabel 

- renamed MEDIA_ENT_F_MUX to MEDIA_ENT_F_VID_MUX

Signed-off-by: Steve Longerbeam 
---
   Documentation/media/uapi/mediactl/media-types.rst | 22 ++
   include/uapi/linux/media.h|  6 ++
   2 files changed, 28 insertions(+)

diff --git a/Documentation/media/uapi/mediactl/media-types.rst 
b/Documentation/media/uapi/mediactl/media-types.rst
index 2a5164a..47ee003 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -299,6 +299,28 @@ Types and flags used to represent the media graph elements
  received on its sink pad and outputs the statistics data on
  its source pad.
   
+-  ..  row 29

+
+   ..  _MEDIA-ENT-F-VID-MUX:
+
+   -  ``MEDIA_ENT_F_VID_MUX``
+
+   - Video multiplexer. An entity capable of multiplexing must have at
+ least two sink pads and one source pad, and must pass the video
+ frame(s) received from the active sink pad to the source pad. Video
+ frame(s) from the inactive sink pads are discarded.
+
+-  ..  row 30
+
+   ..  _MEDIA-ENT-F-VID-IF-BRIDGE:
+
+   -  ``MEDIA_ENT_F_VID_IF_BRIDGE``
+
+   - Video interface bridge. A video interface bridge entity must have at
+ least one sink pad and one source pad. It receives video frame(s) on
+ its sink pad in one bus format (HDMI, eDP, MIPI CSI-2, ...) and
+ converts them and outputs them on its source pad in another bus format
+ (eDP, MIPI CSI-2, parallel, ...).


I'm unhappy with the term 'bus format'. It's too close to 'mediabus format'.
How about calling it "bus protocol"?


How about:

"It receives video frames on its sink pad from an input video bus
 of one type (HDMI, eDP, MIPI CSI-2, ...), and outputs them on its
 source pad to an output video bus of another type (eDP, MIPI
 CSI-2, parallel, ...)."


That's even better!

Regards,

Hans
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v7 00/34] i.MX Media Driver

[Hans Verkuil]

Hi Steve,

On 05/25/2017 02:29 AM, Steve Longerbeam wrote:

In version 7:

- video-mux: switched to Philipp's latest video-mux driver and updated
   bindings docs, that makes use of the mmio-mux framework.

- mmio-mux: includes Philipp's temporary patch that adds mmio-mux support
   to video-mux driver, until mux framework is merged.

- mmio-mux: updates to device tree from Philipp that define the i.MX6 mux
   devices and modifies the video-mux device to become a consumer of the
   video mmio-mux.

- minor updates to Documentation/media/v4l-drivers/imx.rst.

- ov5640: do nothing if entity stream count is greater than 1 in
   ov5640_s_stream().

- Previous versions of this driver had not tested the ability to enable
   multiple independent streams, for instance enabling multiple output
   pads from the imx6-mipi-csi2 subdevice, or enabling both prpenc and
   prpvf outputs. Marek Vasut tested this support and reported issues
   with it.

   v4l2_pipeline_inherit_controls() used the media graph walk APIs, but
   that walks both sink and source pads, so if there are multiple paths
   enabled to video capture devices, controls would be added to the wrong
   video capture device, and no controls added to the other enabled
   capture devices.

   These issues have been fixed. Control inheritance works correctly now
   even with multiple enabled capture paths, and (for example)
   simultaneous capture from prpenc and prpvf works also, and each with
   independent scaling, CSC, and controls. For example prpenc can be
   capturing with a 90 degree rotation, while prpvf is capturing with
   vertical flip.

   So the v4l2_pipeline_inherit_controls() patch has been dropped. The
   new version of control inheritance could be made generically available,
   but it would be more involved to incorporate it into v4l2-core.

- A new function imx_media_fill_default_mbus_fields() is added to setup
   colorimetry at sink pads, and these are propagated to source pads.

- Ensure that the current sink and source rectangles meet alignment
   restrictions before applying a new rotation control setting in
   prp-enc/vf subdevices.

- Chain the s_stream() subdev calls instead of implementing a custom
   stream on/off function that attempts to call a fixed set of subdevices
   in a pipeline in the correct order. This also simplifies imx6-mipi-csi2
   subdevice, since the correct MIPI CSI-2 startup sequence can be
   enforced completely in s_stream(), and s_power() is no longer
   required. This also paves the way for more arbitrary OF graphs
   external to the i.MX6.

- Converted the v4l2_subdev and media_entity ops structures to const.


What is the status as of v7?

From what I can tell patch 2/34 needs an Ack from Rob Herring, patches
4-14 are out of scope for the media subsystem, patches 20-25 and 27-34
are all staging (so fine to be merged from my point of view).

I'm not sure if patch 26 (defconfig) should be applied while the imx
driver is in staging. I would suggest that this patch is moved to the end
of the series.

That leaves patches 15-19. I replied to patch 15 with a comment, patches
16-18 look good to me, although patches 17 and 18 should be combined to one
patch since patch 17 won't compile otherwise. Any idea when the multiplexer is
expected to be merged? (just curious)

I would really like to get this merged for 4.13, so did I miss anything?
From what I can tell it is really just an Ack for patch 2/34.

Regards,

Hans




Marek Vasut (1):
   media: imx: Drop warning upon multiple S_STREAM disable calls

Philipp Zabel (9):
   dt-bindings: Add bindings for video-multiplexer device
   ARM: dts: imx6qdl: add multiplexer controls
   ARM: dts: imx6qdl: Add video multiplexers, mipi_csi, and their
 connections
   add mux and video interface bridge entity functions
   platform: add video-multiplexer subdevice driver
   platform: video-mux: include temporary mmio-mux support
   media: imx: csi: increase burst size for YUV formats
   media: imx: csi: add frame skipping support
   media: imx: csi: add sink selection rectangles

Russell King (3):
   media: imx: csi: add support for bayer formats
   media: imx: csi: add frame size/interval enumeration
   media: imx: capture: add frame sizes/interval enumeration

Steve Longerbeam (21):
   [media] dt-bindings: Add bindings for i.MX media driver
   [media] dt/bindings: Add bindings for OV5640
   ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node
   ARM: dts: imx6qdl: add capture-subsystem device
   ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround
   ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors
   ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
   ARM: dts: imx6-sabreauto: create i2cmux for i2c3
   ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b
   ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture
   ARM: dts: imx6-sabreauto: add the ADV7180 video decoder
   [media] add Omnivision 

Re: [PATCH v7 16/34] [media] add Omnivision OV5640 sensor driver

[Hans Verkuil]

On 05/25/2017 02:29 AM, Steve Longerbeam wrote:

This driver is based on ov5640_mipi.c from Freescale imx_3.10.17_1.0.0_beta
branch, modified heavily to bring forward to latest interfaces and code
cleanup.

Signed-off-by: Steve Longerbeam 


Acked-by: Hans Verkuil 

Thanks,

Hans


---
  drivers/media/i2c/Kconfig  |9 +
  drivers/media/i2c/Makefile |1 +
  drivers/media/i2c/ov5640.c | 2224 
  3 files changed, 2234 insertions(+)
  create mode 100644 drivers/media/i2c/ov5640.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index fd181c9..ff082a7 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -539,6 +539,15 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
  
+config VIDEO_OV5640

+   tristate "OmniVision OV5640 sensor support"
+   depends on OF
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the Omnivision
+ OV5640 camera sensor with a MIPI CSI-2 interface.
+
  config VIDEO_OV5645
tristate "OmniVision OV5645 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 62323ec..dc6b0c4 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -58,6 +58,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
  obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
  obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
  obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
  obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
  obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
new file mode 100644
index 000..2a032bc
--- /dev/null
+++ b/drivers/media/i2c/ov5640.c
@@ -0,0 +1,2224 @@
+/*
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* min/typical/max system clock (xclk) frequencies */
+#define OV5640_XCLK_MIN  600
+#define OV5640_XCLK_MAX 2400
+
+/*
+ * FIXME: there is no subdev API to set the MIPI CSI-2
+ * virtual channel yet, so this is hardcoded for now.
+ */
+#define OV5640_MIPI_VC 1
+
+#define OV5640_DEFAULT_SLAVE_ID 0x3c
+
+#define OV5640_REG_CHIP_ID 0x300a
+#define OV5640_REG_PAD_OUTPUT000x3019
+#define OV5640_REG_SC_PLL_CTRL00x3034
+#define OV5640_REG_SC_PLL_CTRL10x3035
+#define OV5640_REG_SC_PLL_CTRL20x3036
+#define OV5640_REG_SC_PLL_CTRL30x3037
+#define OV5640_REG_SLAVE_ID0x3100
+#define OV5640_REG_SYS_ROOT_DIVIDER0x3108
+#define OV5640_REG_AWB_R_GAIN  0x3400
+#define OV5640_REG_AWB_G_GAIN  0x3402
+#define OV5640_REG_AWB_B_GAIN  0x3404
+#define OV5640_REG_AWB_MANUAL_CTRL 0x3406
+#define OV5640_REG_AEC_PK_EXPOSURE_HI  0x3500
+#define OV5640_REG_AEC_PK_EXPOSURE_MED 0x3501
+#define OV5640_REG_AEC_PK_EXPOSURE_LO  0x3502
+#define OV5640_REG_AEC_PK_MANUAL   0x3503
+#define OV5640_REG_AEC_PK_REAL_GAIN0x350a
+#define OV5640_REG_AEC_PK_VTS  0x350c
+#define OV5640_REG_TIMING_HTS  0x380c
+#define OV5640_REG_TIMING_VTS  0x380e
+#define OV5640_REG_TIMING_TC_REG21 0x3821
+#define OV5640_REG_AEC_CTRL00  0x3a00
+#define OV5640_REG_AEC_B50_STEP0x3a08
+#define OV5640_REG_AEC_B60_STEP0x3a0a
+#define OV5640_REG_AEC_CTRL0D  0x3a0d
+#define OV5640_REG_AEC_CTRL0E  0x3a0e
+#define OV5640_REG_AEC_CTRL0F  0x3a0f
+#define OV5640_REG_AEC_CTRL10  0x3a10
+#define OV5640_REG_AEC_CTRL11  0x3a11
+#define OV5640_REG_AEC_CTRL1B  0x3a1b
+#define OV5640_REG_AEC_CTRL1E  0x3a1e
+#define OV5640_REG_AEC_CTRL1F  0x3a1f
+#define OV5640_REG_HZ5060_CTRL00   0x3c00
+#define OV5640_REG_HZ5060_CTRL01   0x3c01
+#define OV5640_REG_SIGMADELTA_CTRL0C   0x3c0c
+#define OV5640_REG_FRAME_CTRL010x4202
+#define OV5640_REG_MIPI_CTRL00 0x4800
+#define OV5640_REG_DEBUG_MODE  0x4814
+#define OV5640_REG_PRE_ISP_TEST_SET1   0x503d
+#define OV5640_REG_SDE_CTRL0   0x5580
+#define OV5640_REG_SDE_CTRL1   0x5581
+#define 

Re: [PATCH v7 15/34] add mux and video interface bridge entity functions

[Hans Verkuil]

On 05/25/2017 02:29 AM, Steve Longerbeam wrote:

From: Philipp Zabel 

Signed-off-by: Philipp Zabel 

- renamed MEDIA_ENT_F_MUX to MEDIA_ENT_F_VID_MUX

Signed-off-by: Steve Longerbeam 
---
  Documentation/media/uapi/mediactl/media-types.rst | 22 ++
  include/uapi/linux/media.h|  6 ++
  2 files changed, 28 insertions(+)

diff --git a/Documentation/media/uapi/mediactl/media-types.rst 
b/Documentation/media/uapi/mediactl/media-types.rst
index 2a5164a..47ee003 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -299,6 +299,28 @@ Types and flags used to represent the media graph elements
  received on its sink pad and outputs the statistics data on
  its source pad.
  
+-  ..  row 29

+
+   ..  _MEDIA-ENT-F-VID-MUX:
+
+   -  ``MEDIA_ENT_F_VID_MUX``
+
+   - Video multiplexer. An entity capable of multiplexing must have at
+ least two sink pads and one source pad, and must pass the video
+ frame(s) received from the active sink pad to the source pad. Video
+ frame(s) from the inactive sink pads are discarded.
+
+-  ..  row 30
+
+   ..  _MEDIA-ENT-F-VID-IF-BRIDGE:
+
+   -  ``MEDIA_ENT_F_VID_IF_BRIDGE``
+
+   - Video interface bridge. A video interface bridge entity must have at
+ least one sink pad and one source pad. It receives video frame(s) on
+ its sink pad in one bus format (HDMI, eDP, MIPI CSI-2, ...) and
+ converts them and outputs them on its source pad in another bus format
+ (eDP, MIPI CSI-2, parallel, ...).


I'm unhappy with the term 'bus format'. It's too close to 'mediabus format'.
How about calling it "bus protocol"?

Regards,

Hans

  
  ..  tabularcolumns:: |p{5.5cm}|p{12.0cm}|
  
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h

index 4890787..fac96c6 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -105,6 +105,12 @@ struct media_device_info {
  #define MEDIA_ENT_F_PROC_VIDEO_STATISTICS (MEDIA_ENT_F_BASE + 0x4006)
  
  /*

+ * Switch and bridge entitites
+ */
+#define MEDIA_ENT_F_VID_MUX(MEDIA_ENT_F_BASE + 0x5001)
+#define MEDIA_ENT_F_VID_IF_BRIDGE  (MEDIA_ENT_F_BASE + 0x5002)
+
+/*
   * Connectors
   */
  /* It is a responsibility of the entity drivers to add connectors and links */



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel