Re: [PATCH v2 3/4] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

  +struct sta2x11_vip_fh {
  +   struct v4l2_fh fh;
  +};
 
 No need to make a sta2x11_vip_fh struct, just use v4l2_fh directly. It
 doesn't add anything. In fact, it's not even used.

Thank you :)


   static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
   
struct v4l2_format *f)
   
   {
  
  -   struct video_device *dev = priv;
  -   struct sta2x11_vip *vip = video_get_drvdata(dev);
  +   struct sta2x11_vip *vip = video_drvdata(file);
  
  int interlace_lim;
  
  -   if (V4L2_PIX_FMT_UYVY != f-fmt.pix.pixelformat)
  -   return -EINVAL;
  -
  
  if (V4L2_STD_525_60  vip-std)
  
  interlace_lim = 240;
  
  else
  
  @@ -827,6 +522,8 @@ static int vidioc_try_fmt_vid_cap(struct file
  *file, void *priv, 
  return -EINVAL;
 
 No -EINVAL allowed anymore in try_fmt_vid_cap. I generally handle
 unknown field values in try_fmt_vid_cap as if FIELD_ANY was
 specified.

ok, unknown - any

  
  memcpy(vip-format, f-fmt.pix, sizeof(struct 
v4l2_pix_format));
 
 Just use an assignment: vip-format = f-fmt.pix
 

  
  memcpy(f-fmt.pix, vip-format, sizeof(struct 
v4l2_pix_format));
 
 Assignment
 

Fixed


  -
  
   static const struct v4l2_ioctl_ops vip_ioctl_ops = {
   
  .vidioc_querycap = vidioc_querycap,
  
  -   .vidioc_s_std = vidioc_s_std,
  +   /* FMT handling */
  +   .vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
  +   .vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap,
  +   .vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap,
  +   .vidioc_try_fmt_vid_cap = vidioc_try_fmt_vid_cap,
  +   /* Buffer handlers */
  +   .vidioc_reqbufs = vb2_ioctl_reqbufs,
  +   .vidioc_querybuf = vb2_ioctl_querybuf,
  +   .vidioc_qbuf = vb2_ioctl_qbuf,
  +   .vidioc_dqbuf = vb2_ioctl_dqbuf,
  +   .vidioc_create_bufs = vb2_ioctl_create_bufs,
 
 If you want to use create_bufs, then in queue_setup() you need to
 handle the fmt argument. See e.g. vivi.c for an example.

Fixed

I will send a patch v3 tomorrow
-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators

[Federico Vaga]

This patch adds support for prepare/finish callbacks in VB2 allocators.
These callback are used for buffer flushing.

Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Acked-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/v4l2-core/videobuf2-core.c | 11 +++
 include/media/videobuf2-core.h   |  7 +++
 2 file modificati, 18 inserzioni(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 4da3df6..079fa79 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -790,6 +790,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
 {
struct vb2_queue *q = vb-vb2_queue;
unsigned long flags;
+   unsigned int plane;
 
if (vb-state != VB2_BUF_STATE_ACTIVE)
return;
@@ -800,6 +801,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
dprintk(4, Done processing on buffer %d, state: %d\n,
vb-v4l2_buf.index, vb-state);
 
+   /* sync buffers */
+   for (plane = 0; plane  vb-num_planes; ++plane)
+   call_memop(q, finish, vb-planes[plane].mem_priv);
+
/* Add the buffer to the done buffers list */
spin_lock_irqsave(q-done_lock, flags);
vb-state = state;
@@ -975,9 +980,15 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct 
v4l2_buffer *b)
 static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb-vb2_queue;
+   unsigned int plane;
 
vb-state = VB2_BUF_STATE_ACTIVE;
atomic_inc(q-queued_count);
+
+   /* sync buffers */
+   for (plane = 0; plane  vb-num_planes; ++plane)
+   call_memop(q, prepare, vb-planes[plane].mem_priv);
+
q-ops-buf_queue(vb);
 }
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 8dd9b6c..2508609 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -41,6 +41,10 @@ struct vb2_fileio_data;
  *  argument to other ops in this structure
  * @put_userptr: inform the allocator that a USERPTR buffer will no longer
  *  be used
+ * @prepare:   called every time the buffer is passed from userspace to the
+ * driver, usefull for cache synchronisation, optional
+ * @finish:called every time the buffer is passed back from the driver
+ * to the userspace, also optional
  * @vaddr: return a kernel virtual address to a given memory buffer
  * associated with the passed private structure or NULL if no
  * such mapping exists
@@ -65,6 +69,9 @@ struct vb2_mem_ops {
unsigned long size, int write);
void(*put_userptr)(void *buf_priv);
 
+   void(*prepare)(void *buf_priv);
+   void(*finish)(void *buf_priv);
+
void*(*vaddr)(void *buf_priv);
void*(*cookie)(void *buf_priv);
 
-- 
1.7.11.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

The DMA streaming allocator is similar to the DMA contig but it use the
DMA streaming interface (dma_map_single, dma_unmap_single). The
allocator allocates buffers and immediately map the memory for DMA
transfer. For each buffer prepare/finish it does a DMA synchronization.

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/v4l2-core/Kconfig   |   5 +
 drivers/media/v4l2-core/Makefile  |   1 +
 drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++
 include/media/videobuf2-dma-streaming.h   |  32 
 4 file modificati, 243 inserzioni(+)
 create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c
 create mode 100644 include/media/videobuf2-dma-streaming.h

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index 0c54e19..60548a7 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG
#depends on HAS_DMA
select VIDEOBUF2_CORE
select VIDEOBUF2_MEMOPS
+
+config VIDEOBUF2_DMA_STREAMING
+   select VIDEOBUF2_CORE
+   select VIDEOBUF2_MEMOPS
+   tristate
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..0b2756f 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
 obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
 obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
 obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
+obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o
 
 ccflags-y += -I$(srctree)/drivers/media/dvb-core
 ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c 
b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
new file mode 100644
index 000..c839e05
--- /dev/null
+++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
@@ -0,0 +1,205 @@
+/*
+ * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2
+ *
+ * Copyright (C) 2012 Federico Vaga federico.v...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/module.h
+#include linux/slab.h
+#include linux/pagemap.h
+#include linux/dma-mapping.h
+
+#include media/videobuf2-core.h
+#include media/videobuf2-dma-streaming.h
+#include media/videobuf2-memops.h
+
+struct vb2_streaming_conf {
+   struct device   *dev;
+};
+struct vb2_streaming_buf {
+   struct vb2_streaming_conf   *conf;
+   void*vaddr;
+
+   dma_addr_t  dma_handle;
+
+   unsigned long   size;
+   struct vm_area_struct   *vma;
+
+   atomic_trefcount;
+   struct vb2_vmarea_handler   handler;
+};
+
+static void vb2_dma_streaming_put(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   if (atomic_dec_and_test(buf-refcount)) {
+   dma_unmap_single(buf-conf-dev, buf-dma_handle, buf-size,
+DMA_FROM_DEVICE);
+   free_pages_exact(buf-vaddr, buf-size);
+   kfree(buf);
+   }
+
+}
+
+static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size)
+{
+   struct vb2_streaming_conf *conf = alloc_ctx;
+   struct vb2_streaming_buf *buf;
+   int err;
+
+   buf = kzalloc(sizeof(struct vb2_streaming_buf), GFP_KERNEL);
+   if (!buf)
+   return ERR_PTR(-ENOMEM);
+   buf-vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA);
+   if (!buf-vaddr) {
+   err = -ENOMEM;
+   goto out;
+   }
+   buf-dma_handle = dma_map_single(conf-dev, buf-vaddr, size,
+DMA_FROM_DEVICE);
+   err = dma_mapping_error(conf-dev, buf-dma_handle);
+   if (err) {
+   dev_err(conf-dev, dma_map_single failed\n);
+
+   free_pages_exact(buf-vaddr, size);
+   buf-vaddr = NULL;
+   goto out_pages;
+   }
+   buf-conf = conf;
+   buf-size = size;
+   buf-handler.refcount = buf-refcount;
+   buf-handler.put = vb2_dma_streaming_put;
+   buf-handler.arg = buf;
+
+   atomic_inc(buf-refcount);
+   return buf;
+
+out_pages:
+   free_pages_exact(buf-vaddr, buf-size);
+out:
+   kfree(buf);
+   return ERR_PTR(err);
+}
+
+static void *vb2_dma_streaming_cookie(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   return buf-dma_handle;
+}
+
+static void *vb2_dma_streaming_vaddr(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   if (!buf)
+   return NULL;
+   return buf-vaddr

[PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180)

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
 drivers/media/pci/sta2x11/Kconfig   |2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1238 ++-
 2 file modificati, 407 inserzioni(+), 833 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..654339f 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
tristate STA2X11 VIP Video For Linux
depends on STA2X11
select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-   select VIDEOBUF_DMA_CONTIG
+   select VIDEOBUF2_DMA_STREAMING
depends on PCI  VIDEO_V4L2  VIRT_TO_BUS
help
  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index 4c10205..b9ff926 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,6 +1,7 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
  * Copyright (C) 2010   WindRiver Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -19,36 +20,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called COPYING.
  *
- * Author: Andreas Kies andreas.k...@windriver.com
- * Vlad Lungu vlad.lu...@windriver.com
- *
  */
 
 #include linux/types.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/init.h
-#include linux/vmalloc.h
-
 #include linux/videodev2.h
-
 #include linux/kmod.h
-
 #include linux/pci.h
 #include linux/interrupt.h
-#include linux/mutex.h
 #include linux/io.h
 #include linux/gpio.h
 #include linux/i2c.h
 #include linux/delay.h
 #include media/v4l2-common.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-ioctl.h
-#include media/videobuf-dma-contig.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
+#include media/videobuf2-dma-streaming.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +58,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,43 +79,20 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
+
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
 
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
@@ -129,21 +101,27 @@ struct sta2x11_vip {
struct i2c_adapter *adapter;
unsigned int register_save_area[IRQ_COUNT + SAVE_COUNT + AUX_COUNT];
struct v4l2_subdev *decoder;
-   struct v4l2_pix_format format;
-   v4l2_std_id std

[PATCH v3 4/4] adv7180: remove {query/g_/s_}ctrl

[Federico Vaga]

All drivers which use this subdevice use also the control framework.
The v4l2_subdev_core_ops operations {query/g_/s_}ctrl are useless because
device drivers will inherit controls from this subdevice.

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/i2c/adv7180.c | 3 ---
 1 file modificato, 3 rimozioni(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 45ecf8d..43bc2b9 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -402,9 +402,6 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops 
= {
 static const struct v4l2_subdev_core_ops adv7180_core_ops = {
.g_chip_ident = adv7180_g_chip_ident,
.s_std = adv7180_s_std,
-   .queryctrl = v4l2_subdev_queryctrl,
-   .g_ctrl = v4l2_subdev_g_ctrl,
-   .s_ctrl = v4l2_subdev_s_ctrl,
 };
 
 static const struct v4l2_subdev_ops adv7180_ops = {
-- 
1.7.11.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators

[Federico Vaga]

From: Marek Szyprowski m.szyprow...@samsung.com

This patch adds support for prepare/finish callbacks in VB2 allocators.
These callback are used for buffer flushing.

Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Acked-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/v4l2-core/videobuf2-core.c | 11 +++
 include/media/videobuf2-core.h   |  7 +++
 2 file modificati, 18 inserzioni(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 4da3df6..079fa79 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -790,6 +790,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
 {
struct vb2_queue *q = vb-vb2_queue;
unsigned long flags;
+   unsigned int plane;
 
if (vb-state != VB2_BUF_STATE_ACTIVE)
return;
@@ -800,6 +801,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
dprintk(4, Done processing on buffer %d, state: %d\n,
vb-v4l2_buf.index, vb-state);
 
+   /* sync buffers */
+   for (plane = 0; plane  vb-num_planes; ++plane)
+   call_memop(q, finish, vb-planes[plane].mem_priv);
+
/* Add the buffer to the done buffers list */
spin_lock_irqsave(q-done_lock, flags);
vb-state = state;
@@ -975,9 +980,15 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct 
v4l2_buffer *b)
 static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb-vb2_queue;
+   unsigned int plane;
 
vb-state = VB2_BUF_STATE_ACTIVE;
atomic_inc(q-queued_count);
+
+   /* sync buffers */
+   for (plane = 0; plane  vb-num_planes; ++plane)
+   call_memop(q, prepare, vb-planes[plane].mem_priv);
+
q-ops-buf_queue(vb);
 }
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 8dd9b6c..2508609 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -41,6 +41,10 @@ struct vb2_fileio_data;
  *  argument to other ops in this structure
  * @put_userptr: inform the allocator that a USERPTR buffer will no longer
  *  be used
+ * @prepare:   called every time the buffer is passed from userspace to the
+ * driver, usefull for cache synchronisation, optional
+ * @finish:called every time the buffer is passed back from the driver
+ * to the userspace, also optional
  * @vaddr: return a kernel virtual address to a given memory buffer
  * associated with the passed private structure or NULL if no
  * such mapping exists
@@ -65,6 +69,9 @@ struct vb2_mem_ops {
unsigned long size, int write);
void(*put_userptr)(void *buf_priv);
 
+   void(*prepare)(void *buf_priv);
+   void(*finish)(void *buf_priv);
+
void*(*vaddr)(void *buf_priv);
void*(*cookie)(void *buf_priv);
 
-- 
1.7.11.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] base/core.c: improve comment of the function device_find_child()

[Federico Vaga]

On Friday 12 April 2013 14:51:25 Greg Kroah-Hartman wrote:
 On Fri, Apr 12, 2013 at 01:59:32PM +0200, Federico Vaga wrote:
  Signed-off-by: Federico Vaga federico.v...@gmail.com
  ---
  
   drivers/base/core.c | 4 
   1 file changed, 4 insertions(+)
  
  diff --git a/drivers/base/core.c b/drivers/base/core.c
  index 016312437..eb0c6ea 100644
  --- a/drivers/base/core.c
  +++ b/drivers/base/core.c
  @@ -1372,6 +1372,10 @@ int device_for_each_child(struct device *parent,
  void *data, 
* if it does.  If the callback returns non-zero and a reference to the
* current device can be obtained, this function will return to the
caller
* and not iterate over any more devices.
  
  + *
  + * NOTE: internally, the function does get_device() on the retrieved
  child. + * It is duty of the caller performing a put_device() on the
  retrieved + * child device when the caller finishes to work on it.
  
*/
 
 Why not just use the same wording that class_find_device() has, which is
 simpler and easier to understand (IMHO)?

Mh, yes. You are right. I'll send a new patch

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: drivers/base/core.c: about device_find_child() function

[Federico Vaga]

Hi Lars,
 
 Considering that there seems to be a common pattern here where the caller
 only wants to know if the device exists, but is not really interested in the
 device itself, how about adding a helper function for this?

It was my first thought when I opened this thread. But now I'm convinced that 
device_for_each_child() is the best choice (maybe I'm wrong).

device_for_each_child() allow you to perform an operation of each child of a 
device: look for a specific child, do something on every children, retrieve a 
specific group of children, etc.

I think that an helper for this case will be a perfect duplication of 
device_for_each_child() with a different name and comment (borrowed from 
device_find_child()). Maybe, a macro to assign a different name to this 
function:

/*
 * [...]
 * The callback should return 0 if the device doesn't match and non-zero
 * if it does
 * [...]
 */
#define device_has_child(parent, data, match) device_for_each_child(parent, 
data, match)

But, is it useful? It can be a suggestion to developers to prefer 
device_for_each_child() instead of device_find_child() when (s)he is 
interested only about the child existence. So, (s)he does not need to 
put_device(). But I think that is not a strong argumentation, and later in 
time someone will propose his own special use of device_for_each_child().

I think that device_for_each_child() is generic enough to cover this problem.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: drivers/base/core.c: about device_find_child() function

[Federico Vaga]

Thank you very much Greg
 
  I did not study serial_core, I was looking only for device_find_child().
  Probably I'm missing something. Anyway, here what does not convice me:
  
  (line number on next-20130412)
  serial_core.c:2003
  
  tty_dev = device_find_child(uport-dev, match, serial_match_port);
  if (!uport-suspended  device_may_wakeup(tty_dev)) {
  
  if (uport-irq_wake) {
  
  disable_irq_wake(uport-irq);
  uport-irq_wake = 0;
  
  }
  
  +   put_device(tty_dev);
  
  mutex_unlock(port-mutex);
  return 0;
  
  }
  
  +   put_device(tty_dev);
  
  uport-suspended = 0;
  
  serial_core:1936
  
  tty_dev = device_find_child(uport-dev, match, serial_match_port);
  if (device_may_wakeup(tty_dev)) {
  
  if (!enable_irq_wake(uport-irq))
  
  uport-irq_wake = 1;
  
  put_device(tty_dev);
  mutex_unlock(port-mutex);
  return 0;
  
  }
  
  +   put_device(tty_dev);
 
 That looks like a good patch, care to properly send it, (after you test
 it first of course), so that we can apply it?

Yes, I can do it and test it. I hope to find the time for a test in these 
days.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2] base/core.c: improve comment of the function device_find_child()

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/base/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 016312437..3c8512f 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1372,6 +1372,8 @@ int device_for_each_child(struct device *parent, void 
*data,
  * if it does.  If the callback returns non-zero and a reference to the
  * current device can be obtained, this function will return to the caller
  * and not iterate over any more devices.
+ *
+ * NOTE: you will need to drop the reference with put_device() after use.
  */
 struct device *device_find_child(struct device *parent, void *data,
 int (*match)(struct device *dev, void *data))
-- 
1.8.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] serial_core.c: add put_device() after device_find_child()

[Federico Vaga]

The serial core uses device_find_child() but does not drop the reference to
the retrieved child after using it. This patch add the missing put_device().

Signed-off-by: Federico Vaga federico.v...@gmail.com
---

What I have done to test this issue.

I used a machine with an AMBA PL011 serial driver. I tested the patch on
next-20120408 because the last branch [next-20120415] does not boot on this
board.

For test purpose, I added some pr_info() messages to print the refcount
after device_find_child() (lines: 1937,2009), and after put_device()
(lines: 1947, 2021).


Boot the machine *without* put_device(). Then:

echo reboot  /sys/power/disk
echo disk  /sys/power/state
[   87.058575] uart_suspend_port:1937 refcount 4
[   87.058582] uart_suspend_port:1947 refcount 4
[   87.098083] uart_resume_port:2009refcount 5
[   87.098088] uart_resume_port:2021 refcount 5

echo disk  /sys/power/state
[  103.055574] uart_suspend_port:1937 refcount 6
[  103.055580] uart_suspend_port:1947 refcount 6
[  103.095322] uart_resume_port:2009 refcount 7
[  103.095327] uart_resume_port:2021 refcount 7

echo disk  /sys/power/state
[  252.459580] uart_suspend_port:1937 refcount 8
[  252.459586] uart_suspend_port:1947 refcount 8
[  252.499611] uart_resume_port:2009 refcount 9
[  252.499616] uart_resume_port:2021 refcount 9

The refcount continuously increased.


Boot the machine *with* this patch. Then:

echo reboot  /sys/power/disk
echo disk  /sys/power/state
[  159.333559] uart_suspend_port:1937 refcount 4
[  159.333566] uart_suspend_port:1947 refcount 3
[  159.372751] uart_resume_port:2009 refcount 4
[  159.372755] uart_resume_port:2021 refcount 3

echo disk  /sys/power/state
[  185.713614] uart_suspend_port:1937 refcount 4
[  185.713621] uart_suspend_port:1947 refcount 3
[  185.752935] uart_resume_port:2009 refcount 4
[  185.752940] uart_resume_port:2021 refcount 3

echo disk  /sys/power/state
[  207.458584] uart_suspend_port:1937 refcount 4
[  207.458591] uart_suspend_port:1947 refcount 3
[  207.498598] uart_resume_port:2009 refcount 4
[  207.498605] uart_resume_port:2021 refcount 3

The refcount correctly handled.

 drivers/tty/serial/serial_core.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 19cc749..f87dbfd 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1941,6 +1941,8 @@ int uart_suspend_port(struct uart_driver *drv, struct 
uart_port *uport)
mutex_unlock(port-mutex);
return 0;
}
+   put_device(tty_dev);
+
if (console_suspend_enabled || !uart_console(uport))
uport-suspended = 1;
 
@@ -2006,9 +2008,11 @@ int uart_resume_port(struct uart_driver *drv, struct 
uart_port *uport)
disable_irq_wake(uport-irq);
uport-irq_wake = 0;
}
+   put_device(tty_dev);
mutex_unlock(port-mutex);
return 0;
}
+   put_device(tty_dev);
uport-suspended = 0;
 
/*
-- 
1.8.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC PATCH] sparc/kernel/vio.c: add put_device() after device_find_child()

[Federico Vaga]

The vio_remove() function uses device_find_child() but it does not drop
the reference of the retrieved child.

Signed-off-by: Federico Vaga federico.v...@gmail.com
---

I do not have a SPARC system (and I do not know it), so I cannot test this
patch. Please test it.
If I'm right, the device_unregister() does not work properly because,
device_find_child() did get_device() on dev. In essence, the release method
associated to the device will never be invoked because there is a reference
to the device that is not dropped.

 arch/sparc/kernel/vio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c
index 3e244f3..8647fcc 100644
--- a/arch/sparc/kernel/vio.c
+++ b/arch/sparc/kernel/vio.c
@@ -342,6 +342,7 @@ static void vio_remove(struct mdesc_handle *hp, u64 node)
printk(KERN_INFO VIO: Removing device %s\n, dev_name(dev));
 
device_unregister(dev);
+   put_device(dev);
}
 }
 
-- 
1.8.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

drivers/base/core.c: about device_find_child() function

[Federico Vaga]

Hello,

I'm using the function device_find_child() [drivers/base/core.c] to retrieve 
a specific child of a device. I see that this function invokes 
get_device(child) when a child matches. I think that this function must 
return the reference to the child device without getting it.

The function's comment does not explicitly talk about an increment of the 
refcount of the device. So, man 9 device_find_child and various derivative 
webpages do not talk about this. The developer is not correctly informed 
about this function, unless (s)he looks at the source code.

I see that users of this function, usually, immediately do put_device() after 
the call to device_find_child(), so it is not expected that a 
device_find_child() does a get_device() on the found child.


   Immediately does put_device():
 drivers/firewire/core-device.c
 drivers/rpmsg/virtio_rpmsg_bus.c
 drivers/s390/kvm/kvm_virtio.c

   They effectively need a get_device():
 drivers/net/bluetooth/hci_sysfs.c
 drivers/net/dsa/dsa.c

   Maybe bugged because they do not do put_device():
 drivers/media/platform/s5p-mfc/s5p_mfc.c
 drivers/tty/serial/serial_core.c
   Probably I'm wrong on this and I do not find the associated put_device()


I should propose the following solution:

* Deprecate the device_find_child() function

* Create the following functions

   struct device *device_search_child(struct device *parent, void *data,
int (*match)(struct device *dev, void *data))
   {
struct klist_iter i;
struct device *child;

if (!parent)
return NULL;

klist_iter_init(parent-p-klist_children, i);
while ((child = next_device(i)))
if (match(child, data))
break;
klist_iter_exit(i);
return child;
  }

  struct device *get_device_child(struct device *parent, void *data,
int (*match)(struct device *dev, void *data))
  {
struct device *child;

child = device_search_child(parent, data, match);
if (child)
  get_device(child);
return child;
  }


In this way, when a driver needs to find and get a child, it uses 
get_device_child() and , when it finishes its duty, it uses put_device(). In 
this situation, the developer use a pair of function with a symmetric names: 
get_device_child() and put_device().

If the driver do not need to get_device() on a child device, it simply does a 
device_search_child() to retrieve a pointer.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] base/core.c: improve comment of the function device_find_child()

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/base/core.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 016312437..eb0c6ea 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1372,6 +1372,10 @@ int device_for_each_child(struct device *parent, void 
*data,
  * if it does.  If the callback returns non-zero and a reference to the
  * current device can be obtained, this function will return to the caller
  * and not iterate over any more devices.
+ *
+ * NOTE: internally, the function does get_device() on the retrieved child.
+ * It is duty of the caller performing a put_device() on the retrieved
+ * child device when the caller finishes to work on it.
  */
 struct device *device_find_child(struct device *parent, void *data,
 int (*match)(struct device *dev, void *data))
-- 
1.8.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: drivers/base/core.c: about device_find_child() function

[Federico Vaga]

On Thursday 11 April 2013 06:48:44 Greg Kroah-Hartman wrote:
 On Thu, Apr 11, 2013 at 01:52:36PM +0200, Federico Vaga wrote:
  Hello,
  
  I'm using the function device_find_child() [drivers/base/core.c] to
  retrieve a specific child of a device. I see that this function invokes
  get_device(child) when a child matches. I think that this function must
  return the reference to the child device without getting it.
 
 No, it should not, otherwise the device could disappear at any moment,
 and the caller who had the handle, would now have a stale pointer.

I know, I am aware of this, but sometimes it *seems* that it does not
matter. (argument later [**])

  The function's comment does not explicitly talk about an increment of the
  refcount of the device. So, man 9 device_find_child and various
  derivative webpages do not talk about this. The developer is not
  correctly informed about this function, unless (s)he looks at the source
  code.
 
 You are right, I would gladly take a patch adding that comment to the
 function, can you send me one?

Already sent.

  I see that users of this function, usually, immediately do put_device()
  after the call to device_find_child(), so it is not expected that a
  device_find_child() does a get_device() on the found child.
  
 Immediately does put_device():
   drivers/firewire/core-device.c
   drivers/rpmsg/virtio_rpmsg_bus.c
   drivers/s390/kvm/kvm_virtio.c
 
 That's correct.

[**] (argumentation based, obviously, on my limited understanding)

These drivers work like this:

child = device_find_child(parent, data, match_function);
if (child) {
put_device(child);  
do something unrelated with child 
}

In these cases we do not need to get_device(). But we need to know if there 
is a child that match a rule. It can also disapper after the
put_device(child) but the driver continues on its way because it does not
use the child. For example virtio_rpmsg_bus.c:

/* make sure a similar channel doesn't already exist */
tmp = device_find_child(dev, chinfo, rpmsg_channel_match);
if (tmp) {
/* decrement the matched device's refcount back */
put_device(tmp);
dev_err(dev, channel %s:%x:%x already exist\n,
chinfo-name, chinfo-src, chinfo-dst);
return NULL;
}

In this case, it does not matter to get_device(), the driver is interested
only on the child existence, it does not use the child.
Maybe it is wrong a driver that works like this. I mean, why retrieve a
child if you do not want to use it? This can be implemented also with
the function device_for_each_child() and return an error if a channel already
exist (-EBUSY).

The same argumentation for firewire/core-device.c:

revived_dev = device_find_child(card-device,
device, lookup_existing_device);
if (revived_dev) {
put_device(revived_dev);
fw_device_release(device-device);

return;
}

This can be done with device_for_each_child() because it does not use the
the retrieved device. The function fw_device_release() can be done in the
function lookup_existing_device() when it finds the child. 

Also the driver s390/kvm/kvm_virtio.c:

/* device already exists */
dev = device_find_child(kvm_root, d, match_desc);
if (dev) {
/* XXX check for hotplug remove */
put_device(dev);
continue;
}

Probably here, in a future patch XXX will be replaced with some code,
so it is correct to use device_find_child().

Now I'm thinking that device_for_each_child() is better in the above
cases. Am I wrong? Am I missing something? Which is the cleaner solution?


Anyway, my suggestion was more about the function name rather than its 
content (again, I am aware that the refcount must be increased if I
work on the retrieved child). Because the verb 'find' does not imply the
verb 'get', so, a function named device_find_child() should not do
get_device() because it may not obvious for the reader. I think that
the name should be something like get_device_child(), get_child_device(),
get_child(), and for symmetry the user will do put_device() on the
retrived child. But I understand that for legacy reason it could be
better to continue use device_find_child()


 Maybe bugged because they do not do put_device():
   drivers/media/platform/s5p-mfc/s5p_mfc.c

Fixed with commit 6e83e6e25eb49dc57a69b3f8ecc1e764c9775101. I did not see
it before, sorry.

   drivers/tty/serial/serial_core.c
 Probably I'm wrong on this and I do not find the associated 
put_device()

 I think the serial core is correct, but I'll audit it, the media one
 should be fixed as well.

I did not study serial_core, I was looking only

Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

On Tuesday 04 December 2012 14:15:15 Mauro Carvalho Chehab wrote:
 Em 24-09-2012 07:58, Federico Vaga escreveu:
  This patch re-write the driver and use the videobuf2
  interface instead of the old videobuf. Moreover, it uses also
  the control framework which allows the driver to inherit
  controls from its subdevice (ADV7180)
  
  Signed-off-by: Federico Vaga federico.v...@gmail.com
  Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
 
  [..]
  
/*

 * This is the driver for the STA2x11 Video Input Port.
 *
  
  + * Copyright (C) 2012   ST Microelectronics
  
 * Copyright (C) 2010   WindRiver Systems, Inc.
 *
 * This program is free software; you can redistribute it and/or modify
 it
  
  @@ -19,36 +20,30 @@
  
 * The full GNU General Public License is included in this distribution
 in
 * the file called COPYING.
 *
  
  - * Author: Andreas Kies andreas.k...@windriver.com
  - * Vlad Lungu vlad.lu...@windriver.com
 
 Why are you dropping those authorship data?
 
 Ok, it is clear to me that most of the code there got rewritten, and,
 while IANAL, I think they still have some copyrights on it.
 
 So, if you're willing to do that, you need to get authors ack
 on such patch.

I re-write the driver, and also the first version of the driver has many 
modification made by me, many bug fix, style review, remove useless code.
The first time I didn't add myself as author because the logic of the driver 
did not change. This time, plus the old change I think there is nothing of the 
original driver because I rewrite it from the hardware manual. Practically, It 
is a new driver for the same device.

Anyway I will try to contact the original authors for the acked-by.

MODULE_DESCRIPTION(STA2X11 Video Input Port driver);
  
  -MODULE_AUTHOR(Wind River);
 
 Same note applies here: we need Wind River's ack on that to drop it.

I will try also for this. But I think that this is not a windriver driver 
because I re-wrote it from the hardware manual. I used the old driver because 
I thought that it was better than propose a patch that remove the old driver 
and add my driver.
I did not remove the 2010 Copyright from windriver, because they did the job, 
but this work was paid by ST (copyright 2012) and made completely by me.

Is my thinking wrong?

Just a question for the future so I avoid to redo the same error. If I re-
wrote most of a driver I cannot change the authorship automatically without 
the acked-by of the previous author. If I ask to the previous author and he 
does not give me the acked-by (or he is unreachable, he change email address), 
then the driver is written by me but the author is someone else? Right? So, it 
is better if I propose a patch which remove a driver and a patch which add my 
driver?

Thank you

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

Thank you Mauro for the good explanation

 Yeah, there are many changes there that justifies adding you at its
 authorship, and that's ok. Also, anyone saying the size of your patch
 will recognize your and ST efforts to improve the driver.
 
 However, as some parts of the code were preserved, dropping the old
 authors doesn't sound right (and can even be illegal, in the light
 of the GPL license). It would be ok, though, if you would be
 changing it to something like:
 
   Copyright (c) 2010 by ...
 or
   Original driver from ...

Ok, I understand. I will write something like this.
 * Copyright (C) 2012   ST Microelectronics
 *  author: Federico Vaga federico.v...@gmail.com
 * Copyright (C) 2010   WindRiver Systems, Inc.
 *  authors: Andreas Kies andreas.k...@windriver.com
 *   Vlad Lungu vlad.lu...@windriver.com


 The only way of not preserving the original authors here were if you
 start from scratch, without looking at the original code (and you can
 somehow, be able to proof it), otherwise, the code will be fit as a
 derivative work, in the light of GPL, and should be preserving the
 original authorship.
 
 Something started from scratch like that will hardly be accepted upstream,
 as regressions will likely be introduced, and previously supported
 hardware/features may be lost in the process.

I understand
 
 Of course the original author can abdicate to his rights of keeping his
 name on it. Yet, even if he opt/accept to not keep his name explicitly
 there, his copyrights are preserved, with the help of the git history.
 
 That's said, no single kernel developer/company has full copyrights on
 any part of the Kernel, as their code are based on someone else's work.
 For example, all Kernel drivers depend on drivers/base, with in turn,
 depends on memory management, generic helper functions, arch code, etc.

yeah I know :)

 So, IMHO, there's not much point on dropping authorship messages.

So the MODULE_AUTHOR will be Windriver forever until they drop authorship? 
Also if in the next future 0 windriver lines will be in the code?

(general talking, not about this driver)
If I do git blame on a driver with MODULE_AUTHOR(Mr. X); but only the 
MODULE_AUTHOR line is from Mr X; there is not an automatically system which 
remove the MODULE_AUTHOR? Ok, probably it was the original author of the code 
and the comment header with the copyright history gives to Mr X all the 
honours, but there is nothing from him in the code. It is not better to remove 
MODULE_AUTHOR or replace it with who wrotes most of the code?
I know that this is not the best place to talk about this, just a little 
curiosity

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

On Tuesday 04 December 2012 14:04:22 Mauro Carvalho Chehab wrote:
 Em 24-09-2012 09:44, Marek Szyprowski escreveu:
  Hello,
  
  On Monday, September 24, 2012 12:59 PM Federico Vaga wrote:
  The DMA streaming allocator is similar to the DMA contig but it use the
  DMA streaming interface (dma_map_single, dma_unmap_single). The
  allocator allocates buffers and immediately map the memory for DMA
  transfer. For each buffer prepare/finish it does a DMA synchronization.
 
 Hmm.. the explanation didn't convince me, e. g.:
   1) why is it needed;

This allocator is needed because some device (like STA2X11 VIP) cannot work 
with DMA sg or DMA coherent. Some other device (like the one used by Jonathan 
when he proposes vb2-dma-nc allocator) can obtain much better performance with 
DMA streaming than coherent.

   2) why vb2-dma-config can't be patched to use dma_map_single
 (eventually using a different vb2_io_modes bit?);

I did not modify vb2-dma-contig because I was thinking that each DMA memory 
allocator should reflect a DMA API.

   3) what are the usecases for it.
 
 Could you please detail it? Without that, one that would be needing to
 write a driver will have serious doubts about what would be the right
 driver for its usage. Also, please document it at the driver itself.

I did not write all this details because the reasons to use vb2-dma-contig, 
vb2-dma-sg or vb2-dma-streaming are the same reasons because someone choose 
SG, coherent or streaming API. This is already documented in the DMA-*.txt 
files, so I did not rewrite it to avoid duplication.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

  Ok, I understand. I will write something like this.
  
* Copyright (C) 2012   ST Microelectronics
*  author: Federico Vaga federico.v...@gmail.com
* Copyright (C) 2010   WindRiver Systems, Inc.
*  authors: Andreas Kies andreas.k...@windriver.com
*   Vlad Lungu vlad.lu...@windriver.com
 
 Sounds perfect to me.

I will answer to this with a patch

 As you said, the best place to discuss about it is likely at LKML.
 [...]
 Btw, this is why it is called git blame, and not git authorship:
 it is a tool to identify who was the last one that modified the code.
 Its main usage is to identify who might have introduced a bug on the
 code.

I know I know, it was just a stupid example to expose the problem that I have 
in my mind. I know that it is very difficult (impossible?) to assign the 
authorship of a single line, and git blame it is not the tool to do this :)

I think you understand what I mean despite the stupid example

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

 Not sure if you got my point: the main point of removing MODULE_AUTHOR
 and other copyright stuff is that such patch may easily be doing something
 that could be considered a copyright violation, being bad not only to
 the affected driver, but to the entire Kernel.
 
 So, we need to handle it with due care. Getting other authors's
 acks on such patch seems to be the only safe way of doing that.

Yes I got the point.

Thank you

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v6 1/2] sta2x11_vip: convert to videobuf2, control framework, file handler

[Federico Vaga]

This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180). Finally the driver does not
implement custom file operation but it uses the generic ones from
videobuf2 and v4l2_fh

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
 drivers/media/pci/sta2x11/Kconfig   |2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1073 +--
 2 file modificati, 434 inserzioni(+), 641 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..a94ccad 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
tristate STA2X11 VIP Video For Linux
depends on STA2X11
select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-   select VIDEOBUF_DMA_CONTIG
+   select VIDEOBUF2_DMA_CONTIG
depends on PCI  VIDEO_V4L2  VIRT_TO_BUS
help
  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index ed1337a..b4521b5 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,7 +1,11 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
+ * author: Federico Vaga federico.v...@gmail.com
  * Copyright (C) 2010   WindRiver Systems, Inc.
+ * authors: Andreas Kies andreas.k...@windriver.com
+ *  Vlad Lungu   vlad.lu...@windriver.com
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -19,36 +23,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called COPYING.
  *
- * Author: Andreas Kies andreas.k...@windriver.com
- * Vlad Lungu vlad.lu...@windriver.com
- *
  */
 
 #include linux/types.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/init.h
-#include linux/vmalloc.h
-
 #include linux/videodev2.h
-
 #include linux/kmod.h
-
 #include linux/pci.h
 #include linux/interrupt.h
-#include linux/mutex.h
 #include linux/io.h
 #include linux/gpio.h
 #include linux/i2c.h
 #include linux/delay.h
 #include media/v4l2-common.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-ioctl.h
-#include media/videobuf-dma-contig.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
+#include media/videobuf2-dma-contig.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +61,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,13 +82,21 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
+
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
+
 /**
  * struct sta2x11_vip - All internal data for one instance of device
  * @v4l2_dev: device registered in v4l layer
@@ -99,29 +105,26 @@
  * @adapter: contains I2C adapter information
  * @register_save_area: All relevant register are saved here during suspend
  * @decoder: contains information about video DAC
+ * @ctrl_hdl: handler for control framework
  * @format: pixel format, fixed UYVY
  * @std: video standard (e.g. PAL/NTSC)
  * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
  * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
  * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
+ * @alloc_ctx: context for videobuf2
+ * @vb_vidq: queue maintained by videobuf2 layer
+ * @buffer_list: list of buffer in use
+ * @sequence: sequence number of acquired buffer
+ * @active: current active buffer
+ * @lock: used in videobuf2 callback
  * @tcount: Number of top frames
  * @bcount: Number of bottom

Re: [PATCH v6 1/2] sta2x11_vip: convert to videobuf2, control framework, file handler

[Federico Vaga]

Hi Hans,

thank you very much for your review and your patience.

 OK, I'm going to give this my Acked-by, but I really wish you would
 have split this up into smaller changes. It's hard to review since
 you have made so many changes in this one patch. Even though I'm
 giving my ack, Mauro might decide against it, so if you have time
 to spread out the changes in multiple patches, then please do so.

I tried to do smaller patch but there is always some incoherent part 
and the driver cannot work without all the patches. I should write 
some fake patches to make a coherent series.
I reduce the size of the patch since v4/5; I leaved 
unchanged some code/comments to simplify the patch.

 So, given the fact that this changes just a single driver not
 commonly used in existing deployments, assuming that you have
 tested the changes (you did that, right? Just checking...), that
 these are really useful improvements, and that I reviewed the code
 (as well as I could) and didn't see any problems, I'm giving my ack
 anyway:

Tested every time I sent a patch

 Acked-by: Hans Verkuil hans.verk...@cisco.com

Thank you again

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v6 1/2] sta2x11_vip: convert to videobuf2, control framework, file handler

[Federico Vaga]

This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180). Finally the driver does not
implement custom file operation but it uses the generic ones from
videobuf2 and v4l2_fh

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
 drivers/media/pci/sta2x11/Kconfig   |2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1071 +--
 2 file modificati, 432 inserzioni(+), 641 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..a94ccad 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
tristate STA2X11 VIP Video For Linux
depends on STA2X11
select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-   select VIDEOBUF_DMA_CONTIG
+   select VIDEOBUF2_DMA_CONTIG
depends on PCI  VIDEO_V4L2  VIRT_TO_BUS
help
  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index ed1337a..834ac55 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,7 +1,11 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
+ * author: Federico Vaga federico.v...@gmail.com
  * Copyright (C) 2010   WindRiver Systems, Inc.
+ * authors: Andreas Kies andreas.k...@windriver.com
+ *  Vlad Lungu   vlad.lu...@windriver.com
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -19,36 +23,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called COPYING.
  *
- * Author: Andreas Kies andreas.k...@windriver.com
- * Vlad Lungu vlad.lu...@windriver.com
- *
  */
 
 #include linux/types.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/init.h
-#include linux/vmalloc.h
-
 #include linux/videodev2.h
-
 #include linux/kmod.h
-
 #include linux/pci.h
 #include linux/interrupt.h
-#include linux/mutex.h
 #include linux/io.h
 #include linux/gpio.h
 #include linux/i2c.h
 #include linux/delay.h
 #include media/v4l2-common.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-ioctl.h
-#include media/videobuf-dma-contig.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
+#include media/videobuf2-dma-contig.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +61,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,13 +82,21 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
+
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
+
 /**
  * struct sta2x11_vip - All internal data for one instance of device
  * @v4l2_dev: device registered in v4l layer
@@ -99,29 +105,26 @@
  * @adapter: contains I2C adapter information
  * @register_save_area: All relevant register are saved here during suspend
  * @decoder: contains information about video DAC
+ * @ctrl_hdl: handler for control framework
  * @format: pixel format, fixed UYVY
  * @std: video standard (e.g. PAL/NTSC)
  * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
  * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
  * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
+ * @alloc_ctx: context for videobuf2
+ * @vb_vidq: queue maintained by videobuf2 layer
+ * @buffer_list: list of buffer in use
+ * @sequence: sequence number of acquired buffer
+ * @active: current active buffer
+ * @lock: used in videobuf2 callback
  * @tcount: Number of top frames
  * @bcount: Number of bottom

[PATCH v6 2/2] adv7180: remove {query/g_/s_}ctrl

[Federico Vaga]

All drivers which use this subdevice use also the control framework.
The v4l2_subdev_core_ops operations {query/g_/s_}ctrl are useless because
device drivers will inherit controls from this subdevice.

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/i2c/adv7180.c | 3 ---
 1 file modificato, 3 rimozioni(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 45ecf8d..43bc2b9 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -402,9 +402,6 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops 
= {
 static const struct v4l2_subdev_core_ops adv7180_core_ops = {
.g_chip_ident = adv7180_g_chip_ident,
.s_std = adv7180_s_std,
-   .queryctrl = v4l2_subdev_queryctrl,
-   .g_ctrl = v4l2_subdev_g_ctrl,
-   .s_ctrl = v4l2_subdev_s_ctrl,
 };
 
 static const struct v4l2_subdev_ops adv7180_ops = {
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 3/4] sta2x11_vip: convert to videobuf2 and control framework

[federico . vaga]

From: Federico Vaga federico.v...@gmail.com

This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180)

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
 drivers/media/pci/sta2x11/Kconfig   |2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1235 ++-
 2 file modificati, 411 inserzioni(+), 826 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..654339f 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
tristate STA2X11 VIP Video For Linux
depends on STA2X11
select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-   select VIDEOBUF_DMA_CONTIG
+   select VIDEOBUF2_DMA_STREAMING
depends on PCI  VIDEO_V4L2  VIRT_TO_BUS
help
  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index 4c10205..f423039 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,6 +1,7 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
  * Copyright (C) 2010   WindRiver Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -19,36 +20,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called COPYING.
  *
- * Author: Andreas Kies andreas.k...@windriver.com
- * Vlad Lungu vlad.lu...@windriver.com
- *
  */
 
 #include linux/types.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/init.h
-#include linux/vmalloc.h
-
 #include linux/videodev2.h
-
 #include linux/kmod.h
-
 #include linux/pci.h
 #include linux/interrupt.h
-#include linux/mutex.h
 #include linux/io.h
 #include linux/gpio.h
 #include linux/i2c.h
 #include linux/delay.h
 #include media/v4l2-common.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-ioctl.h
-#include media/videobuf-dma-contig.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
+#include media/videobuf2-dma-streaming.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +58,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,44 +79,24 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
 
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
+
+struct sta2x11_vip_fh {
+   struct v4l2_fh fh;
+};
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
struct video_device *video_dev;
@@ -129,21 +104,27 @@ struct sta2x11_vip {
struct i2c_adapter *adapter;
unsigned int register_save_area

[PATCH 1/4] v4l: vb2: add prepare/finish callbacks to allocators

[Federico Vaga]

This patch adds support for prepare/finish callbacks in VB2 allocators.
These callback are used for buffer flushing.

Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Acked-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/v4l2-core/videobuf2-core.c | 11 +++
 include/media/videobuf2-core.h   |  7 +++
 2 file modificati, 18 inserzioni(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 4da3df6..079fa79 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -790,6 +790,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
 {
struct vb2_queue *q = vb-vb2_queue;
unsigned long flags;
+   unsigned int plane;
 
if (vb-state != VB2_BUF_STATE_ACTIVE)
return;
@@ -800,6 +801,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
dprintk(4, Done processing on buffer %d, state: %d\n,
vb-v4l2_buf.index, vb-state);
 
+   /* sync buffers */
+   for (plane = 0; plane  vb-num_planes; ++plane)
+   call_memop(q, finish, vb-planes[plane].mem_priv);
+
/* Add the buffer to the done buffers list */
spin_lock_irqsave(q-done_lock, flags);
vb-state = state;
@@ -975,9 +980,15 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct 
v4l2_buffer *b)
 static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb-vb2_queue;
+   unsigned int plane;
 
vb-state = VB2_BUF_STATE_ACTIVE;
atomic_inc(q-queued_count);
+
+   /* sync buffers */
+   for (plane = 0; plane  vb-num_planes; ++plane)
+   call_memop(q, prepare, vb-planes[plane].mem_priv);
+
q-ops-buf_queue(vb);
 }
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 8dd9b6c..f374f99 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -41,6 +41,10 @@ struct vb2_fileio_data;
  *  argument to other ops in this structure
  * @put_userptr: inform the allocator that a USERPTR buffer will no longer
  *  be used
+ * @prepare:   called everytime the buffer is passed from userspace to the
+ * driver, usefull for cache synchronisation, optional
+ * @finish:called everytime the buffer is passed back from the driver
+ * to the userspace, also optional
  * @vaddr: return a kernel virtual address to a given memory buffer
  * associated with the passed private structure or NULL if no
  * such mapping exists
@@ -65,6 +69,9 @@ struct vb2_mem_ops {
unsigned long size, int write);
void(*put_userptr)(void *buf_priv);
 
+   void(*prepare)(void *buf_priv);
+   void(*finish)(void *buf_priv);
+
void*(*vaddr)(void *buf_priv);
void*(*cookie)(void *buf_priv);
 
-- 
1.7.11.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

The DMA streaming allocator is similar to the DMA contig but it use the
DMA streaming interface (dma_map_single, dma_unmap_single). The
allocator allocates buffers and immediately map the memory for DMA
transfer. For each buffer prepare/finish it does a DMA synchronization.

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/v4l2-core/Kconfig   |   5 +
 drivers/media/v4l2-core/Makefile  |   1 +
 drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++
 include/media/videobuf2-dma-streaming.h   |  32 
 4 file modificati, 243 inserzioni(+)
 create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c
 create mode 100644 include/media/videobuf2-dma-streaming.h

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index 0c54e19..60548a7 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG
#depends on HAS_DMA
select VIDEOBUF2_CORE
select VIDEOBUF2_MEMOPS
+
+config VIDEOBUF2_DMA_STREAMING
+   select VIDEOBUF2_CORE
+   select VIDEOBUF2_MEMOPS
+   tristate
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..0b2756f 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
 obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
 obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
 obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
+obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o
 
 ccflags-y += -I$(srctree)/drivers/media/dvb-core
 ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c 
b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
new file mode 100644
index 000..16d5569
--- /dev/null
+++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
@@ -0,0 +1,205 @@
+/*
+ * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2
+ *
+ * Copyright (C) 2012 Federico Vaga federico.v...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/module.h
+#include linux/slab.h
+#include linux/pagemap.h
+#include linux/dma-mapping.h
+
+#include media/videobuf2-core.h
+#include media/videobuf2-dma-streaming.h
+#include media/videobuf2-memops.h
+
+struct vb2_streaming_conf {
+   struct device   *dev;
+};
+struct vb2_streaming_buf {
+   struct vb2_streaming_conf   *conf;
+   void*vaddr;
+
+   dma_addr_t  dma_handle;
+
+   unsigned long   size;
+   struct vm_area_struct   *vma;
+
+   atomic_trefcount;
+   struct vb2_vmarea_handler   handler;
+};
+
+static void vb2_dma_streaming_put(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   if (atomic_dec_and_test(buf-refcount)) {
+   dma_unmap_single(buf-conf-dev, buf-dma_handle, buf-size,
+DMA_FROM_DEVICE);
+   free_pages_exact(buf-vaddr, buf-size);
+   kfree(buf);
+   }
+
+}
+
+static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size)
+{
+   struct vb2_streaming_conf *conf = alloc_ctx;
+   struct vb2_streaming_buf *buf;
+   int err;
+
+   buf = kzalloc(sizeof *buf, GFP_KERNEL);
+   if (!buf)
+   return ERR_PTR(-ENOMEM);
+   buf-vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA);
+   if (!buf-vaddr) {
+   err = -ENOMEM;
+   goto out;
+   }
+   buf-dma_handle = dma_map_single(conf-dev, buf-vaddr, size,
+DMA_FROM_DEVICE);
+   err = dma_mapping_error(conf-dev, buf-dma_handle);
+   if (err) {
+   dev_err(conf-dev, dma_map_single failed\n);
+
+   free_pages_exact(buf-vaddr, size);
+   buf-vaddr = NULL;
+   goto out_pages;
+   }
+   buf-conf = conf;
+   buf-size = size;
+   buf-handler.refcount = buf-refcount;
+   buf-handler.put = vb2_dma_streaming_put;
+   buf-handler.arg = buf;
+
+   atomic_inc(buf-refcount);
+   return buf;
+
+out_pages:
+   free_pages_exact(buf-vaddr, buf-size);
+out:
+   kfree(buf);
+   return ERR_PTR(err);
+}
+
+static void *vb2_dma_streaming_cookie(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   return buf-dma_handle;
+}
+
+static void *vb2_dma_streaming_vaddr(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   if (!buf)
+   return NULL;
+   return buf-vaddr;
+}
+
+static unsigned

[PATCH v2 3/4] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180)

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
 drivers/media/pci/sta2x11/Kconfig   |2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1235 ++-
 2 file modificati, 411 inserzioni(+), 826 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..654339f 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
tristate STA2X11 VIP Video For Linux
depends on STA2X11
select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-   select VIDEOBUF_DMA_CONTIG
+   select VIDEOBUF2_DMA_STREAMING
depends on PCI  VIDEO_V4L2  VIRT_TO_BUS
help
  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index 4c10205..f423039 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,6 +1,7 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
  * Copyright (C) 2010   WindRiver Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -19,36 +20,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called COPYING.
  *
- * Author: Andreas Kies andreas.k...@windriver.com
- * Vlad Lungu vlad.lu...@windriver.com
- *
  */
 
 #include linux/types.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/init.h
-#include linux/vmalloc.h
-
 #include linux/videodev2.h
-
 #include linux/kmod.h
-
 #include linux/pci.h
 #include linux/interrupt.h
-#include linux/mutex.h
 #include linux/io.h
 #include linux/gpio.h
 #include linux/i2c.h
 #include linux/delay.h
 #include media/v4l2-common.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-ioctl.h
-#include media/videobuf-dma-contig.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
+#include media/videobuf2-dma-streaming.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +58,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,44 +79,24 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
 
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
+
+struct sta2x11_vip_fh {
+   struct v4l2_fh fh;
+};
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
struct video_device *video_dev;
@@ -129,21 +104,27 @@ struct sta2x11_vip {
struct i2c_adapter *adapter;
unsigned int register_save_area[IRQ_COUNT + SAVE_COUNT + AUX_COUNT

[PATCH v2 4/4] adv7180: remove {query/g_/s_}ctrl

[Federico Vaga]

All drivers which use this subdevice use also the control framework.
The v4l2_subdev_core_ops operations {query/g_/s_}ctrl are useless because
device drivers will inherit the controls from this subdevice.

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/i2c/adv7180.c | 3 ---
 1 file modificato, 3 rimozioni(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 45ecf8d..43bc2b9 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -402,9 +402,6 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops 
= {
 static const struct v4l2_subdev_core_ops adv7180_core_ops = {
.g_chip_ident = adv7180_g_chip_ident,
.s_std = adv7180_s_std,
-   .queryctrl = v4l2_subdev_queryctrl,
-   .g_ctrl = v4l2_subdev_g_ctrl,
-   .s_ctrl = v4l2_subdev_s_ctrl,
 };
 
 static const struct v4l2_subdev_ops adv7180_ops = {
-- 
1.7.11.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] v4l: vb2: add prepare/finish callbacks to allocators

[Federico Vaga]

  
  + * @prepare:   called everytime the buffer is passed from userspace
  to the
 nitpick: everytime - every time
 
  + * driver, usefull for cache synchronisation, optional
  + * @finish:called everytime the buffer is passed back from the
  driver
 ditto.
 

This patch come from here: https://patchwork.kernel.org/patch/1323411/

I send it with my patch set because my work require this patch but it is 
not in the next tree. I think it is convenient to fix the original 
patch, probably it will be integrated in the kernel before this one; so 
this patch will be useless.

Anyway, I will apply this comment fix.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/4] v4l: vb2: add prepare/finish callbacks to allocators

[Federico Vaga]

This patch adds support for prepare/finish callbacks in VB2 allocators.
These callback are used for buffer flushing.

Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
Acked-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/v4l2-core/videobuf2-core.c | 11 +++
 include/media/videobuf2-core.h   |  7 +++
 2 file modificati, 18 inserzioni(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
b/drivers/media/v4l2-core/videobuf2-core.c
index 4da3df6..079fa79 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -790,6 +790,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
 {
struct vb2_queue *q = vb-vb2_queue;
unsigned long flags;
+   unsigned int plane;
 
if (vb-state != VB2_BUF_STATE_ACTIVE)
return;
@@ -800,6 +801,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum 
vb2_buffer_state state)
dprintk(4, Done processing on buffer %d, state: %d\n,
vb-v4l2_buf.index, vb-state);
 
+   /* sync buffers */
+   for (plane = 0; plane  vb-num_planes; ++plane)
+   call_memop(q, finish, vb-planes[plane].mem_priv);
+
/* Add the buffer to the done buffers list */
spin_lock_irqsave(q-done_lock, flags);
vb-state = state;
@@ -975,9 +980,15 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct 
v4l2_buffer *b)
 static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
struct vb2_queue *q = vb-vb2_queue;
+   unsigned int plane;
 
vb-state = VB2_BUF_STATE_ACTIVE;
atomic_inc(q-queued_count);
+
+   /* sync buffers */
+   for (plane = 0; plane  vb-num_planes; ++plane)
+   call_memop(q, prepare, vb-planes[plane].mem_priv);
+
q-ops-buf_queue(vb);
 }
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 8dd9b6c..f374f99 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -41,6 +41,10 @@ struct vb2_fileio_data;
  *  argument to other ops in this structure
  * @put_userptr: inform the allocator that a USERPTR buffer will no longer
  *  be used
+ * @prepare:   called everytime the buffer is passed from userspace to the
+ * driver, usefull for cache synchronisation, optional
+ * @finish:called everytime the buffer is passed back from the driver
+ * to the userspace, also optional
  * @vaddr: return a kernel virtual address to a given memory buffer
  * associated with the passed private structure or NULL if no
  * such mapping exists
@@ -65,6 +69,9 @@ struct vb2_mem_ops {
unsigned long size, int write);
void(*put_userptr)(void *buf_priv);
 
+   void(*prepare)(void *buf_priv);
+   void(*finish)(void *buf_priv);
+
void*(*vaddr)(void *buf_priv);
void*(*cookie)(void *buf_priv);
 
-- 
1.7.11.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/v4l2-core/Kconfig   |   5 +
 drivers/media/v4l2-core/Makefile  |   1 +
 drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++
 include/media/videobuf2-dma-streaming.h   |  24 +++
 4 file modificati, 235 inserzioni(+)
 create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c
 create mode 100644 include/media/videobuf2-dma-streaming.h

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index 0c54e19..60548a7 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG
#depends on HAS_DMA
select VIDEOBUF2_CORE
select VIDEOBUF2_MEMOPS
+
+config VIDEOBUF2_DMA_STREAMING
+   select VIDEOBUF2_CORE
+   select VIDEOBUF2_MEMOPS
+   tristate
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..0b2756f 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
 obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
 obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
 obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
+obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o
 
 ccflags-y += -I$(srctree)/drivers/media/dvb-core
 ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c 
b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
new file mode 100644
index 000..23475a6
--- /dev/null
+++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
@@ -0,0 +1,205 @@
+/*
+ * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2
+ *
+ * Copyright (C) 2012 Federico Vaga federico.v...@gmail.com
+ * *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/module.h
+#include linux/slab.h
+#include linux/pagemap.h
+#include linux/dma-mapping.h
+
+#include media/videobuf2-core.h
+#include media/videobuf2-dma-streaming.h
+#include media/videobuf2-memops.h
+
+struct vb2_streaming_conf {
+   struct device   *dev;
+};
+struct vb2_streaming_buf {
+   struct vb2_streaming_conf   *conf;
+   void*vaddr;
+
+   dma_addr_t  dma_handle;
+
+   unsigned long   size;
+   struct vm_area_struct   *vma;
+
+   atomic_trefcount;
+   struct vb2_vmarea_handler   handler;
+};
+
+static void vb2_dma_streaming_put(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   if (atomic_dec_and_test(buf-refcount)) {
+   dma_unmap_single(buf-conf-dev, buf-dma_handle, buf-size,
+DMA_FROM_DEVICE);
+   free_pages_exact(buf-vaddr, buf-size);
+   kfree(buf);
+   }
+
+}
+
+static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size)
+{
+   struct vb2_streaming_conf *conf = alloc_ctx;
+   struct vb2_streaming_buf *buf;
+   int err;
+
+   buf = kzalloc(sizeof *buf, GFP_KERNEL);
+   if (!buf)
+   return ERR_PTR(-ENOMEM);
+   buf-vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA);
+   if (!buf-vaddr) {
+   err = -ENOMEM;
+   goto out;
+   }
+   buf-dma_handle = dma_map_single(conf-dev, buf-vaddr, size,
+DMA_FROM_DEVICE);
+   err = dma_mapping_error(conf-dev, buf-dma_handle);
+   if (err) {
+   dev_err(conf-dev, dma_map_single failed\n);
+
+   free_pages_exact(buf-vaddr, size);
+   buf-vaddr = NULL;
+   goto out_pages;
+   }
+   buf-conf = conf;
+   buf-size = size;
+   buf-handler.refcount = buf-refcount;
+   buf-handler.put = vb2_dma_streaming_put;
+   buf-handler.arg = buf;
+
+   atomic_inc(buf-refcount);
+   return buf;
+
+out_pages:
+   free_pages_exact(buf-vaddr, buf-size);
+out:
+   kfree(buf);
+   return ERR_PTR(err);
+}
+
+static void *vb2_dma_streaming_cookie(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   return (void *)buf-dma_handle;
+}
+
+static void *vb2_dma_streaming_vaddr(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   if (!buf)
+   return NULL;
+   return buf-vaddr;
+}
+
+static unsigned int vb2_dma_streaming_num_users(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   return atomic_read(buf-refcount);
+}
+
+static int vb2_dma_streaming_mmap(void *buf_priv, struct vm_area_struct *vma)
+{
+   struct vb2_streaming_buf

[PATCH 2/4] adv7180: remove {query/g_/s_}ctrl

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/i2c/adv7180.c | 3 ---
 1 file modificato, 3 rimozioni(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 45ecf8d..43bc2b9 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -402,9 +402,6 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops 
= {
 static const struct v4l2_subdev_core_ops adv7180_core_ops = {
.g_chip_ident = adv7180_g_chip_ident,
.s_std = adv7180_s_std,
-   .queryctrl = v4l2_subdev_queryctrl,
-   .g_ctrl = v4l2_subdev_g_ctrl,
-   .s_ctrl = v4l2_subdev_s_ctrl,
 };
 
 static const struct v4l2_subdev_ops adv7180_ops = {
-- 
1.7.11.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 4/4] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
 drivers/media/pci/sta2x11/Kconfig   |2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1235 ++-
 2 file modificati, 411 inserzioni(+), 826 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..654339f 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
tristate STA2X11 VIP Video For Linux
depends on STA2X11
select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-   select VIDEOBUF_DMA_CONTIG
+   select VIDEOBUF2_DMA_STREAMING
depends on PCI  VIDEO_V4L2  VIRT_TO_BUS
help
  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index 4c10205..93d56da 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,6 +1,7 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
  * Copyright (C) 2010   WindRiver Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -19,36 +20,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called COPYING.
  *
- * Author: Andreas Kies andreas.k...@windriver.com
- * Vlad Lungu vlad.lu...@windriver.com
- *
  */
 
 #include linux/types.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/init.h
-#include linux/vmalloc.h
-
 #include linux/videodev2.h
-
 #include linux/kmod.h
-
 #include linux/pci.h
 #include linux/interrupt.h
-#include linux/mutex.h
 #include linux/io.h
 #include linux/gpio.h
 #include linux/i2c.h
 #include linux/delay.h
 #include media/v4l2-common.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-ioctl.h
-#include media/videobuf-dma-contig.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
+#include media/videobuf2-dma-streaming.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +58,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,44 +79,24 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
 
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
+
+struct sta2x11_vip_fh {
+   struct v4l2_fh fh;
+};
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
struct video_device *video_dev;
@@ -129,21 +104,27 @@ struct sta2x11_vip {
struct i2c_adapter *adapter;
unsigned int register_save_area[IRQ_COUNT + SAVE_COUNT + AUX_COUNT];
struct v4l2_subdev *decoder;
-   struct v4l2_pix_format format;
-   v4l2_std_id std;
-   unsigned int input;
-   int users;
-   int disabled;
-   struct mutex mutex; /* exclusive access

Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

 typo: steaming - streaming :-)

fixed

 The header and esp. the source could really do with more
 documentation. It is not at all clear from the code what the
 dma-streaming allocator does and how it differs from other
 allocators.

The other allocators are not documented and to understand them I read 
the code. All the memory allocators reflect a kind of DMA interface: SG 
for scatter/gather, contig for choerent and (now) streaming for 
streaming. So, I'm not sure to understand what do you think is the 
correct way to document a memory allocator; I can write one or two lines 
of comment to summarize each function. what do you think?

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

 On Thursday, September 13, 2012 3:53 PM Federico Vaga wrote:
  Signed-off-by: Federico Vaga federico.v...@gmail.com
 
 A few words explaining why this memory handling module is required or
 beneficial will definitely improve the commit :)

ok, I will write some lines

  +static void *vb2_dma_streaming_cookie(void *buf_priv)
  +{
  +   struct vb2_streaming_buf *buf = buf_priv;
  +
  +   return (void *)buf-dma_handle;
  +}
 
 Please change this function to:
 
 static void *vb2_dma_streaming_cookie(void *buf_priv)
 {
   struct vb2_streaming_buf *buf = buf_priv;
   return buf-dma_handle;
 }
 
 and add a following static inline to
 include/media/videobuf2-dma-streaming.h:
 
 static inline dma_addr_t
 vb2_dma_streaming_plane_paddr(struct vb2_buffer *vb, unsigned int
 plane_no) {
 dma_addr_t *dma_addr = vb2_plane_cookie(vb, plane_no);
 return *dma_addr;
 }
 
 Do not use 'cookie' callback directly in the driver, the driver should
 use the above proxy.
 
 The buf-dma_handle workaround is required for some possible
 configurations with 64bit dma addresses, see commit 472af2b05bdefc.

ACK.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

 Well, there is some documentation here:
 
   https://lwn.net/Articles/447435/

I know this, I learned from this page :)

What I'm saying is that I don't know what to write inside the code to 
make it clearer than now. I think is clear, because if you know the 
videobuf2, you know what I'm doing in each vb2_mem_ops. I suppose that 
this is the reason why there are no comments inside the other memory 
allocator. Maybe I am wrong.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

In data giovedì 13 settembre 2012 11:45:31, Jonathan Corbet ha scritto:
 On Thu, 13 Sep 2012 17:46:32 +0200
 
 Federico Vaga federico.v...@gmail.com wrote:
   A few words explaining why this memory handling module is required
   or
   beneficial will definitely improve the commit :)
  
  ok, I will write some lines
 
 In general, all of these patches need *much* better changelogs (i.e.
 they need changelogs).  What are you doing, and *why* are you doing
 it?  The future will want to know.

I can improve the comment about the ADV7180: it is not clear why I'm 
removing that functions. (and the memory allocator commit is also in the 
todo list).

The STA2X11_VIP commit, I think is clear, I convert it to use videobu2 
and control framework. I can add some extra lines to explain why: 
because videobuf is obsolete

 I'm going to try to look at the actual code, but time isn't something
 I have a lot of, still...

The actual code is the same of the previous one with yours (plural) 
suggestions from the RFC submission (few week ago). I did not write 
anything else.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Update VIP to videobuf2 and control framework

[Federico Vaga]

In data mercoledì 1 agosto 2012 07:04:18, Jonathan Corbet ha scritto:
 On Wed, 1 Aug 2012 08:41:56 +0200
 
 Hans Verkuil hverk...@xs4all.nl wrote:
   The second patch adds a new memory allocator for the videobuf2. I
   name it videobuf2-dma-streaming but I think streaming is not
   the best choice, so suggestions are welcome. My inspiration for
   this buffer come from videobuf-dma-contig (cached) version. After
   I made this buffer I found the videobuf2-dma-nc made by Jonathan
   Corbet and I improve the allocator with some suggestions
   (http://patchwork.linuxtv.org/patch/7441/). The VIP doesn't work
   with videobu2-dma-contig and I think this solution is easier the
   sg. 
  I leave this to the vb2 experts. It's not obvious to me why we would
  need a fourth memory allocator.
 
 I first wrote my version after observing that performance dropped by a
 factor of three on the OLPC XO 1.75 when using contiguous, coherent
 memory.  If the architecture needs to turn off caching, things really
 slow down, to the point that it's unusable.  There's no real reason
 why a V4L2 device shouldn't be able to use streaming mappings in this
 situation; it performs better and doesn't eat into the limited
 amounts of coherent DMA space available on some systems.
 
 I never got my version into a mergeable state only because I finally
 figured out how to bludgeon the hardware into doing s/g DMA and didn't
 need it anymore.  But I think it's a worthwhile functionality to
 have, and, with CMA, it could be the preferred mode for a number of
 devices.
 
 jon

I think that the memory allocator is the most questionable patch, but if 
there are not any other comments I will send my three patches for the 
inclusion. It is summer, time for vacation, so I'll wait for another 
week or two for critical comments and then I will send patches.


-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] [media] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

 Getting back to your patch - in your approach cpu cache handling is
 missing. I suspect that it worked fine only because it has been
 tested on some simple platform without any cpu caches (or with very
 small ones).

Is missing from the memory allocator because I do it on the device 
driver. The current operations don't allow me to do that in the memory 
allocator.


 Please check the following thread:
 http://www.spinics.net/lists/linux-media/msg51768.html where Tomasz
 has posted his ongoing effort on updating and extending videobuf2 and
 dma-contig allocator. Especially the patch
 http://www.spinics.net/lists/linux-media/msg51776.html will be
 interesting for you, because cpu cache synchronization
 (dma_sync_single_for_device / dma_sync_single_for_cpu) should be
 called from prepare/finish callbacks.

You are right, it is interesting because avoid me to use cache sync in 
my driver. Can I work on these patches?

From this page I understand that these patches are not approved yet
https://patchwork.kernel.org/project/linux-media/list/?page=2

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] [media] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

 You can take the patch which adds prepare/finish methods to memory
 allocators. It should not have any dependency on the other stuff from
 that thread. I'm fine with merging it either together with Your patch
 or via Tomasz's patchset, whatever comes first.

Thank you. I'll do the job

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Update STA2X11 to videobuf2 and control framework

[Federico Vaga]

As suggested I moved the Video Buffer Input (VIP) of the STA2X11 board to the
videobuf2. This patch series is an RFC.

The first patch is just an update to the adv7180 because the VIP (the only
user) now use the control framework so query{g_|s_|ctrl} are not necessery.

The second patch adds a new memory allocator for the videobuf2. I name it
videobuf2-dma-streaming but I think streaming is not the best choice, so
suggestions are welcome. My inspiration for this buffer come from
videobuf-dma-contig (cached) version. After I made this buffer I found the
videobuf2-dma-nc made by Jonathan Corbet and I improve the allocator with
some suggestions (http://patchwork.linuxtv.org/patch/7441/). The VIP doesn't
work with videobu2-dma-contig and I think this solution is easier the sg.

The third patch updates the VIP to videobuf2 and control framework. I made also
some restyling to the driver and change some mechanism so I take the ownership
of the driver and I add the copyright of ST Microelectronics. Some trivial
code is unchanged. The patch probably needs some extra update and probably,
you will give me many suggestions. 
I add the control framework to the VIP but without any control. I add it to 
inherit controls from adv7180.

--
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/3 RFC] [media] adv7180: remove {query/g_/s_}ctrl

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/video/adv7180.c | 3 ---
 1 file modificato, 3 rimozioni(-)

diff --git a/drivers/media/video/adv7180.c b/drivers/media/video/adv7180.c
index 07bb550..bcc7d60 100644
--- a/drivers/media/video/adv7180.c
+++ b/drivers/media/video/adv7180.c
@@ -402,9 +402,6 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops 
= {
 static const struct v4l2_subdev_core_ops adv7180_core_ops = {
.g_chip_ident = adv7180_g_chip_ident,
.s_std = adv7180_s_std,
-   .queryctrl = v4l2_subdev_queryctrl,
-   .g_ctrl = v4l2_subdev_g_ctrl,
-   .s_ctrl = v4l2_subdev_s_ctrl,
 };
 
 static const struct v4l2_subdev_ops adv7180_ops = {
-- 
1.7.11.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Update VIP to videobuf2 and control framework

[Federico Vaga]

As suggested I moved the Video Buffer Input (VIP) of the STA2X11 board to the
videobuf2. This patch series is an RFC.

The first patch is just an update to the adv7180 because the VIP (the only
user) now use the control framework so query{g_|s_|ctrl} are not necessery.

The second patch adds a new memory allocator for the videobuf2. I name it
videobuf2-dma-streaming but I think streaming is not the best choice, so
suggestions are welcome. My inspiration for this buffer come from
videobuf-dma-contig (cached) version. After I made this buffer I found the
videobuf2-dma-nc made by Jonathan Corbet and I improve the allocator with
some suggestions (http://patchwork.linuxtv.org/patch/7441/). The VIP doesn't
work with videobu2-dma-contig and I think this solution is easier the sg.

The third patch updates the VIP to videobuf2 and control framework. I made also
some restyling to the driver and change some mechanism so I take the ownership
of the driver and I add the copyright of ST Microelectronics. Some trivial
code is unchanged. The patch probably needs some extra update.
I add the control framework to the VIP but without any control. I add it to 
inherit controls from adv7180.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/3] [media] adv7180: remove {query/g_/s_}ctrl

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/video/adv7180.c | 3 ---
 1 file modificato, 3 rimozioni(-)

diff --git a/drivers/media/video/adv7180.c b/drivers/media/video/adv7180.c
index 07bb550..bcc7d60 100644
--- a/drivers/media/video/adv7180.c
+++ b/drivers/media/video/adv7180.c
@@ -402,9 +402,6 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops 
= {
 static const struct v4l2_subdev_core_ops adv7180_core_ops = {
.g_chip_ident = adv7180_g_chip_ident,
.s_std = adv7180_s_std,
-   .queryctrl = v4l2_subdev_queryctrl,
-   .g_ctrl = v4l2_subdev_g_ctrl,
-   .s_ctrl = v4l2_subdev_s_ctrl,
 };
 
 static const struct v4l2_subdev_ops adv7180_ops = {
-- 
1.7.11.2

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/3] [media] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/video/Kconfig   |   6 +-
 drivers/media/video/Makefile  |   1 +
 drivers/media/video/videobuf2-dma-streaming.c | 187 ++
 include/media/videobuf2-dma-streaming.h   |  24 
 4 file modificati, 217 inserzioni(+). 1 rimozione(-)
 create mode 100644 drivers/media/video/videobuf2-dma-streaming.c
 create mode 100644 include/media/videobuf2-dma-streaming.h

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index be6d718..6c60b59 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -59,6 +59,10 @@ config VIDEOBUF2_DMA_NC
select VIDEOBUF2_CORE
select VIDEOBUF2_MEMOPS
tristate
+config VIDEOBUF2_DMA_STREAMING
+   select VIDEOBUF2_CORE
+   select VIDEOBUF2_MEMOPS
+   tristate
 
 config VIDEOBUF2_VMALLOC
select VIDEOBUF2_CORE
@@ -844,7 +848,7 @@ config STA2X11_VIP
tristate STA2X11 VIP Video For Linux
depends on STA2X11
select VIDEO_ADV7180 if VIDEO_HELPER_CHIPS_AUTO
-   select VIDEOBUF_DMA_CONTIG
+   select VIDEOBUF2_DMA_STREAMING
depends on PCI  VIDEO_V4L2  VIRT_TO_BUS
help
  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
index 30234af..c1a08b9e 100644
--- a/drivers/media/video/Makefile
+++ b/drivers/media/video/Makefile
@@ -140,6 +140,7 @@ obj-$(CONFIG_VIDEOBUF2_VMALLOC) += 
videobuf2-vmalloc.o
 obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
 obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
 obj-$(CONFIG_VIDEOBUF2_DMA_NC) += videobuf2-dma-nc.o
+obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING)  += videobuf2-dma-streaming.o
 
 obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
 
diff --git a/drivers/media/video/videobuf2-dma-streaming.c 
b/drivers/media/video/videobuf2-dma-streaming.c
new file mode 100644
index 000..d96d3d9
--- /dev/null
+++ b/drivers/media/video/videobuf2-dma-streaming.c
@@ -0,0 +1,187 @@
+/*
+ * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2
+ *
+ * Copyright (C) 2012 Federico Vaga federico.v...@gmail.com
+ * *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/module.h
+#include linux/slab.h
+#include linux/pagemap.h
+#include linux/dma-mapping.h
+
+#include media/videobuf2-core.h
+#include media/videobuf2-dma-streaming.h
+#include media/videobuf2-memops.h
+
+struct vb2_streaming_conf {
+   struct device   *dev;
+};
+struct vb2_streaming_buf {
+   struct vb2_streaming_conf   *conf;
+   void*vaddr;
+
+   dma_addr_t  dma_handle;
+
+   unsigned long   size;
+   struct vm_area_struct   *vma;
+
+   atomic_trefcount;
+   struct vb2_vmarea_handler   handler;
+};
+
+static void vb2_dma_streaming_put(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   if (atomic_dec_and_test(buf-refcount)) {
+   dma_unmap_single(buf-conf-dev, buf-dma_handle, buf-size,
+DMA_FROM_DEVICE);
+   free_pages_exact(buf-vaddr, buf-size);
+   kfree(buf);
+   }
+
+}
+
+static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size)
+{
+   struct vb2_streaming_conf *conf = alloc_ctx;
+   struct vb2_streaming_buf *buf;
+   int err;
+
+   buf = kzalloc(sizeof *buf, GFP_KERNEL);
+   if (!buf)
+   return ERR_PTR(-ENOMEM);
+   buf-vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA);
+   if (!buf-vaddr) {
+   err = -ENOMEM;
+   goto out;
+   }
+   buf-dma_handle = dma_map_single(conf-dev, buf-vaddr, size,
+DMA_FROM_DEVICE);
+   err = dma_mapping_error(conf-dev, buf-dma_handle);
+   if (err) {
+   dev_err(conf-dev, dma_map_single failed\n);
+
+   free_pages_exact(buf-vaddr, size);
+   buf-vaddr = NULL;
+   goto out_pages;
+   }
+   buf-conf = conf;
+   buf-size = size;
+   buf-handler.refcount = buf-refcount;
+   buf-handler.put = vb2_dma_streaming_put;
+   buf-handler.arg = buf;
+
+   atomic_inc(buf-refcount);
+   return buf;
+
+out_pages:
+   free_pages_exact(buf-vaddr, buf-size);
+out:
+   kfree(buf);
+   return ERR_PTR(err);
+}
+
+static void *vb2_dma_streaming_cookie(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv;
+
+   return (void *)buf-dma_handle;
+}
+
+static void *vb2_dma_streaming_vaddr(void *buf_priv)
+{
+   struct vb2_streaming_buf *buf = buf_priv

[PATCH 3/3] [media] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
 drivers/media/video/sta2x11_vip.c | 1134 ++---
 1 file modificato, 410 inserzioni(+), 724 rimozioni(-)

diff --git a/drivers/media/video/sta2x11_vip.c 
b/drivers/media/video/sta2x11_vip.c
index 4c10205..5a75718 100644
--- a/drivers/media/video/sta2x11_vip.c
+++ b/drivers/media/video/sta2x11_vip.c
@@ -1,6 +1,7 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
  * Copyright (C) 2010   WindRiver Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -19,36 +20,28 @@
  * The full GNU General Public License is included in this distribution in
  * the file called COPYING.
  *
- * Author: Andreas Kies andreas.k...@windriver.com
- * Vlad Lungu vlad.lu...@windriver.com
- *
  */
 
 #include linux/types.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/init.h
-#include linux/vmalloc.h
-
 #include linux/videodev2.h
-
 #include linux/kmod.h
-
 #include linux/pci.h
 #include linux/interrupt.h
-#include linux/mutex.h
 #include linux/io.h
 #include linux/gpio.h
 #include linux/i2c.h
 #include linux/delay.h
 #include media/v4l2-common.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-ioctl.h
-#include media/videobuf-dma-contig.h
+#include media/videobuf2-dma-streaming.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +56,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,43 +77,20 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
+
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
 
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
@@ -129,21 +99,28 @@ struct sta2x11_vip {
struct i2c_adapter *adapter;
unsigned int register_save_area[IRQ_COUNT + SAVE_COUNT + AUX_COUNT];
struct v4l2_subdev *decoder;
-   struct v4l2_pix_format format;
-   v4l2_std_id std;
-   unsigned int input;
-   int users;
-   int disabled;
-   struct mutex mutex; /* exclusive access during open */
-   spinlock_t slock;   /* spin lock for hardware and queue access */
-   struct videobuf_queue vb_vidq;
-   struct list_head capture;
-   struct videobuf_buffer *active;
-   int started, closing, tcount, bcount;
+   struct v4l2_ctrl_handler ctrl_hdl;
+
+
+   struct v4l2_pix_format format;  /* pixel format, fixed UYVY */
+   v4l2_std_id std;/* Video standard (PAL/NTSC)*/
+   unsigned int input; /* Input line (0 or 1) */
+   int disabled; /* 1 disabled 0 enabled */
+   spinlock_t slock; /* spin lock for hardware */
+
+   struct vb2_alloc_ctx *alloc_ctx;
+   struct vb2_queue vb_vidq; /* queue maintaned by videobuf2 */
+   struct list_head buffer_list; /* list of buffers */
+   unsigned int

Re: [PATCH 1/3] [media] adv7180: remove {query/g_/s_}ctrl

[Federico Vaga]

I'm sorry for the email duplication, I press the wrong key on git-send email.
Ignore these emails. Sorry

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Update VIP to videobuf2 and control framework

[Federico Vaga]

I use git send-email command to send patches but I think I made a
mistake. If something
is wrong or confused please tell me and I try to resend all the
patches hopefully without mistake. Sorry again.

2012/7/31 Federico Vaga federico.v...@gmail.com:
 As suggested I moved the Video Buffer Input (VIP) of the STA2X11 board to the
 videobuf2. This patch series is an RFC.

 The first patch is just an update to the adv7180 because the VIP (the only
 user) now use the control framework so query{g_|s_|ctrl} are not necessery.

 The second patch adds a new memory allocator for the videobuf2. I name it
 videobuf2-dma-streaming but I think streaming is not the best choice, so
 suggestions are welcome. My inspiration for this buffer come from
 videobuf-dma-contig (cached) version. After I made this buffer I found the
 videobuf2-dma-nc made by Jonathan Corbet and I improve the allocator with
 some suggestions (http://patchwork.linuxtv.org/patch/7441/). The VIP doesn't
 work with videobu2-dma-contig and I think this solution is easier the sg.

 The third patch updates the VIP to videobuf2 and control framework. I made 
 also
 some restyling to the driver and change some mechanism so I take the ownership
 of the driver and I add the copyright of ST Microelectronics. Some trivial
 code is unchanged. The patch probably needs some extra update.
 I add the control framework to the VIP but without any control. I add it to
 inherit controls from adv7180.




-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Update VIP to videobuf2 and control framework

[Federico Vaga]

Hi Hans,
 
 Did you run the latest v4l2-compliance tool from the v4l-utils.git
 repository over your driver? I'm sure you didn't since VIP is missing
 support for control events and v4l2-compliance would certainly
 complain about that.
 
 Always check with v4l2-compliance whenever you make changes! It's
 continuously improved as well, so a periodic check wouldn't hurt.

I applied all your suggestions, and some extra simplification; now I'm 
running v4l2-compliance but I have this error:


Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
fail: v4l2-compliance.cpp(322): doioctl(node, 
VIDIOC_G_PRIORITY, prio)
test VIDIOC_G/S_PRIORITY: FAIL


which I don't undestand. I don't have vidio_{g|s}_priority functions in 
my implementation. And I'm using the V4L2_FL_USE_FH_PRIO flag as 
suggested in the documentation:

---
- flags: optional. Set to V4L2_FL_USE_FH_PRIO if you want to let the 
framework handle the VIDIOC_G/S_PRIORITY ioctls. This requires that you 
use struct v4l2_fh. Eventually this flag will disappear once all drivers 
use the core priority handling. But for now it has to be set explicitly.
--

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: Update VIP to videobuf2 and control framework

[Federico Vaga]

  I applied all your suggestions, and some extra simplification;
  [...]

  ---
  - flags: optional. Set to V4L2_FL_USE_FH_PRIO if you want to let the
  framework handle the VIDIOC_G/S_PRIORITY ioctls. This requires that
  you use struct v4l2_fh.
 
   ^^
 
 Are you using struct v4l2_fh? The version you posted didn't. You need
 this anyway to implement control events.

Yes I'm using it now, it is part of the extra simplification that I did.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/3 v2] [media] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com

---
 drivers/media/video/sta2x11_vip.c | 1239 +
 1 file modificato, 414 inserzioni(+), 825 rimozioni(-)

diff --git a/drivers/media/video/sta2x11_vip.c 
b/drivers/media/video/sta2x11_vip.c
index 4c10205..ffd9f0a 100644
--- a/drivers/media/video/sta2x11_vip.c
+++ b/drivers/media/video/sta2x11_vip.c
@@ -1,6 +1,7 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
  * Copyright (C) 2010   WindRiver Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -19,36 +20,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called COPYING.
  *
- * Author: Andreas Kies andreas.k...@windriver.com
- * Vlad Lungu vlad.lu...@windriver.com
- *
  */
 
 #include linux/types.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/init.h
-#include linux/vmalloc.h
-
 #include linux/videodev2.h
-
 #include linux/kmod.h
-
 #include linux/pci.h
 #include linux/interrupt.h
-#include linux/mutex.h
 #include linux/io.h
 #include linux/gpio.h
 #include linux/i2c.h
 #include linux/delay.h
 #include media/v4l2-common.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-ioctl.h
-#include media/videobuf-dma-contig.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
+#include media/videobuf2-dma-streaming.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +58,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,44 +79,24 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
 
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
+
+struct sta2x11_vip_fh {
+   struct v4l2_fh fh;
+};
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
struct video_device *video_dev;
@@ -129,21 +104,27 @@ struct sta2x11_vip {
struct i2c_adapter *adapter;
unsigned int register_save_area[IRQ_COUNT + SAVE_COUNT + AUX_COUNT];
struct v4l2_subdev *decoder;
-   struct v4l2_pix_format format;
-   v4l2_std_id std;
-   unsigned int input;
-   int users;
-   int disabled;
-   struct mutex mutex; /* exclusive access during open */
-   spinlock_t slock;   /* spin lock for hardware and queue access */
-   struct videobuf_queue vb_vidq;
-   struct list_head capture;
-   struct videobuf_buffer *active;
-   int started, closing, tcount, bcount;
+   struct v4l2_ctrl_handler ctrl_hdl;
+
+
+   struct v4l2_pix_format format;  /* pixel format, fixed UYVY */
+   v4l2_std_id std;/* Video standard (PAL/NTSC)*/
+   unsigned int input; /* Input line (0 or 1) */
+   int disabled; /* 1 disabled 0 enabled */
+   spinlock_t slock; /* spin lock for hardware */
+
+   struct vb2_alloc_ctx *alloc_ctx

Re: Update VIP to videobuf2 and control framework

[Federico Vaga]

 In that case I need to see your latest version of the source code to
 see why it doesn't work.

I send it as patch v2 of the previous one

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/3 v2] [media] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

  +   vip-video_dev-flags |= V4L2_FL_USES_V4L2_FH |
  V4L2_FL_USE_FH_PRIO;
 Been there, done that :-)
 
 V4L2_FL_USE_FH_PRIO is a bit number, not a bit mask. Use set_bit
 instead:
 
   set_bit(V4L2_FL_USE_FH_PRIO, vip-video_dev-flags);
 
 No need to set V4L2_FL_USES_V4L2_FH, BTW. That will be set
 automatically as soon as v4l2_fh_open is called.

I saw unsigned long flags; in the header but without reading the 
comment :) Thank you. I will test it in these days but I think it's all 
done.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [GIT PULL FOR v3.5] Move sta2x11_vip to staging

[Federico Vaga]

 Any news on this?

Hi Hans,

I'm on it :)

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] adv7180: add support to user controls

[Federico Vaga]

 If you could do that work, then that would be much appreciated. You have the
 hardware, after all, so that makes it easier for you.

Hi Hans,

I'll submit a patch for the control framework on the ADV7180 

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH RFC] [media] adv7180.c: convert to v4l2 control framework

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/video/adv7180.c |  221 +
 1 file changed, 90 insertions(+), 131 deletions(-)

diff --git a/drivers/media/video/adv7180.c b/drivers/media/video/adv7180.c
index 174bffa..7705456 100644
--- a/drivers/media/video/adv7180.c
+++ b/drivers/media/video/adv7180.c
@@ -26,11 +26,10 @@
 #include media/v4l2-ioctl.h
 #include linux/videodev2.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-chip-ident.h
 #include linux/mutex.h
 
-#define DRIVER_NAME adv7180
-
 #define ADV7180_INPUT_CONTROL_REG  0x00
 #define ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM   0x00
 #define ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM_PED 0x10
@@ -55,21 +54,21 @@
 
 #define ADV7180_AUTODETECT_ENABLE_REG  0x07
 #define ADV7180_AUTODETECT_DEFAULT 0x7f
-
+/* Contrast */
 #define ADV7180_CON_REG0x08/*Unsigned */
-#define CON_REG_MIN0
-#define CON_REG_DEF128
-#define CON_REG_MAX255
-
+#define ADV7180_CON_MIN0
+#define ADV7180_CON_DEF128
+#define ADV7180_CON_MAX255
+/* Brightness*/
 #define ADV7180_BRI_REG0x0a/*Signed */
-#define BRI_REG_MIN-128
-#define BRI_REG_DEF0
-#define BRI_REG_MAX127
-
+#define ADV7180_BRI_MIN-128
+#define ADV7180_BRI_DEF0
+#define ADV7180_BRI_MAX127
+/* Hue */
 #define ADV7180_HUE_REG0x0b/*Signed, inverted */
-#define HUE_REG_MIN-127
-#define HUE_REG_DEF0
-#define HUE_REG_MAX128
+#define ADV7180_HUE_MIN-127
+#define ADV7180_HUE_DEF0
+#define ADV7180_HUE_MAX128
 
 #define ADV7180_ADI_CTRL_REG   0x0e
 #define ADV7180_ADI_CTRL_IRQ_SPACE 0x20
@@ -98,12 +97,12 @@
 #define ADV7180_ICONF1_ACTIVE_LOW  0x01
 #define ADV7180_ICONF1_PSYNC_ONLY  0x10
 #define ADV7180_ICONF1_ACTIVE_TO_CLR   0xC0
-
+/* Saturation */
 #define ADV7180_SD_SAT_CB_REG  0xe3/*Unsigned */
 #define ADV7180_SD_SAT_CR_REG  0xe4/*Unsigned */
-#define SAT_REG_MIN0
-#define SAT_REG_DEF128
-#define SAT_REG_MAX255
+#define ADV7180_SAT_MIN0
+#define ADV7180_SAT_DEF128
+#define ADV7180_SAT_MAX255
 
 #define ADV7180_IRQ1_LOCK  0x01
 #define ADV7180_IRQ1_UNLOCK0x02
@@ -121,18 +120,18 @@
 #define ADV7180_NTSC_V_BIT_END_MANUAL_NVEND0x4F
 
 struct adv7180_state {
+   struct v4l2_ctrl_handler ctrl_hdl;
struct v4l2_subdev  sd;
struct work_struct  work;
struct mutexmutex; /* mutual excl. when accessing chip */
int irq;
v4l2_std_id curr_norm;
boolautodetect;
-   s8  brightness;
-   s16 hue;
-   u8  contrast;
-   u8  saturation;
u8  input;
 };
+#define to_adv7180_sd(_ctrl) container_of(_ctrl-handler, \
+  struct adv7180_state,\
+  ctrl_hdl)-sd
 
 static v4l2_std_id adv7180_std_to_v4l2(u8 status1)
 {
@@ -237,7 +236,7 @@ static int adv7180_s_routing(struct v4l2_subdev *sd, u32 
input,
if (ret)
return ret;
 
-   /*We cannot discriminate between LQFP and 40-pin LFCSP, so accept
+   /* We cannot discriminate between LQFP and 40-pin LFCSP, so accept
 * all inputs and let the card driver take care of validation
 */
if ((input  ADV7180_INPUT_CONTROL_INSEL_MASK) != input)
@@ -316,117 +315,39 @@ out:
return ret;
 }
 
-static int adv7180_queryctrl(struct v4l2_subdev *sd, struct v4l2_queryctrl *qc)
-{
-   switch (qc-id) {
-   case V4L2_CID_BRIGHTNESS:
-   return v4l2_ctrl_query_fill(qc, BRI_REG_MIN, BRI_REG_MAX,
-   1, BRI_REG_DEF);
-   case V4L2_CID_HUE:
-   return v4l2_ctrl_query_fill(qc, HUE_REG_MIN, HUE_REG_MAX,
-   1, HUE_REG_DEF);
-   case V4L2_CID_CONTRAST:
-   return v4l2_ctrl_query_fill(qc, CON_REG_MIN, CON_REG_MAX,
-   1, CON_REG_DEF);
-   case V4L2_CID_SATURATION:
-   return v4l2_ctrl_query_fill(qc, SAT_REG_MIN, SAT_REG_MAX,
-   1, SAT_REG_DEF);
-   default:
-   break;
-   }
-
-   return -EINVAL;
-}
-
-static int adv7180_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
-   struct adv7180_state *state = to_state(sd);
-   int ret = mutex_lock_interruptible

Re: [PATCH RFC] [media] adv7180.c: convert to v4l2 control framework

[Federico Vaga]

Hi Hans,

Thank you for your review

  +static int adv7180_init_controls(struct adv7180_state *state)
  +{
  +   v4l2_ctrl_handler_init(state-ctrl_hdl, 2);
 
 2 - 4, since there are 4 controls. It's a hint only, but it helps
 optimizing the internal hash data structure.

Sure :)

  
  @@ -445,9 +402,9 @@ static const struct v4l2_subdev_video_ops
  adv7180_video_ops = { 
   static const struct v4l2_subdev_core_ops adv7180_core_ops = {
   
  .g_chip_ident = adv7180_g_chip_ident,
  .s_std = adv7180_s_std,
  
  -   .queryctrl = adv7180_queryctrl,
  -   .g_ctrl = adv7180_g_ctrl,
  -   .s_ctrl = adv7180_s_ctrl,
  +   .queryctrl = v4l2_subdev_queryctrl,
  +   .g_ctrl = v4l2_subdev_g_ctrl,
  +   .s_ctrl = v4l2_subdev_s_ctrl,
 
 If adv7180 is currently *only* used by bridge/platform drivers that
 also use the control framework, then you can remove
 queryctrl/g/s_ctrl altogether.

I'm not sure to undestand this point. I grepped for the adv7180 and it 
seem that I'm the only user of the adv7180 (sta2x11 VIP driver). In the
VIP driver I don't use the control framework (there aren't controls), so 
I think these lines must be there. Am I wrong?

I think you are thinking at the Inheriting Controls section of the 
v4l2-controls.txt document. Right?


  -   ret = i2c_smbus_write_byte_data(client, ADV7180_HUE_REG,
  state-hue); +  ret = i2c_smbus_write_byte_data(client,
  ADV7180_HUE_REG,
  +   ADV7180_HUE_DEF);
 
 It shouldn't be necessary to initialize the controls since
 v4l2_ctrl_handler_setup does that for you already.

Removed

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH RFC] [media] adv7180.c: convert to v4l2 control framework

[Federico Vaga]

   @@ -445,9 +402,9 @@ static const struct v4l2_subdev_video_ops
   adv7180_video_ops = { 
static const struct v4l2_subdev_core_ops adv7180_core_ops = {

 .g_chip_ident = adv7180_g_chip_ident,
 .s_std = adv7180_s_std,
   
   - .queryctrl = adv7180_queryctrl,
   - .g_ctrl = adv7180_g_ctrl,
   - .s_ctrl = adv7180_s_ctrl,
   + .queryctrl = v4l2_subdev_queryctrl,
   + .g_ctrl = v4l2_subdev_g_ctrl,
   + .s_ctrl = v4l2_subdev_s_ctrl,
  
  I'm not sure to undestand this point. I grepped for the adv7180
  and it seem that I'm the only user of the adv7180 (sta2x11 VIP
  driver). In the VIP driver I don't use the control framework (there
  aren't controls), so I think these lines must be there. Am I wrong?
 
 Correct. But once sta2x11 is converted to using the control framework,
 then these lines can be dropped since no one else is using this
 subdevice driver.

What do you suggest? I re-submit this patch and when sta2x11 is fixed a 
I submit a new patch to remove these lines; or wait the full conversion 
of the sta2x11 driver and submit both patch?

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] [media] adv7180.c: convert to v4l2 control framework

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/video/adv7180.c |  235 +++--
 1 file changed, 84 insertions(+), 151 deletions(-)

diff --git a/drivers/media/video/adv7180.c b/drivers/media/video/adv7180.c
index 174bffa..07bb550 100644
--- a/drivers/media/video/adv7180.c
+++ b/drivers/media/video/adv7180.c
@@ -26,11 +26,10 @@
 #include media/v4l2-ioctl.h
 #include linux/videodev2.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-chip-ident.h
 #include linux/mutex.h
 
-#define DRIVER_NAME adv7180
-
 #define ADV7180_INPUT_CONTROL_REG  0x00
 #define ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM   0x00
 #define ADV7180_INPUT_CONTROL_AD_PAL_BG_NTSC_J_SECAM_PED 0x10
@@ -55,21 +54,21 @@
 
 #define ADV7180_AUTODETECT_ENABLE_REG  0x07
 #define ADV7180_AUTODETECT_DEFAULT 0x7f
-
+/* Contrast */
 #define ADV7180_CON_REG0x08/*Unsigned */
-#define CON_REG_MIN0
-#define CON_REG_DEF128
-#define CON_REG_MAX255
-
+#define ADV7180_CON_MIN0
+#define ADV7180_CON_DEF128
+#define ADV7180_CON_MAX255
+/* Brightness*/
 #define ADV7180_BRI_REG0x0a/*Signed */
-#define BRI_REG_MIN-128
-#define BRI_REG_DEF0
-#define BRI_REG_MAX127
-
+#define ADV7180_BRI_MIN-128
+#define ADV7180_BRI_DEF0
+#define ADV7180_BRI_MAX127
+/* Hue */
 #define ADV7180_HUE_REG0x0b/*Signed, inverted */
-#define HUE_REG_MIN-127
-#define HUE_REG_DEF0
-#define HUE_REG_MAX128
+#define ADV7180_HUE_MIN-127
+#define ADV7180_HUE_DEF0
+#define ADV7180_HUE_MAX128
 
 #define ADV7180_ADI_CTRL_REG   0x0e
 #define ADV7180_ADI_CTRL_IRQ_SPACE 0x20
@@ -98,12 +97,12 @@
 #define ADV7180_ICONF1_ACTIVE_LOW  0x01
 #define ADV7180_ICONF1_PSYNC_ONLY  0x10
 #define ADV7180_ICONF1_ACTIVE_TO_CLR   0xC0
-
+/* Saturation */
 #define ADV7180_SD_SAT_CB_REG  0xe3/*Unsigned */
 #define ADV7180_SD_SAT_CR_REG  0xe4/*Unsigned */
-#define SAT_REG_MIN0
-#define SAT_REG_DEF128
-#define SAT_REG_MAX255
+#define ADV7180_SAT_MIN0
+#define ADV7180_SAT_DEF128
+#define ADV7180_SAT_MAX255
 
 #define ADV7180_IRQ1_LOCK  0x01
 #define ADV7180_IRQ1_UNLOCK0x02
@@ -121,18 +120,18 @@
 #define ADV7180_NTSC_V_BIT_END_MANUAL_NVEND0x4F
 
 struct adv7180_state {
+   struct v4l2_ctrl_handler ctrl_hdl;
struct v4l2_subdev  sd;
struct work_struct  work;
struct mutexmutex; /* mutual excl. when accessing chip */
int irq;
v4l2_std_id curr_norm;
boolautodetect;
-   s8  brightness;
-   s16 hue;
-   u8  contrast;
-   u8  saturation;
u8  input;
 };
+#define to_adv7180_sd(_ctrl) container_of(_ctrl-handler, \
+  struct adv7180_state,\
+  ctrl_hdl)-sd
 
 static v4l2_std_id adv7180_std_to_v4l2(u8 status1)
 {
@@ -237,7 +236,7 @@ static int adv7180_s_routing(struct v4l2_subdev *sd, u32 
input,
if (ret)
return ret;
 
-   /*We cannot discriminate between LQFP and 40-pin LFCSP, so accept
+   /* We cannot discriminate between LQFP and 40-pin LFCSP, so accept
 * all inputs and let the card driver take care of validation
 */
if ((input  ADV7180_INPUT_CONTROL_INSEL_MASK) != input)
@@ -316,117 +315,39 @@ out:
return ret;
 }
 
-static int adv7180_queryctrl(struct v4l2_subdev *sd, struct v4l2_queryctrl *qc)
-{
-   switch (qc-id) {
-   case V4L2_CID_BRIGHTNESS:
-   return v4l2_ctrl_query_fill(qc, BRI_REG_MIN, BRI_REG_MAX,
-   1, BRI_REG_DEF);
-   case V4L2_CID_HUE:
-   return v4l2_ctrl_query_fill(qc, HUE_REG_MIN, HUE_REG_MAX,
-   1, HUE_REG_DEF);
-   case V4L2_CID_CONTRAST:
-   return v4l2_ctrl_query_fill(qc, CON_REG_MIN, CON_REG_MAX,
-   1, CON_REG_DEF);
-   case V4L2_CID_SATURATION:
-   return v4l2_ctrl_query_fill(qc, SAT_REG_MIN, SAT_REG_MAX,
-   1, SAT_REG_DEF);
-   default:
-   break;
-   }
-
-   return -EINVAL;
-}
-
-static int adv7180_g_ctrl(struct v4l2_subdev *sd, struct v4l2_control *ctrl)
-{
-   struct adv7180_state *state = to_state(sd);
-   int ret = mutex_lock_interruptible

[PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180)

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
 drivers/media/pci/sta2x11/Kconfig   |2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1239 ++-
 2 file modificati, 409 inserzioni(+), 832 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..654339f 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
tristate STA2X11 VIP Video For Linux
depends on STA2X11
select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-   select VIDEOBUF_DMA_CONTIG
+   select VIDEOBUF2_DMA_STREAMING
depends on PCI  VIDEO_V4L2  VIRT_TO_BUS
help
  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index 4c10205..ee61acc 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,7 +1,11 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
+ * author: Federico Vaga federico.v...@gmail.com
  * Copyright (C) 2010   WindRiver Systems, Inc.
+ * authors: Andreas Kies andreas.k...@windriver.com
+ *  Vlad Lungu   vlad.lu...@windriver.com
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -19,36 +23,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called COPYING.
  *
- * Author: Andreas Kies andreas.k...@windriver.com
- * Vlad Lungu vlad.lu...@windriver.com
- *
  */
 
 #include linux/types.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/init.h
-#include linux/vmalloc.h
-
 #include linux/videodev2.h
-
 #include linux/kmod.h
-
 #include linux/pci.h
 #include linux/interrupt.h
-#include linux/mutex.h
 #include linux/io.h
 #include linux/gpio.h
 #include linux/i2c.h
 #include linux/delay.h
 #include media/v4l2-common.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-ioctl.h
-#include media/videobuf-dma-contig.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
+#include media/videobuf2-dma-streaming.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +61,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,43 +82,20 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
+
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
 
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
@@ -129,21 +104,27 @@ struct

Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

Sorry for the late answer to this.

  This allocator is needed because some device (like STA2X11 VIP) cannot
  work
  with DMA sg or DMA coherent. Some other device (like the one used by
  Jonathan when he proposes vb2-dma-nc allocator) can obtain much better
  performance with DMA streaming than coherent.
 
 Ok, please add such explanations at the patch's descriptions, as it is
 important not only for me, but to others that may need to use it..

OK

 2) why vb2-dma-config can't be patched to use dma_map_single
  
  (eventually using a different vb2_io_modes bit?);
  
  I did not modify vb2-dma-contig because I was thinking that each DMA
  memory
  allocator should reflect a DMA API.
 
 The basic reason for having more than one VB low-level handling (vb2 was
 inspired on this concept) is that some DMA APIs are very different than
 the other ones (see vmalloc x DMA S/G for example).
 
 I didn't make a diff between videobuf2-dma-streaming and
 videobuf2-dma-contig, so I can't tell if it makes sense to merge them or
 not, but the above argument seems too weak. I was expecting for a technical
 reason why it wouldn't make sense for merging them.

I cannot work on this now. But I think that I can do an integration like the 
one that I pushed some month ago (a8f3c203e19b702fa5e8e83a9b6fb3c5a6d1cce4).
Wind River made that changes to videobuf-contig and I tested, fixed and 
pushed.

 3) what are the usecases for it.
  
  Could you please detail it? Without that, one that would be needing to
  write a driver will have serious doubts about what would be the right
  driver for its usage. Also, please document it at the driver itself.

I don't have a full understand of the board so I don't know exactly why 
dma_alloc_coherent does not work. I focused my development on previous work by 
Wind River. I asked to Wind River (which did all the work on this board) for 
the technical explanation about why coherent doesn't work, but they do not 
know. That's why I made the new allocator: coherent doesn't work and HW 
doesn't support SG.

 I'm not a DMA performance expert. As such, from that comment, it sounded to
 me that replacing dma-config/dma-sg by dma streaming will always give
 performance optimizations the hardware allow.

me too, I'm not a DMA performance expert. I'm just an user of the DMA API. On 
my hardware simply it works only with that interface, it is not a performance 
problem.

 On a separate but related issue, while doing DMABUF tests with an Exynos4
 hardware, using a s5p sensor, sending data to s5p-tv, I noticed a CPU
 consumption of about 42%, which seems too high. Could it be related to
 not using the DMA streaming API?

As I wrote above, I'm not a DMA performance expert. I skip this

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH RFC] spidev.c: add sysfs attributes for SPI configuration

[Federico Vaga]

This patch introduce the use of the sysfs attribute for the spidev
configuration. This avoid the user to have a specific program which does
ioctl() on spidev. The user can easily does cat (to read) and echo (to
write) on the sysfs file and configure SPI.

The patch exports the following attributes: bits-per-word, lsb-first,
mode and speed-hz.

Example:
# cat /sys/bus/spi/devices/spi1.0/speed-hz
50
# echo 45  /sys/bus/spi/devices/spi1.0/speed-hz
# dmesg | tail -n 4
spidev spi1.0: DEactivate 60, mr 000f0011
spidev spi1.0: setup: 449447 Hz bpw 8 mode 0x0 - csr0 dd02
spidev spi1.0: setup mode 0, 8 bits/w, 45 Hz max -- 0
spidev spi1.0: 45 Hz (max)

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/spi/spidev.c | 258 +--
 1 file modificato, 208 inserzioni(+), 50 rimozioni(-)

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 830adbe..4aa0832 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -31,6 +31,7 @@
 #include linux/mutex.h
 #include linux/slab.h
 #include linux/compat.h
+#include linux/sysfs.h
 
 #include linux/spi/spi.h
 #include linux/spi/spidev.h
@@ -92,6 +93,201 @@ static unsigned bufsiz = 4096;
 module_param(bufsiz, uint, S_IRUGO);
 MODULE_PARM_DESC(bufsiz, data bytes in biggest supported SPI message);
 
+
+/*-*/
+
+/* SYSFS */
+enum spidev_config_enum {
+   SPIDEV_SPEED_HZ,
+   SPIDEV_BIT_PER_WORD,
+   SPIDEV_LSB_FIRST,
+   SPIDEV_MODE,
+};
+struct spidev_config_attr {
+   struct device_attribute attr;
+   enum spidev_config_enum cmd;
+};
+#define to_spidev_attr(_attr) \
+   container_of(_attr, struct spidev_config_attr, attr)
+
+static int spidev_conf_mode(struct spi_device *spi, u32 tmp)
+{
+   u8 save = spi-mode;
+   int err = 0;
+
+   if (tmp  ~SPI_MODE_MASK)
+   return -EINVAL;
+
+   tmp |= spi-mode  ~SPI_MODE_MASK;
+   spi-mode = (u8)tmp;
+   err = spi_setup(spi);
+   if (err  0)
+   spi-mode = save;
+   else
+   dev_dbg(spi-dev, spi mode %02x\n, tmp);
+
+   return err;
+}
+static int spidev_conf_lsb(struct spi_device *spi, u32 tmp)
+{
+   u8 save = spi-mode;
+   int err = 0;
+
+   if (tmp)
+   spi-mode |= SPI_LSB_FIRST;
+   else
+   spi-mode = ~SPI_LSB_FIRST;
+   err = spi_setup(spi);
+   if (err  0)
+   spi-mode = save;
+   else
+   dev_dbg(spi-dev, %csb first\n, (tmp ? 'l' : 'm'));
+
+   return err;
+}
+static int spidev_conf_bpw(struct spi_device *spi, u32 tmp)
+{
+   u8 save = spi-bits_per_word;
+   int err = 0;
+
+   spi-bits_per_word = tmp;
+   err = spi_setup(spi);
+   if (err  0)
+   spi-bits_per_word = save;
+   else
+   dev_dbg(spi-dev, %d bits per word\n, tmp);
+
+   return err;
+}
+static int spidev_conf_speedhz(struct spi_device *spi, u32 tmp)
+{
+   u32 save = spi-max_speed_hz;
+   int err = 0;
+
+   spi-max_speed_hz = tmp;
+   err = spi_setup(spi);
+   if (err  0)
+   spi-max_speed_hz = save;
+   else
+   dev_dbg(spi-dev, %d Hz (max)\n, tmp);
+
+   return err;
+}
+
+/* Return to user space the current SPI configuration */
+static ssize_t spidev_show(struct device *dev, struct device_attribute *attr,
+   char *buf)
+{
+   struct spidev_config_attr *sattr = to_spidev_attr(attr);
+   struct spidev_data *spidev;
+   struct spi_device *spi;
+   ssize_t count = 0;
+
+   spidev = spi_get_drvdata(to_spi_device(dev));
+
+   spin_lock_irq(spidev-spi_lock);
+   spi = spi_dev_get(spidev-spi);
+   spin_unlock_irq(spidev-spi_lock);
+
+   mutex_lock(spidev-buf_lock);
+   switch (sattr-cmd) {
+   case SPIDEV_MODE:
+   count = sprintf(buf, %d\n, (spi-mode  SPI_MODE_MASK));
+   break;
+   case SPIDEV_LSB_FIRST:
+   count = sprintf(buf, %d\n,
+   ((spi-mode  SPI_LSB_FIRST) ?  1 : 0));
+   break;
+   case SPIDEV_BIT_PER_WORD:
+   count = sprintf(buf, %d\n, spi-bits_per_word);
+   break;
+   case SPIDEV_SPEED_HZ:
+   count = sprintf(buf, %d\n, spi-max_speed_hz);
+   break;
+   }
+   mutex_unlock(spidev-buf_lock);
+   spi_dev_put(spi);
+
+   return count;
+}
+/* Configure the SPI from userspace */
+static ssize_t spidev_store(struct device *dev, struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   struct spidev_config_attr *sattr = to_spidev_attr(attr);
+   struct spidev_data *spidev;
+   struct spi_device *spi;
+   int err = 0;
+   u32 tmp;
+
+   spidev = spi_get_drvdata(to_spi_device(dev));
+
+   spin_lock_irq

Re: [PATCH RFC] spidev.c: add sysfs attributes for SPI configuration

[Federico Vaga]

On Wednesday 19 December 2012 15:09:25 Grant Likely wrote:
 Not a good idea. sysfs is not a good place for operational
 interfaces. Please use the spi character devices for direct
 manipulation of the SPI configuration.

Hello,

Can you explain why it is not a good idea? I do not understand; what 
is the advantage of ioctl through char device? Or what it the issue 
with sysfs?

Thank you very much


-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

 I can take a look at the dma coherent issues with that board, but I 
will
 need some help as I don't have this hardware.

I have the hardware, but I don't have the full knowledge of the 
boards. As I told before, I asked to windriver which develop the 
software for the whole board, but they cannot help me.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH RFC] spidev.c: add sysfs attributes for SPI configuration

[Federico Vaga]

 I'm cautious about adding operational interfaces to sysfs because it
 can be quite difficult to get the locking right. To begin with it
 splits up a single interface into multiple files, any of which can
 be held open by a process. Then there is the question of ordering
 of operations when there are multiple users. For instance, if there
 were two users, each of which using different transfer parameters,
 a sysfs interface doesn't provide any mechanism to group setting up
 the device with the transfer.
 
 These are lessons learned the hard way with the gpio sysfs abi. I
 don't want to get caught in the same trap for spi.
 
 g.

I understand the problem, but I think that for very simple test on 
devices, sysfs is easier. For example, it happens that a SPI device 
does not work correctly with a driver, so I want to verify the SPI 
traffic by writing directly the device commands and check with an 
oscilloscope. I think that an easy way is to use sysfs like this:

echo 123456  speed_hz
[other options if needed]
echo  -n -e \x12\x34  /dev/spidev1.1
[oscilloscope]
hexdump -n 2 /dev/spidev1.1

This sysfs interface should be used only for testing/debugging, not to 
develop an user space driver on it; moreover, the ioctl interface is 
still there.

spidev_test and spidev_fdx does not allow me to customize tx buffer and 
I think (very personal opinion) that for debugging purpose is better 
sysfs with well known programs (echo, cat, hexdump, od) and 
oscilloscope. 

I know that I'm not so persuasive :) I can develop a simple program 
that can write custom tx buf with ioctl

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

 After all those discussions, I'm ok on adding this new driver, but please
 add a summary of those discussions at the patch description. As I said,
 the reason why this driver is needed is not obvious. So, it needs to be
 very well described.

ack. I will ask more information to ST about the board because the 
architecture side it is not in the kernel mainline, but it should be.

 Patch 1/4 of this series doesn't apply anymore (maybe it were already
 applied?).

Probably already applied

 So, could you please send us a v4, rebased on the top of staging/for_v3.9
 branch of the media-tree?

I will do it

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

On Thursday 03 January 2013 17:13:14 Federico Vaga wrote:
  After all those discussions, I'm ok on adding this new driver, but please
  add a summary of those discussions at the patch description. As I said,
  the reason why this driver is needed is not obvious. So, it needs to be
  very well described.
 
 ack. I will ask more information to ST about the board because the
 architecture side it is not in the kernel mainline, but it should be.

I have more information about DMA on the board that I'm using; probably, I can 
make dma-contig work with my device. Unfortunately, I cannot test at the 
moment; I hope to do a test on Monday.


-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator

[Federico Vaga]

 I have more information about DMA on the board that I'm using; probably, I
 can make dma-contig work with my device.

Ok, the driver STA2X11 now works with a patched dma-contig allocator. So, my 
streaming allocator it is not mandatory. 

I based my work on the previous work made by Windriver, but now I understand 
the DMA problem and the solution easy.
I investigated (asked to Alessandro Rubini who worked on this board) about 
this DMA issue. The problem is that on the sta2x11 architecture only the first 
512MB are available through the PCI bus, but the allocator can allocate memory 
for DMA above this limit. By using GFP_DMA flags the allocation take place 
under the 16MB so it works.

If you think that the streaming allocator can be useful for someone else (who 
has performance problem with uncached DMA like Jonathan when he did dma-nc 
allocator), I can resend the patch.
I cannot do performance test at the moment because I don't have the time, so I 
cannot personally justify the presence of a new allocator. I think that I will 
do some performance test with this driver; if I will find that dma-streaming 
works better I will propose it again.

I will propose V4 patches soon.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v4 1/3] videobuf2-dma-contig: user can specify GFP flags

[Federico Vaga]

This is useful when you need to specify specific GFP flags during memory
allocation (e.g. GFP_DMA).

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 7 ++-
 include/media/videobuf2-dma-contig.h   | 5 +
 2 file modificati, 7 inserzioni(+), 5 rimozioni(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 10beaee..bb411c0 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -21,10 +21,6 @@
 #include media/videobuf2-dma-contig.h
 #include media/videobuf2-memops.h
 
-struct vb2_dc_conf {
-   struct device   *dev;
-};
-
 struct vb2_dc_buf {
struct device   *dev;
void*vaddr;
@@ -165,7 +161,8 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long 
size)
/* align image size to PAGE_SIZE */
size = PAGE_ALIGN(size);
 
-   buf-vaddr = dma_alloc_coherent(dev, size, buf-dma_addr, GFP_KERNEL);
+   buf-vaddr = dma_alloc_coherent(dev, size, buf-dma_addr,
+   
GFP_KERNEL | conf-mem_flags);
if (!buf-vaddr) {
dev_err(dev, dma_alloc_coherent of size %ld failed\n, size);
kfree(buf);
diff --git a/include/media/videobuf2-dma-contig.h 
b/include/media/videobuf2-dma-contig.h
index 8197f87..22733f4 100644
--- a/include/media/videobuf2-dma-contig.h
+++ b/include/media/videobuf2-dma-contig.h
@@ -16,6 +16,11 @@
 #include media/videobuf2-core.h
 #include linux/dma-mapping.h
 
+struct vb2_dc_conf {
+   struct device   *dev;
+   gfp_t   mem_flags;
+};
+
 static inline dma_addr_t
 vb2_dma_contig_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
 {
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH V4 2/3] sta2x11_vip: convert to videobuf2 and control framework

[Federico Vaga]

This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180)

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
 drivers/media/pci/sta2x11/Kconfig   |2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1244 ++-
 2 file modificati, 414 inserzioni(+), 832 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig 
b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..a94ccad 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
tristate STA2X11 VIP Video For Linux
depends on STA2X11
select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-   select VIDEOBUF_DMA_CONTIG
+   select VIDEOBUF2_DMA_CONTIG
depends on PCI  VIDEO_V4L2  VIRT_TO_BUS
help
  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c 
b/drivers/media/pci/sta2x11/sta2x11_vip.c
index ed1337a..e379e03 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,7 +1,11 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012   ST Microelectronics
+ * author: Federico Vaga federico.v...@gmail.com
  * Copyright (C) 2010   WindRiver Systems, Inc.
+ * authors: Andreas Kies andreas.k...@windriver.com
+ *  Vlad Lungu   vlad.lu...@windriver.com
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -19,36 +23,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called COPYING.
  *
- * Author: Andreas Kies andreas.k...@windriver.com
- * Vlad Lungu vlad.lu...@windriver.com
- *
  */
 
 #include linux/types.h
 #include linux/kernel.h
 #include linux/module.h
 #include linux/init.h
-#include linux/vmalloc.h
-
 #include linux/videodev2.h
-
 #include linux/kmod.h
-
 #include linux/pci.h
 #include linux/interrupt.h
-#include linux/mutex.h
 #include linux/io.h
 #include linux/gpio.h
 #include linux/i2c.h
 #include linux/delay.h
 #include media/v4l2-common.h
 #include media/v4l2-device.h
+#include media/v4l2-ctrls.h
 #include media/v4l2-ioctl.h
-#include media/videobuf-dma-contig.h
+#include media/v4l2-fh.h
+#include media/v4l2-event.h
+#include media/videobuf2-dma-contig.h
 
 #include sta2x11_vip.h
 
-#define DRV_NAME sta2x11_vip
 #define DRV_VERSION 1.3
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +61,8 @@
 #define DVP_TFS0x08
 #define DVP_BFO0x0C
 #define DVP_BFS0x10
-#define DVP_VTP 0x14
-#define DVP_VBP 0x18
+#define DVP_VTP0x14
+#define DVP_VBP0x18
 #define DVP_VMP0x1C
 #define DVP_ITM0x98
 #define DVP_ITS0x9C
@@ -84,43 +82,20 @@
 
 #define DVP_HLFLN_SD   0x0001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip-iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip-iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
+
+struct vip_buffer {
+   struct vb2_buffer   vb;
+   struct list_headlist;
+   dma_addr_t  dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+   return container_of(vb2, struct vip_buffer, vb);
+}
 
 struct sta2x11_vip {
struct v4l2_device v4l2_dev;
@@ -129,21 +104,27 @@ struct

[PATCH V4 3/3] adv7180: remove {query/g_/s_}ctrl

[Federico Vaga]

All drivers which use this subdevice use also the control framework.
The v4l2_subdev_core_ops operations {query/g_/s_}ctrl are useless because
device drivers will inherit controls from this subdevice.

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 drivers/media/i2c/adv7180.c | 3 ---
 1 file modificato, 3 rimozioni(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 45ecf8d..43bc2b9 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -402,9 +402,6 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops 
= {
 static const struct v4l2_subdev_core_ops adv7180_core_ops = {
.g_chip_ident = adv7180_g_chip_ident,
.s_std = adv7180_s_std,
-   .queryctrl = v4l2_subdev_queryctrl,
-   .g_ctrl = v4l2_subdev_g_ctrl,
-   .s_ctrl = v4l2_subdev_s_ctrl,
 };
 
 static const struct v4l2_subdev_ops adv7180_ops = {
-- 
1.7.11.7

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] videobuf2-dma-contig: user can specify GFP flags

[Federico Vaga]

  @@ -165,7 +161,8 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned
  long size) 
  /* align image size to PAGE_SIZE */
  size = PAGE_ALIGN(size);
  
  -   buf-vaddr = dma_alloc_coherent(dev, size, buf-dma_addr, 
GFP_KERNEL);
  +   buf-vaddr = dma_alloc_coherent(dev, size, buf-dma_addr,
  +   
GFP_KERNEL | conf-mem_flags);
 
 I think we can add GFP_DMA flag unconditionally to the vb2_dc_contig
 allocator.
 It won't hurt existing clients as most of nowadays platforms doesn't
 have DMA
 zone (GFP_DMA is ignored in such case), but it should fix the issues
 with some
 older and non-standard systems.

I did not set GFP_DMA fixed in the allocator because I do not want to brake 
something in the future. On x86 platform GFP_DMA allocates under 16MB and this 
limit can be too strict. When many other drivers use GFP_DMA we can saturate 
this tiny zone.
As you said, this fix the issue with _older_ and _non-standard_ (like sta2x11) 
systems. But this fix has effect on every other standard and new systems. 
That's why I preferred to set the flag optionally.

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] videobuf2-dma-contig: user can specify GFP flags

[Federico Vaga]

 Ok, then I would simply pass the flags from the driver without any
 alternation
 in the allocator itself, so drivers can pass 'GFP_KERNEL' or
 'GFP_KERNEL | GFP_DMA' depending on their preference. Please also update
 all
 the existing clients of vb2_dma_dc allocator.

I taked a look at drivers that use dma-contig. They use the structure 
vb2_alloc_ctx which is just a name, there is not a real vb2_alloc_ctx 
structure implementation. Now driver must gain access to vb2_dc_conf to set 
the correct flags.

I have the following ideas:

  1.  replace all the names and expose vb2_dc_conf to all drivers (like dma-
sg, it export vb2_dma_sg_desc to all its users)

  2.  create an helper which configure flags. This maintain the vb2_dc_conf 
private
  vb2_set_mem_flags(struct vb2_alloc_ctx *alloc_ctx, gfp_t flags)

  3.  rename vb2_dc_conf to vb2_alloc_ctx because it is an implementation 
vb2_alloc_ctx and (at the moment) it is used only by dma-contig

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

DWC2 and/or S3C-HSOTG for STA2X11 board

[Federico Vaga]

Hello,

I have an x86 board made by STMicroelectronics (STA2X11) with the Synopsis 
USB-OTG DesignWare 2 on it and connected through the PCI-e bus.


I know that there are two drivers for the same controller:

   (host)   drivers/staging/dwc2/*
   (device) drivers/usb/gadget/s3c-hsotg.{c|h}


So, at the moment I cannot have a board with both host/device working at the 
same time. I have to choose to use the block as device or host, right?

I know that the plan is to merge the s3c-hsotg in the dwc2 driver 
(https://lwn.net/Articles/540283/). Are still accepted patch to s3c-hsotg? Or 
it is work in progress right now (soon), so it is better to wait after the 
merge?

In order to use the s3c-hsotg I must implement a PCI wrapper that uses this 
driver. It will be accepted in the kernel even if it will be removed sooner or 
later because of the driver merge?

Thank you :)

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: DWC2 and/or S3C-HSOTG for STA2X11 board

[Federico Vaga]

Thank you Felipe

[add CC Giancarlo from ST]

On Tuesday 16 July 2013 15:04:25 Felipe Balbi wrote:
 Hi,
 
 On Tue, Jul 16, 2013 at 02:01:33PM +0200, Federico Vaga wrote:
  Hello,
  
  I have an x86 board made by STMicroelectronics (STA2X11) with the Synopsis
  USB-OTG DesignWare 2 on it and connected through the PCI-e bus.
  
  I know that there are two drivers for the same controller:
 (host)   drivers/staging/dwc2/*
 (device) drivers/usb/gadget/s3c-hsotg.{c|h}
  
  So, at the moment I cannot have a board with both host/device working at
  the same time. I have to choose to use the block as device or host,
  right?
  
  I know that the plan is to merge the s3c-hsotg in the dwc2 driver
  (https://lwn.net/Articles/540283/). Are still accepted patch to s3c-hsotg?
  Or it is work in progress right now (soon), so it is better to wait after
  the merge?
  
  In order to use the s3c-hsotg I must implement a PCI wrapper that uses
  this
  driver. It will be accepted in the kernel even if it will be removed
  sooner or later because of the driver merge?
 
 currently s3c-hsotg has too much knowledge of the Samsung platform. My
 suggestion would be to help dwc2 get in better shape. It should be
 rather easy to support your board since that already has a PCI wrapper
 driver.
 
 So, stick to host only for now, help clean up dwc2 and move it out of
 staging, then later it should be fairly simple to merge the device side
 in it.

Is there something like a TODO list of dwc2 known problems?

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[RFC] assign some address families for local use

[Federico Vaga]

Hello,

We are working on new protocols and we think that is useful to have some 
address protocol families index assigned for local use. So we will not have 
conflict every time a new protocol is included within the Linux kernel.
Doubt: index 27 and 28 are not assigned to any address family, can be 
explicitly assigned for local use?

We also thought to increase AF_MAX to 64 to avoid to modify it every time.
Doubt: array like af_family_key_strings (net/core/sock.c) will have some NULL 
pointer. I see that a string is specified also for index 27 and 28 even if 
there is not a protocol assigned for these. Is a NULL string a problem for 
these vectors? Typically is used in this way:

af_family_clock_key_strings[newsk-sk_family]

So, if I set sk_family with an unassigned index I will have a NULL pointer and 
a DEBUG_LOCK_WARN_ON() from lockdep_init_map() (kernel/lockdep.c)


I attached to this email the patch that do these stuff.

-- 
Federico VagaFrom 8ce4f2576aa8e95ea22921c31bdffd049460951d Mon Sep 17 00:00:00 2001
From: Federico Vaga federico.v...@gmail.com
Date: Wed, 15 May 2013 12:32:03 +0200
Subject: [PATCH] include/linux/socket.h: assign address families for local use

The patch assigns 4 address families for local use only. This is
useful because it allows to maintain an address family outside kernel
source without conflict. It is also useful during development until a
number is officially assigned.

This is the same kind of policy applied for major number
(Documentation/devices.text)

This patch also increases the number of maximum address (protocol)
families to 64. In this way for a while nobody need to increase this
value. The cost, in terms of memory, is tiny. I made an (very)
approximate calculation about the cost of an unused address family by
following NPROTO, AF_MAX and PF_MAX usage. If I did not big errors it
should be about 70Byte on 32bit systems and 130Byte on 64bit systems for
each new address family.

I also compiled a kernel on a x86_64 machine:
Without patch
   text	   data	bss	dec	hex		filename
10935491 1398904 1175552 13509947  ce253b	vmlinux

With patch
   text	   data	bss	dec	hex		filename
10935427 1399544 1175552 13510523  ce277b	vmlinux

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 include/linux/socket.h | 12 +++-
 net/core/sock.c| 12 +---
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/include/linux/socket.h b/include/linux/socket.h
index 428c37a..4775d69 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -179,7 +179,12 @@ struct ucred {
 #define AF_ALG		38	/* Algorithm sockets		*/
 #define AF_NFC		39	/* NFC sockets			*/
 #define AF_VSOCK	40	/* vSockets			*/
-#define AF_MAX		41	/* For now.. */
+#define AF_LOCAL1	41	/* Local use sockets		*/
+#define AF_LOCAL2	42	/* Local use sockets		*/
+#define AF_LOCAL3	43	/* Local use sockets		*/
+#define AF_LOCAL4	44	/* Local use sockets		*/
+/* new address families here */
+#define AF_MAX		64
 
 /* Protocol families, same as address families. */
 #define PF_UNSPEC	AF_UNSPEC
@@ -223,6 +228,11 @@ struct ucred {
 #define PF_ALG		AF_ALG
 #define PF_NFC		AF_NFC
 #define PF_VSOCK	AF_VSOCK
+#define PF_LOCAL1	AF_LOCAL1
+#define PF_LOCAL2	AF_LOCAL2
+#define PF_LOCAL3	AF_LOCAL3
+#define PF_LOCAL4	AF_LOCAL4
+/* new protocol families here */
 #define PF_MAX		AF_MAX
 
 /* Maximum queue length specifiable by listen.  */
diff --git a/net/core/sock.c b/net/core/sock.c
index 6ba327d..9bf66ab 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -210,7 +210,9 @@ static const char *const af_family_key_strings[AF_MAX+1] = {
   sk_lock-AF_TIPC  , sk_lock-AF_BLUETOOTH, sk_lock-IUCV,
   sk_lock-AF_RXRPC , sk_lock-AF_ISDN , sk_lock-AF_PHONET   ,
   sk_lock-AF_IEEE802154, sk_lock-AF_CAIF , sk_lock-AF_ALG  ,
-  sk_lock-AF_NFC   , sk_lock-AF_MAX
+  sk_lock-AF_NFC   , sk_lock-AF_LOCAL1   , sk_lock-AF_LOCAL2   ,
+  sk_lock-AF_LOCAL3, sk_lock-AF_LOCAL4   ,
+  [AF_MAX] = sk_lock-AF_MAX
 };
 static const char *const af_family_slock_key_strings[AF_MAX+1] = {
   slock-AF_UNSPEC, slock-AF_UNIX , slock-AF_INET ,
@@ -226,7 +228,9 @@ static const char *const af_family_slock_key_strings[AF_MAX+1] = {
   slock-AF_TIPC  , slock-AF_BLUETOOTH, slock-AF_IUCV ,
   slock-AF_RXRPC , slock-AF_ISDN , slock-AF_PHONET   ,
   slock-AF_IEEE802154, slock-AF_CAIF , slock-AF_ALG  ,
-  slock-AF_NFC   , slock-AF_MAX
+  slock-AF_NFC   , slock-AF_LOCAL1   , slock-AF_LOCAL2   ,
+  slock-AF_LOCAL3, slock-AF_LOCAL4   ,
+  [AF_MAX] = slock-AF_MAX
 };
 static const char *const af_family_clock_key_strings[AF_MAX+1] = {
   clock-AF_UNSPEC, clock-AF_UNIX , clock-AF_INET ,
@@ -242,7 +246,9 @@ static const char *const af_family_clock_key_strings[AF_MAX+1] = {
   clock-AF_TIPC  , clock-AF_BLUETOOTH, clock-AF_IUCV ,
   clock-AF_RXRPC , clock-AF_ISDN , clock-AF_PHONET   ,
   clock-AF_IEEE802154, clock-AF_CAIF , clock-AF_ALG  ,
-  clock-AF_NFC   , clock-AF_MAX

[PATCH] net/core/sock.c: add missing VSOCK string in af_family_*_key_strings

[Federico Vaga]

The three arrays of strings: af_family_kay_strings,
af_family_slock_key_strings and af_family_clock_key_strings have not
VSOCK's string

Signed-off-by: Federico Vaga federico.v...@gmail.com
---
 net/core/sock.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 6ba327d..88868a9 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -210,7 +210,7 @@ static const char *const af_family_key_strings[AF_MAX+1] = {
   sk_lock-AF_TIPC  , sk_lock-AF_BLUETOOTH, sk_lock-IUCV,
   sk_lock-AF_RXRPC , sk_lock-AF_ISDN , sk_lock-AF_PHONET   ,
   sk_lock-AF_IEEE802154, sk_lock-AF_CAIF , sk_lock-AF_ALG  ,
-  sk_lock-AF_NFC   , sk_lock-AF_MAX
+  sk_lock-AF_NFC   , sk_lock-AF_VSOCK, sk_lock-AF_MAX
 };
 static const char *const af_family_slock_key_strings[AF_MAX+1] = {
   slock-AF_UNSPEC, slock-AF_UNIX , slock-AF_INET ,
@@ -226,7 +226,7 @@ static const char *const 
af_family_slock_key_strings[AF_MAX+1] = {
   slock-AF_TIPC  , slock-AF_BLUETOOTH, slock-AF_IUCV ,
   slock-AF_RXRPC , slock-AF_ISDN , slock-AF_PHONET   ,
   slock-AF_IEEE802154, slock-AF_CAIF , slock-AF_ALG  ,
-  slock-AF_NFC   , slock-AF_MAX
+  slock-AF_NFC   , slock-AF_VSOCK,slock-AF_MAX
 };
 static const char *const af_family_clock_key_strings[AF_MAX+1] = {
   clock-AF_UNSPEC, clock-AF_UNIX , clock-AF_INET ,
@@ -242,7 +242,7 @@ static const char *const 
af_family_clock_key_strings[AF_MAX+1] = {
   clock-AF_TIPC  , clock-AF_BLUETOOTH, clock-AF_IUCV ,
   clock-AF_RXRPC , clock-AF_ISDN , clock-AF_PHONET   ,
   clock-AF_IEEE802154, clock-AF_CAIF , clock-AF_ALG  ,
-  clock-AF_NFC   , clock-AF_MAX
+  clock-AF_NFC   , clock-AF_VSOCK, clock-AF_MAX
 };
 
 /*
-- 
1.8.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] dwc2/pci.c: add STMICRO vendor and device ID for STA2X11 board

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@gmail.com
Acked-by: Giancarlo Asnaghi giancarlo.asna...@st.com
---
 drivers/staging/dwc2/pci.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/dwc2/pci.c b/drivers/staging/dwc2/pci.c
index 69c65eb..7029b9f 100644
--- a/drivers/staging/dwc2/pci.c
+++ b/drivers/staging/dwc2/pci.c
@@ -162,6 +162,10 @@ static DEFINE_PCI_DEVICE_TABLE(dwc2_pci_ids) = {
{
PCI_DEVICE(PCI_VENDOR_ID_SYNOPSYS, PCI_PRODUCT_ID_HAPS_HSOTG),
},
+   {
+   PCI_DEVICE(PCI_VENDOR_ID_STMICRO,
+  PCI_DEVICE_ID_STMICRO_USB_OTG),
+   },
{ /* end: all zeroes */ }
 };
 MODULE_DEVICE_TABLE(pci, dwc2_pci_ids);
-- 
1.8.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: DWC2 and/or S3C-HSOTG for STA2X11 board

[Federico Vaga]

Hi Paul,

Sorry for the delayed answer :(

 As part of the merge, we will need to develop a PCI wrapper for
 s3c-hsotg anyway, so I think it would not be wasted effort.
 Actually, as a POC I already did this as a quick hack, just to
 make sure that the driver will work on our PCIe prototyping
 platform (it does).

 As Felipe says, currently s3c-hsotg does have too much knowledge
 of Samsung platform. But it should be fairly easy to move that
 knowledge from the core code to a platform-device wrapper,
 similar to platform.c in the dwc2 driver. So if you would like
 to work on that (creating a PCI wrapper and a platform wrapper)
 I think it would be useful.

 If you want, I can send you my hacked-up code for the PCI
 version of the driver, to use as a starting point.

Yes, it will be really useful, thanks.

I will try to do both wrapper (PCI, platform), but I do not know how much
time does it takes because I am really busy at the moment

You know the hardware better than me, so: have you other suggestion
to point me on the right way?

Thank you :)

--
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] ipoctal: protect only the real critical section

[Federico Vaga]

In some conditions (echo or particular sequence of special
characters), on buffer push, the tty layer calls the write operation
while we are holding the spinlock. This means deadlock within the same
process on kernels version  3.12. It seems not a problem on recent
kernel, but the patch still valid as locking optimization.

The protected variables by the spinlock are: xmit_buf, nb_bytes,
pointer_read and pointer_write. So, this patch reduces the locked area
in the IRQ handler only to these variables. Most of the code inside the
locked area in the IRQ handler is not protected elsewhere; it means
that it is not protected at all.

Signed-off-by: Federico Vaga federico.v...@cern.ch
---
 drivers/ipack/devices/ipoctal.c |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/ipack/devices/ipoctal.c b/drivers/ipack/devices/ipoctal.c
index 141094e..69687f1 100644
--- a/drivers/ipack/devices/ipoctal.c
+++ b/drivers/ipack/devices/ipoctal.c
@@ -177,19 +177,20 @@ static void ipoctal_irq_tx(struct ipoctal_channel 
*channel)
if (channel-nb_bytes == 0)
return;
 
+   spin_lock(channel-lock);
value = channel-tty_port.xmit_buf[*pointer_write];
iowrite8(value, channel-regs-w.thr);
channel-stats.tx++;
(*pointer_write)++;
*pointer_write = *pointer_write % PAGE_SIZE;
channel-nb_bytes--;
+   spin_unlock(channel-lock);
 }
 
 static void ipoctal_irq_channel(struct ipoctal_channel *channel)
 {
u8 isr, sr;
 
-   spin_lock(channel-lock);
/* The HW is organized in pair of channels.  See which register we need
 * to read from */
isr = ioread8(channel-block_regs-r.isr);
@@ -213,8 +214,6 @@ static void ipoctal_irq_channel(struct ipoctal_channel 
*channel)
/* TX of each character */
if ((isr  channel-isr_tx_rdy_mask)  (sr  SR_TX_READY))
ipoctal_irq_tx(channel);
-
-   spin_unlock(channel-lock);
 }
 
 static irqreturn_t ipoctal_irq_handler(void *arg)
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: PCIe bus enumeration

[Federico Vaga]

On Friday 04 July 2014 15:26:12 Bjorn Helgaas wrote:
 On Fri, Jul 04, 2014 at 09:55:20AM +0200, Federico Vaga wrote:
   I assume these ports don't support hotplug.  If they *did*
   support
   hotplug, those ports would have to exist because they handle the
   hotplug events (presence detect, etc.)
  
  I asked: yes, they do not support hotplug
  
   If you can collect the complete lspci -vv output from your
   machine (with a device plugged in, so we can see the port
   leading to it), that will help make this more concrete.  And
   maybe one with no devices plugged in, so we can see exactly
   what changes.
  
  I attached two files with the output. I putted a card in slot 10
  and took the output, then moved the card on slot 11 and took the
  output.
  
  As you can see with diff the bridge behind the slot disappear when
  it is empty.
 
 Perfect, thanks!  For some reason, it really helps me to be able to
 stare at the actual data.  Here's the situation with slot 10
 occupied:
 
   00:01.0 82Q35 Root Port to [bus 05]  PCIe SltCap slot #21
   05:00.0 CERN/ECP/EDU Device  slot 10
   00:1c.0 82801I Express Port 1 to [bus 04]PCIe SltCap slot #22
   00:1c.3 (not present at all)
   00:1c.4 82801I Express Port 5 to [bus 03]PCIe SltCap slot #0
   03:00.0 Realtek NIC
 
 and here it is with slot 11 occupied:
 
   00:01.0 (not present at all)
   00:1c.0 82801I Express Port 1 to [bus 05]PCIe SltCap slot #22
   00:1c.3 82801I Express Port 4 to [bus 04]PCIe SltCap slot #25
   04:00.0 CERN/ECP/EDU Device  slot 11
   00:1c.4 82801I Express Port 5 to [bus 03]PCIe SltCap slot #0
   03:00.0 Realtek NIC
 
 I'm pretty sure this is a function of your BIOS.  There are often
 device-specific ways to enable or disable individual devices (like
 the root ports here), and the BIOS is likely disabling these ports
 when there is nothing below them.  I don't know why it would turn
 off 00:1c.3 when its slot is empty, but it doesn't turn off
 00:1c.0, which also leads to an empty slot. But I don't think
 Linux is involved in this, and if the BIOS disables devices, there
 really isn't anything Linux can do about it.

It seems to happen also on some classic PC. I didn't experiment it 
by myself, some friends reported me this behavior in the recent past.

So, It looks like that some BIOS disable the bridge when there is 
nothing behind it. Why? Power save? :/

 If you can get to an EFI shell on this box, you might be able to
 confirm this with the pci command.  Booting Linux with
 pci=earlydump is similar in that it dumps PCI config space before
 we change anything.

yes I confirm, the bridge are not there if I don't plug the card.

 To solve this problem, I think you need slot information even when
 there's no hotplug.  This has been raised before [1, 2], and I
 think it's a good idea, but nobody has implemented it yet.

Yes, but if the BIOS disable the bridge there is nothing we can do.

 Another curious thing is that you refer to slot 10, but there's no
 obvious connection between that and the slot 21 in the PCIe
 capability of the Root Port leading to that slot.  But I guess you
 said the slots are in a backplane (they're not an integral part of
 the motherboard).  In that case, there's no way for the motherboard
 to know what the labels on the backplane are.

It is written on the backplane. I said slot 10 because I'm counting 
the available slot, but on the backplane they are 22, 25, and other 
no-consecutive numbers.

If I use `biosdecode` I can get that information, but only for the 
first level of bridges. On some backplane I have PCI bridges behind 
bridges, and in this case biosdecode doesn't help: it just tell me 
about the bridge on the motherboard.

At the moment, I'm using the PCI bridge address to make the 
association with a specific slot. When they are on they have always 
the same address. A colleague did a map between physical slot and PCI 
bridge address; from this we can extract the bus number and identify 
the cards. But well I was looking for better solutions :)

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: PCIe bus enumeration

[Federico Vaga]

(I'm changing my email address to the work one. Initially it was just 
my personal curiosity but now you are helping me with my work, so I 
think is correct in this way)

  So, It looks like that some BIOS disable the bridge when there is
  nothing behind it. Why? Power save? :/
 
 Could be power savings, or possibly to conserve bus numbers, which
 are a limited resource.

what is the maximum number of buses?

  If you can get to an EFI shell on this box, you might be able to
  confirm this with the pci command.  Booting Linux with
  pci=earlydump is similar in that it dumps PCI config space
  before
  we change anything.
  
  yes I confirm, the bridge are not there if I don't plug the card.
  
  To solve this problem, I think you need slot information even
  when
  there's no hotplug.  This has been raised before [1, 2], and I
  think it's a good idea, but nobody has implemented it yet.
  
  Yes, but if the BIOS disable the bridge there is nothing we can
  do.
 
 Well, it's true that it's hard to get constant *bus numbers*, but
 it's never really been a good idea to rely on those, because
 they're assigned at the discretion of the OS, and there are reasons
 why the OS might want to reallocate them, e.g., to accommodate a
 deep hot-plugged hierarchy.  If you shift focus to *slot numbers*,
 then I think there's a lot more we can do.

At this point I'm a little bit confused about the definition slot 
numbers :) You mean the 22, 25, ...

  Another curious thing is that you refer to slot 10, but there's
  no obvious connection between that and the slot 21 in the PCIe
  capability of the Root Port leading to that slot.  But I guess
  you said the slots are in a backplane (they're not an integral
  part of the motherboard).  In that case, there's no way for the
  motherboard to know what the labels on the backplane are.
  
  It is written on the backplane. I said slot 10 because I'm
  counting
  the available slot, but on the backplane they are 22, 25, and
  other
  no-consecutive numbers.
 
 The 22, 25, etc., are in the same range as the slot numbers in the
 PCIe Slot Capabilities registers, so maybe the backplane is
 constructed to make this possible.  The external PCIe chassis I'm
 familiar with have one fast link on a cable leading to the box, with
 a PCIe switch inside the box.  The upstream port is connected to
 the incoming link, and there's a downstream port connected to each
 slot. In this case, the slot numbers in the downstream ports' Slot
 Capabilities registers can be made to match the silkscreen labels
 on the board since everything is fixed by the hardware.
 
 Your backplane sounds a little different (you have Ports on the root
 bus leading directly to slots in the backplane, so I assume those
 Ports are on the motherboard, not the backplane), but maybe the
 motherboard  backplane are designed as a unit so the Port slot
 numbers could match the backplane.

Yes, the backplane is almost empty. Except for the 9 PCIe backplane 
which has PCI bridges on it. At the moment I cannot check physically 
this kind of backplane, but from the lspci output I understand that 
there is a bridge on the backplane because the motherboard is the 
same.

 
  If I use `biosdecode` I can get that information, but only for the
  first level of bridges. On some backplane I have PCI bridges
  behind bridges, and in this case biosdecode doesn't help: it just
  tell me about the bridge on the motherboard.
 
 What specific biosdecode information are you using? 

I was looking at the PCI interrupt routing, but it seems that it 
returns only information about the last bridge in the interrupt's 
routing. Here an example with a different backplane (9 PCIe). 

It seems fine for backplane without PCI Bridge on the backplane.

I attached two files, one for each type of backplane.


Maybe I'm just misunderstanding the output of biosdecode. I didn't 
find an explanation of its output: I'm guessing the meaning.

 There's a fair
 amount of stuff in the PCI-to-PCI bridge spec about slot and chassis
 numbering, including some about expansion chassis.  I doubt that
 Linux implements all that, so there's probably room for a lot of
 improvement.  I attached your lspci output to the bugzilla
 (https://bugzilla.kernel.org/show_bug.cgi?id=72681).  Maybe you
 could attach the biosdecode info there, too, and we could see if
 there's a way we can make this easier.

ok

-- 
Federico Vaga-[:00]-+-00.0
   +-01.0-[05]00.0
   +-02.0
   +-03.0
   +-03.2
   +-03.3
   +-19.0
   +-1a.0
   +-1a.1
   +-1a.2
   +-1a.7
   +-1b.0
   +-1c.0-[04]--
   +-1c.4-[03]00.0
   +-1d.0
   +-1d.1
   +-1d.2
   +-1d.7
   +-1e.0-[01-02]0c.0-[02]--
   +-1f.0
   +-1f.2
   +-1f.3
   +-1f.5
   \-1f.6


# biosdecode 2.12
BIOS32 Service Directory present.
Revision: 0

Re: PCIe bus enumeration

[Federico Vaga]

On Tuesday 08 July 2014 12:23:39 Bjorn Helgaas wrote:
 On Tue, Jul 8, 2014 at 1:15 AM, Federico Vaga 
federico.v...@cern.ch wrote:
   So, It looks like that some BIOS disable the bridge when there
   is
   nothing behind it. Why? Power save? :/
  
  Could be power savings, or possibly to conserve bus numbers,
  which
  are a limited resource.
  
  what is the maximum number of buses?
 
 256.

Well, it is not a small number. I will ask directly to the company who 
sell this crate and ask them what is going on in the BIOS

  At this point I'm a little bit confused about the definition slot
  numbers :) You mean the 22, 25, ...
 
 Right.  Bus numbers are under software control, to some degree (as a
 general rule, an x86 BIOS assigns them and Linux leaves them alone,
 but they *can* be changed so they aren't a good thing to rely on).
 The bus number of a root bus is usually determined by hardware or
 by an arch-specific host bridge driver.  The bus number below a
 PCI-PCI bridge is determined by the bridge's secondary bus number
 register, which software can change.
 
 Slot numbers are based on the Physical Slot Number in the PCIe Slot
 Capability register.  This is set by some hardware mechanism such as
 pin strapping or a serial EEPROM.  Software can't change it, so you
 can rely on it to be constant.  (There's also a mechanism for
 getting a slot number from ACPI, but that should also return a
 constant value).  The problem is that I don't think the Linux slot
 number support is very good, so I'm sure there's plenty of stuff
 that we *should* be able to do that we can't do *yet*.

Mh, I understand. Let's say that I have time to spend on this problem 
(I do not know) and contributing to the PCI subsystem. How many 
differences are there between 3.2, 3.6, 3.16/next? We are using 
3.2/3.6 at the moment, but probably you should expect that it will 
work on the last version :)

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] ipoctal: request_irq after configuration

[Federico Vaga]

The request for an IRQ handler must be done after whole configuration. This
was not the case for this driver which request the IRQ in the middle of
the configuration. Sometimes, it happens that something is not completely
configured, we recieve an interrupt thus we stumble into troubles in the
IRQ handler.

Signed-off-by: Federico Vaga federico.v...@cern.ch
---
 drivers/ipack/devices/ipoctal.c |   15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/ipack/devices/ipoctal.c b/drivers/ipack/devices/ipoctal.c
index a7ec6f9..72fd761 100644
--- a/drivers/ipack/devices/ipoctal.c
+++ b/drivers/ipack/devices/ipoctal.c
@@ -344,13 +344,6 @@ static int ipoctal_inst_slot(struct ipoctal *ipoctal, 
unsigned int bus_nr,
 block_regs[i].w.imr);
}
 
-   /*
-* IP-OCTAL has different addresses to copy its IRQ vector.
-* Depending of the carrier these addresses are accesible or not.
-* More info in the datasheet.
-*/
-   ipoctal-dev-bus-ops-request_irq(ipoctal-dev,
-  ipoctal_irq_handler, ipoctal);
/* Dummy write */
iowrite8(1, ipoctal-mem8_space + 1);
 
@@ -411,6 +404,14 @@ static int ipoctal_inst_slot(struct ipoctal *ipoctal, 
unsigned int bus_nr,
dev_set_drvdata(tty_dev, channel);
}
 
+   /*
+* IP-OCTAL has different addresses to copy its IRQ vector.
+* Depending of the carrier these addresses are accesible or not.
+* More info in the datasheet.
+*/
+   ipoctal-dev-bus-ops-request_irq(ipoctal-dev,
+  ipoctal_irq_handler, ipoctal);
+
return 0;
 }
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

PCIe bus enumeration

[Federico Vaga]

Hello,

(I haven't a deep knowledge of the PCIe specification, maybe I'm just 
missing something)

is there a way to force the PCI subsystem to assign a bus-number to 
every PCIe bridge, even if there is nothing connected?


My aim is to have a bus enumeration constant and independent from what 
I plugged on the system. So, I can associate a physical slot to linux 
device address bb:dd.f. Is it possible?

I can do the mapping with a simple shell script by discovering the 
new bus number, but I'm wondering if there is a way to have a 
constant bus enumeration.



My Humble Observation
-
It seems (to me) that for PCI the kernel assigns a bus-number to every 
PCI bridges and sub-bridges even if there is nothing connected:


e.g. from lspci -t

  [...]
  +-1e.0-[04-05]0c.0-[05]--

00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev 92)
04:0c.0 PCI bridge: Texas Instruments PCI2050 PCI-to-PCI Bridge (rev 
02)


The behavior on PCIe seems different. When there is nothing plugged on 
a bus, then the kernel doesn't assign any bus-number and it doesn't 
detect any PCI-Bridge at all. So, when I reboot the system with a new 
PCIe card the bus enumeration may change.


I tried to use the following pci kernel parameters:

assign-busses : because I want to force the kernel to re-enumerate the 
busses, hopefully _all_ buses even if they are empty.

pcie_scan_all : not clear the explanation, but it sounds like it tells 
to the kernel to inspect everything.

bfsort : because, maybe, for a bfsort it must assign a number to each 
bridge at the same level before inspect the next one.

noacpi : in order to scan independently from BIOS information


The result is always the same (empty buses are not enumerated). 



Thank you :)

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: PCIe bus enumeration

[Federico Vaga]

(Sorry for double emailing, a sw update changes my configuration to 
HTML email as default.So, the linux kernel mailing list complains that 
probably I'm spamming)

On Thursday 03 July 2014 13:43:14 Bjorn Helgaas wrote:
 On Thu, Jul 3, 2014 at 10:45 AM, Federico Vaga 
federico.v...@gmail.com wrote:
  Hello,
  
  (I haven't a deep knowledge of the PCIe specification, maybe I'm
  just missing something)
  
  is there a way to force the PCI subsystem to assign a bus-number
  to
  every PCIe bridge, even if there is nothing connected?
  
  
  My aim is to have a bus enumeration constant and independent from
  what I plugged on the system. So, I can associate a physical slot
  to linux device address bb:dd.f. Is it possible?

More information that I forgot to add. I'm working on kernel 3.2 and 
3.6.

 The /sys/bus/pci/slots/*/address files might help.  On my system, I
 have:
 
   $ grep . /sys/bus/pci/slots/*/address /dev/null
   /sys/bus/pci/slots/5/address::03:00

My slots directory is empty on 3.2, 3.6, 3.14. I have to compile the 
kernel with a 
particular configuration? Use a kernel parameter?

 lspci -v also shows:
 
   03:00.0 Unassigned class [ff00]: Realtek Semiconductor Co., Ltd.
 Device 5227 (rev 01)
 Physical Slot: 5

My lspci hasn't the Physical Slot field. However, where does it take 
this information? 
From the BIOS I suppose, a recent BIOS.

So if you look at your motherboard you can identify the which is the 
slot 5

 If you want to start with a physical slot number and figure out the
 bb.dd associated with it, the /sys/bus/pci/slots files are probably
 the most straightforward way.
 
  I can do the mapping with a simple shell script by discovering the
  new bus number, but I'm wondering if there is a way to have a
  constant bus enumeration.
  
  
  
  My Humble Observation
  -
  It seems (to me) that for PCI the kernel assigns a bus-number to
  every PCI bridges and sub-bridges even if there is nothing
  connected:
  
  
  e.g. from lspci -t
  
[...]
+-1e.0-[04-05]0c.0-[05]--
  
  00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev 92)
  04:0c.0 PCI bridge: Texas Instruments PCI2050 PCI-to-PCI Bridge
  (rev 02)
 
 Yes.  I think you're talking about the bridge secondary bus
 number. In this case the 04:0c.0 bridge has secondary bus 05, and
 there are no devices on bus 05.

yep

  The behavior on PCIe seems different. When there is nothing
  plugged on a bus, then the kernel doesn't assign any bus-number
  and it doesn't detect any PCI-Bridge at all. So, when I reboot
  the system with a new PCIe card the bus enumeration may change.
 
 I don't think the behavior should be different on PCIe, but maybe if
 you have an example, it will help me figure out why it is
 different.  My current machine has three Root Ports (which are
 treated as PCI-to-PCI bridges), and they all have secondary bus
 numbers assigned, even though only two have devices below them:
 
+-1c.0-[01]--
+-1c.3-[02]00.0
+-1c.5-[03]00.0

What I observed is that when several PCIe slot belong to a single PCI 
Bridge, and you 
plug a board in one on these, then it enumerates all secondary buses, 
also the 
empty ones (like your case, all your slot belong to device 1c).

But, if you un-plug the devices on secondary bus 02 and 03, you should 
not see the 
device 1c anymore. This is what is happening with my machine 
[industrial backplane 
with several PCI(e) slots and the motherboard plugged in a special 
slot.].

Even on sysfs the device doesn't appear.

 We have to assign a secondary bus number in order to enumerate below
 the bridge.  We can't even tell whether the bus is empty until we
 enumerate it.

Yep, I read the code and that's what I understood. 

 We should assign a secondary bus number, then
 enumerate the secondary bus (possibly finding nothing).  If we
 don't find anything, I think we currently leave the secondary bus
 number assigned even though the bus is empty.

I'll try to check :)


Thank you Bjorn

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: PCIe bus enumeration

[Federico Vaga]

On Tuesday 08 July 2014 14:27:00 Bjorn Helgaas wrote:
 On Tue, Jul 8, 2014 at 1:20 PM, Federico Vaga 
federico.v...@cern.ch wrote:
  On Tuesday 08 July 2014 12:23:39 Bjorn Helgaas wrote:
  On Tue, Jul 8, 2014 at 1:15 AM, Federico Vaga
  
  federico.v...@cern.ch wrote:
So, It looks like that some BIOS disable the bridge when
there
is
nothing behind it. Why? Power save? :/
   
   Could be power savings, or possibly to conserve bus numbers,
   which
   are a limited resource.
   
   what is the maximum number of buses?
  
  256.
  
  Well, it is not a small number. I will ask directly to the company
  who sell this crate and ask them what is going on in the BIOS
 
 Yeah, it's not usually a problem until you get to the really big
 machines.  The BIOS vendor could give you a much better reason; I'm
 only speculating.

Just to complete the discussion (I forgot to do it). The vendor point 
me to the correct BIOS configuration to keep all the PCIe port enable 
even if there is nothing in the slot. Now the bus number enumeration 
seems constant

Thank you

-- 
Federico Vaga
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] ipoctal: clear break interrupt as soon as it occurs

[Federico Vaga]

In some condition we receive the break interrupt but nothing is putted
in the Rx FIFO and the correspondend bit in the status register is not
set. Thus, no-one clear the interrupt and the handler will be called
forever.

This patch clear the break interrupt as soon as it occurs. Then, if the
break character '\0' is putted in the fifo we will manage it.

We can also unmask the Break interrupt but its bit in ISR is still set
on break. So I think is better to keep the registers clean.

Signed-off-by: Federico Vaga federico.v...@cern.ch
---
 drivers/ipack/devices/ipoctal.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/ipack/devices/ipoctal.c b/drivers/ipack/devices/ipoctal.c
index e41bef0..90b96a1b 100644
--- a/drivers/ipack/devices/ipoctal.c
+++ b/drivers/ipack/devices/ipoctal.c
@@ -151,7 +151,6 @@ static void ipoctal_irq_rx(struct ipoctal_channel *channel, 
u8 sr)
flag = TTY_FRAME;
}
if (sr  SR_RECEIVED_BREAK) {
-   iowrite8(CR_CMD_RESET_BREAK_CHANGE, 
channel-regs-w.cr);
channel-stats.rcv_break++;
flag = TTY_BREAK;
}
@@ -196,6 +195,9 @@ static void ipoctal_irq_channel(struct ipoctal_channel 
*channel)
isr = ioread8(channel-block_regs-r.isr);
sr = ioread8(channel-regs-r.sr);
 
+   if (isr  (IMR_DELTA_BREAK_A | IMR_DELTA_BREAK_B))
+   iowrite8(CR_CMD_RESET_BREAK_CHANGE, channel-regs-w.cr);
+
if ((sr  SR_TX_EMPTY)  (channel-nb_bytes == 0)) {
iowrite8(CR_DISABLE_TX, channel-regs-w.cr);
/* In case of RS-485, change from TX to RX when finishing TX.
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

IPACK - ipack and ipoctal improvements

[Federico Vaga]

I tested these patches only on kernel 3.2 and 3.6 because I cannot easly
test them on the next branch. I think that there are not important
modification in the kernel that can affect the behave of these patches.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/3] ipack: save carrier owner to allow device to get it

[Federico Vaga]

There was not any kind of protection against carrier driver removal.
In this way, device driver can 'get' the carrier driver when it is
using it.

Signed-off-by: Federico Vaga federico.v...@cern.ch
---
 drivers/ipack/carriers/tpci200.c |3 ++-
 drivers/ipack/ipack.c|4 +++-
 include/linux/ipack.h|   24 +++-
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/ipack/carriers/tpci200.c b/drivers/ipack/carriers/tpci200.c
index de5e321..9b23843 100644
--- a/drivers/ipack/carriers/tpci200.c
+++ b/drivers/ipack/carriers/tpci200.c
@@ -572,7 +572,8 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
/* Register the carrier in the industry pack bus driver */
tpci200-info-ipack_bus = ipack_bus_register(pdev-dev,
  TPCI200_NB_SLOT,
- tpci200_bus_ops);
+ tpci200_bus_ops,
+ THIS_MODULE);
if (!tpci200-info-ipack_bus) {
dev_err(pdev-dev,
error registering the carrier on ipack driver\n);
diff --git a/drivers/ipack/ipack.c b/drivers/ipack/ipack.c
index d0016ba..c0e7b62 100644
--- a/drivers/ipack/ipack.c
+++ b/drivers/ipack/ipack.c
@@ -206,7 +206,8 @@ static struct bus_type ipack_bus_type = {
 };
 
 struct ipack_bus_device *ipack_bus_register(struct device *parent, int slots,
-   const struct ipack_bus_ops *ops)
+   const struct ipack_bus_ops *ops,
+   struct module *owner)
 {
int bus_nr;
struct ipack_bus_device *bus;
@@ -225,6 +226,7 @@ struct ipack_bus_device *ipack_bus_register(struct device 
*parent, int slots,
bus-parent = parent;
bus-slots = slots;
bus-ops = ops;
+   bus-owner = owner;
return bus;
 }
 EXPORT_SYMBOL_GPL(ipack_bus_register);
diff --git a/include/linux/ipack.h b/include/linux/ipack.h
index 1888e06..8bddc3f 100644
--- a/include/linux/ipack.h
+++ b/include/linux/ipack.h
@@ -172,6 +172,7 @@ struct ipack_bus_ops {
  * @ops: bus operations for the mezzanine drivers
  */
 struct ipack_bus_device {
+   struct module *owner;
struct device *parent;
int slots;
int bus_nr;
@@ -189,7 +190,8 @@ struct ipack_bus_device {
  * available bus device in ipack.
  */
 struct ipack_bus_device *ipack_bus_register(struct device *parent, int slots,
-   const struct ipack_bus_ops *ops);
+   const struct ipack_bus_ops *ops,
+   struct module *owner);
 
 /**
  * ipack_bus_unregister -- unregister an ipack bus
@@ -265,3 +267,23 @@ void ipack_put_device(struct ipack_device *dev);
 .format = (_format), \
 .vendor = (vend), \
 .device = (dev)
+
+/**
+ * ipack_get_carrier - it increase the carrier ref. counter of
+ * the carrier module
+ * @dev: mezzanine device which wants to get the carrier
+ */
+static inline int ipack_get_carrier(struct ipack_device *dev)
+{
+   return try_module_get(dev-bus-owner);
+}
+
+/**
+ * ipack_get_carrier - it decrease the carrier ref. counter of
+ * the carrier module
+ * @dev: mezzanine device which wants to get the carrier
+ */
+static inline void ipack_put_carrier(struct ipack_device *dev)
+{
+   module_put(dev-bus-owner);
+}
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/3] ipoctal: reset function istead of duplicate code

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@cern.ch
---
 drivers/ipack/devices/ipoctal.c |   35 ++-
 1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/drivers/ipack/devices/ipoctal.c b/drivers/ipack/devices/ipoctal.c
index 90b96a1b..e531379 100644
--- a/drivers/ipack/devices/ipoctal.c
+++ b/drivers/ipack/devices/ipoctal.c
@@ -55,6 +55,16 @@ struct ipoctal {
u8 __iomem  *int_space;
 };
 
+static void ipoctal_reset_channel(struct ipoctal_channel *channel)
+{
+   iowrite8(CR_DISABLE_RX | CR_DISABLE_TX, channel-regs-w.cr);
+   channel-rx_enable = 0;
+   iowrite8(CR_CMD_RESET_RX, channel-regs-w.cr);
+   iowrite8(CR_CMD_RESET_TX, channel-regs-w.cr);
+   iowrite8(CR_CMD_RESET_ERR_STATUS, channel-regs-w.cr);
+   iowrite8(CR_CMD_RESET_MR, channel-regs-w.cr);
+}
+
 static int ipoctal_port_activate(struct tty_port *port, struct tty_struct *tty)
 {
struct ipoctal_channel *channel;
@@ -306,10 +316,7 @@ static int ipoctal_inst_slot(struct ipoctal *ipoctal, 
unsigned int bus_nr,
channel-isr_rx_rdy_mask = ISR_RxRDY_FFULL_A;
}
 
-   iowrite8(CR_DISABLE_RX | CR_DISABLE_TX, channel-regs-w.cr);
-   channel-rx_enable = 0;
-   iowrite8(CR_CMD_RESET_RX, channel-regs-w.cr);
-   iowrite8(CR_CMD_RESET_TX, channel-regs-w.cr);
+   ipoctal_reset_channel(channel);
iowrite8(MR1_CHRL_8_BITS | MR1_ERROR_CHAR | MR1_RxINT_RxRDY,
 channel-regs-w.mr); /* mr1 */
iowrite8(0, channel-regs-w.mr); /* mr2 */
@@ -469,11 +476,7 @@ static void ipoctal_set_termios(struct tty_struct *tty,
cflag = tty-termios.c_cflag;
 
/* Disable and reset everything before change the setup */
-   iowrite8(CR_DISABLE_RX | CR_DISABLE_TX, channel-regs-w.cr);
-   iowrite8(CR_CMD_RESET_RX, channel-regs-w.cr);
-   iowrite8(CR_CMD_RESET_TX, channel-regs-w.cr);
-   iowrite8(CR_CMD_RESET_ERR_STATUS, channel-regs-w.cr);
-   iowrite8(CR_CMD_RESET_MR, channel-regs-w.cr);
+   ipoctal_reset_channel(channel);
 
/* Set Bits per chars */
switch (cflag  CSIZE) {
@@ -611,12 +614,7 @@ static void ipoctal_hangup(struct tty_struct *tty)
 
tty_port_hangup(channel-tty_port);
 
-   iowrite8(CR_DISABLE_RX | CR_DISABLE_TX, channel-regs-w.cr);
-   channel-rx_enable = 0;
-   iowrite8(CR_CMD_RESET_RX, channel-regs-w.cr);
-   iowrite8(CR_CMD_RESET_TX, channel-regs-w.cr);
-   iowrite8(CR_CMD_RESET_ERR_STATUS, channel-regs-w.cr);
-   iowrite8(CR_CMD_RESET_MR, channel-regs-w.cr);
+   ipoctal_reset_channel(channel);
 
clear_bit(ASYNCB_INITIALIZED, channel-tty_port.flags);
wake_up_interruptible(channel-tty_port.open_wait);
@@ -629,12 +627,7 @@ static void ipoctal_shutdown(struct tty_struct *tty)
if (channel == NULL)
return;
 
-   iowrite8(CR_DISABLE_RX | CR_DISABLE_TX, channel-regs-w.cr);
-   channel-rx_enable = 0;
-   iowrite8(CR_CMD_RESET_RX, channel-regs-w.cr);
-   iowrite8(CR_CMD_RESET_TX, channel-regs-w.cr);
-   iowrite8(CR_CMD_RESET_ERR_STATUS, channel-regs-w.cr);
-   iowrite8(CR_CMD_RESET_MR, channel-regs-w.cr);
+   ipoctal_reset_channel(channel);
clear_bit(ASYNCB_INITIALIZED, channel-tty_port.flags);
 }
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 3/3] ipoctal: get carrier driver to avoid rmmod

[Federico Vaga]

Signed-off-by: Federico Vaga federico.v...@cern.ch
---
 drivers/ipack/devices/ipoctal.c |   30 +++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/ipack/devices/ipoctal.c b/drivers/ipack/devices/ipoctal.c
index e531379..035d544 100644
--- a/drivers/ipack/devices/ipoctal.c
+++ b/drivers/ipack/devices/ipoctal.c
@@ -55,6 +55,12 @@ struct ipoctal {
u8 __iomem  *int_space;
 };
 
+static inline struct ipoctal *chan_to_ipoctal(struct ipoctal_channel *chan,
+ unsigned int index)
+{
+   return container_of(chan, struct ipoctal, channel[index]);
+}
+
 static void ipoctal_reset_channel(struct ipoctal_channel *channel)
 {
iowrite8(CR_DISABLE_RX | CR_DISABLE_TX, channel-regs-w.cr);
@@ -82,12 +88,20 @@ static int ipoctal_port_activate(struct tty_port *port, 
struct tty_struct *tty)
 
 static int ipoctal_open(struct tty_struct *tty, struct file *file)
 {
-   struct ipoctal_channel *channel;
+   struct ipoctal_channel *channel = dev_get_drvdata(tty-dev);
+   struct ipoctal *ipoctal = chan_to_ipoctal(channel, tty-index);
+   int err;
 
-   channel = dev_get_drvdata(tty-dev);
tty-driver_data = channel;
 
-   return tty_port_open(channel-tty_port, tty, file);
+   if (!ipack_get_carrier(ipoctal-dev))
+   return -EBUSY;
+
+   err = tty_port_open(channel-tty_port, tty, file);
+   if (err)
+   ipack_put_carrier(ipoctal-dev);
+
+   return err;
 }
 
 static void ipoctal_reset_stats(struct ipoctal_stats *stats)
@@ -631,6 +645,15 @@ static void ipoctal_shutdown(struct tty_struct *tty)
clear_bit(ASYNCB_INITIALIZED, channel-tty_port.flags);
 }
 
+static void ipoctal_cleanup(struct tty_struct *tty)
+{
+   struct ipoctal_channel *channel = tty-driver_data;
+   struct ipoctal *ipoctal = chan_to_ipoctal(channel, tty-index);
+
+   /* release the carrier driver */
+   ipack_put_carrier(ipoctal-dev);
+}
+
 static const struct tty_operations ipoctal_fops = {
.ioctl =NULL,
.open = ipoctal_open,
@@ -642,6 +665,7 @@ static const struct tty_operations ipoctal_fops = {
.get_icount =   ipoctal_get_icount,
.hangup =   ipoctal_hangup,
.shutdown = ipoctal_shutdown,
+   .cleanup =  ipoctal_cleanup,
 };
 
 static int ipoctal_probe(struct ipack_device *dev)
-- 
1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

trace-cmd bug fixes

[Federico Vaga]

This set of patches contains some fixes found while studying
the trace-cmd code.

[PATCH 4/5] trace-cmd: fix argument parsing minor BUG

[Federico Vaga]

For some reason the list command does not use anymore `getopt()`
to parse the arguments, instead it uses a custum implementation.

During this change [5da0eff trace-cmd: Add regex for listing of events]
the variable `optind` has been forgotten.

To reproduce the problem try to use invalid arguments. The application
will not report the correct invalid argument

$ ./trace-cmd list -a
list: invalid option -- 'i'

Signed-off-by: Federico Vaga <federico.v...@vaga.pv.it>
---
 trace-cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/trace-cmd.c b/trace-cmd.c
index 1a62faa..a05df92 100644
--- a/trace-cmd.c
+++ b/trace-cmd.c
@@ -706,7 +706,7 @@ int main (int argc, char **argv)
break;
default:
fprintf(stderr, "list: invalid option 
-- '%c'\n",
-   argv[optind][1]);
+   argv[i][1]);
usage(argv);
}
}
-- 
2.9.3

[PATCH 5/5] trace-cmd: BUG fix malloc() pointer validation

[Federico Vaga]

To reproduce the bug

mkdir /sys/kernel/debug/tracing/instances/test
./trace-cmd show -B test -s -f
  Failed to allocate instance path snapshot

Signed-off-by: Federico Vaga <federico.v...@vaga.pv.it>
---
 trace-cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/trace-cmd.c b/trace-cmd.c
index a05df92..320229e 100644
--- a/trace-cmd.c
+++ b/trace-cmd.c
@@ -616,7 +616,7 @@ int main (int argc, char **argv)
if (buffer) {
path = malloc(strlen(buffer) + strlen("instances//") +
  strlen(file) + 1);
-   if (path)
+   if (!path)
die("Failed to allocate instance path %s", 
file);
sprintf(path, "instances/%s/%s", buffer, file);
file = path;
-- 
2.9.3

[PATCH 2/5] plugin:python: check asprintf() errors

[Federico Vaga]

The code was validating the string but the man page for asprintf(3)
says clearly:


If memory allocation wasn't possible, or some other error occurs,
these functions  will return -1, and the contents of strp are undefined.


So, we cannot really rely on the returned string pointer.

Signed-off-by: Federico Vaga <federico.v...@vaga.pv.it>
---
 plugin_python.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/plugin_python.c b/plugin_python.c
index d3da8b0..2997679 100644
--- a/plugin_python.c
+++ b/plugin_python.c
@@ -24,6 +24,7 @@ static int load_plugin(struct pevent *pevent, const char 
*path,
const char *name, void *data)
 {
PyObject *globals = data;
+   int err;
int len = strlen(path) + strlen(name) + 2;
int nlen = strlen(name) + 1;
char *full = malloc(len);
@@ -41,9 +42,9 @@ static int load_plugin(struct pevent *pevent, const char 
*path,
strcpy(n, name);
n[nlen - 4] = '\0';
 
-   asprintf(, pyload, full, n);
-   if (!load)
-   return -ENOMEM;
+   err = asprintf(, pyload, full, n);
+   if (err < 0)
+   return err;
 
res = PyRun_String(load, Py_file_input, globals, globals);
if (!res) {
-- 
2.9.3

[PATCH 3/5] trace-cmd:read: BUG initialize input_files item to zero

[Federico Vaga]

On allocation the data structure was not initialized. Later on some
attribute of this structure are used (e.g. tsoffset) assuming that the
default value is zero, but it is not always true.

Signed-off-by: Federico Vaga <federico.v...@vaga.pv.it>
---
 trace-read.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/trace-read.c b/trace-read.c
index 79519bd..9773a47 100644
--- a/trace-read.c
+++ b/trace-read.c
@@ -295,6 +295,7 @@ static void add_input(const char *file)
item = malloc(sizeof(*item));
if (!item)
die("Failed to allocate for %s", file);
+   memset(item, 0, sizeof(*item));
item->file = file;
list_add_tail(>list, _files);
last_input_file = item;
-- 
2.9.3

[PATCH 1/5] plugin:python: fix compiler warning

[Federico Vaga]

The function `load_plugin` is passed, as argument, to
`trace_util_load_plugins()` but the prototype was not exactly the same.

Signed-off-by: Federico Vaga <federico.v...@vaga.pv.it>
---
 plugin_python.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/plugin_python.c b/plugin_python.c
index da07d27..d3da8b0 100644
--- a/plugin_python.c
+++ b/plugin_python.c
@@ -20,7 +20,7 @@ static const char pyload[] =
 "finally:\n"
 "   file.close()\n";
 
-static void load_plugin(struct pevent *pevent, const char *path,
+static int load_plugin(struct pevent *pevent, const char *path,
const char *name, void *data)
 {
PyObject *globals = data;
@@ -32,7 +32,7 @@ static void load_plugin(struct pevent *pevent, const char 
*path,
PyObject *res;
 
if (!full || !n)
-   return;
+   return -ENOMEM;
 
strcpy(full, path);
strcat(full, "/");
@@ -43,7 +43,7 @@ static void load_plugin(struct pevent *pevent, const char 
*path,
 
asprintf(, pyload, full, n);
if (!load)
-   return;
+   return -ENOMEM;
 
res = PyRun_String(load, Py_file_input, globals, globals);
if (!res) {
@@ -53,6 +53,8 @@ static void load_plugin(struct pevent *pevent, const char 
*path,
Py_DECREF(res);
 
free(load);
+
+   return 0;
 }
 
 int PEVENT_PLUGIN_LOADER(struct pevent *pevent)
-- 
2.9.3

[PATCH V3 1/2] use direname instead of custom code

[Federico Vaga]

Prefer well known functions like `dirname(3)` instead of custom
implementation for the same functionality

Signed-off-by: Federico Vaga <federico.v...@vaga.pv.it>
---
 trace-record.c | 45 ++---
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/trace-record.c b/trace-record.c
index 020a373..79e4fe4 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -45,6 +45,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "trace-local.h"
 #include "trace-msg.h"
@@ -2215,12 +2216,24 @@ static void set_max_graph_depth(struct buffer_instance 
*instance, char *max_grap
die("could not write to max_graph_depth");
 }
 
+
+/**
+ * create_event - create and event descriptor
+ * @instance: instance to use
+ * @path: path to event attribute
+ * @old_event: event descriptor to use as base
+ *
+ * NOTE: the function purpose is to create a data structure to describe
+ * an ftrace event. During the process it becomes handy to change the
+ * string `path`. So, do not rely on the content of `path` after you
+ * invoke this function.
+ */
 static struct event_list *
 create_event(struct buffer_instance *instance, char *path, struct event_list 
*old_event)
 {
struct event_list *event;
struct stat st;
-   char *p;
+   char *p, *path_dirname;
int ret;
 
event = malloc(sizeof(*event));
@@ -2234,14 +2247,14 @@ create_event(struct buffer_instance *instance, char 
*path, struct event_list *ol
if (!event->filter_file)
die("malloc filter file");
}
-   for (p = path + strlen(path) - 1; p > path; p--)
-   if (*p == '/')
-   break;
-   *p = '\0';
-   p = malloc(strlen(path) + strlen("/enable") + 1);
+
+   path_dirname = dirname(path);
+
+   p = malloc(strlen(path_dirname) + strlen("/enable") + 1);
if (!p)
die("Failed to allocate enable path for %s", path);
-   sprintf(p, "%s/enable", path);
+   sprintf(p, "%s/enable", path_dirname);
+
ret = stat(p, );
if (ret >= 0)
event->enable_file = p;
@@ -2249,10 +2262,11 @@ create_event(struct buffer_instance *instance, char 
*path, struct event_list *ol
free(p);
 
if (event->trigger) {
-   p = malloc(strlen(path) + strlen("/trigger") + 1);
+   p = malloc(strlen(path_dirname) + strlen("/trigger") + 1);
if (!p)
die("Failed to allocate trigger path for %s", path);
-   sprintf(p, "%s/trigger", path);
+   sprintf(p, "%s/trigger", path_dirname);
+
ret = stat(p, );
if (ret > 0)
die("trigger specified but not supported by this 
kernel");
@@ -2266,8 +2280,7 @@ static void make_sched_event(struct buffer_instance 
*instance,
 struct event_list **event, struct event_list 
*sched,
 const char *sched_path)
 {
-   char *path;
-   char *p;
+   char *path, *path_dirname, *sched_filter_file_tmp;
 
/* Do nothing if the event already exists */
if (*event)
@@ -2277,11 +2290,13 @@ static void make_sched_event(struct buffer_instance 
*instance,
if (!path)
die("Failed to allocate path for %s", sched_path);
 
-   sprintf(path, "%s", sched->filter_file);
+   /* we do not want to corrupt sched->filter_file when using dirname() */
+   sched_filter_file_tmp = strdup(sched->filter_file);
+   if (!sched_filter_file_tmp)
+   die("Failed to allocate path for %s", sched_path);
+   path_dirname = dirname(sched_filter_file_tmp);
 
-   /* Remove the /filter from filter file */
-   p = path + strlen(path) - strlen("filter");
-   sprintf(p, "%s/filter", sched_path);
+   sprintf(path, "%s/%s/filter", path_dirname, sched_path);
 
*event = create_event(instance, path, sched);
free(path);
-- 
2.13.3