[PATCH v6 1/2] sta2x11_vip: convert to videobuf2, control framework, file handler
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
Acked-by: Giancarlo Asnaghi
---
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
* Copyright (C) 2010 WindRiver Systems, Inc.
+ * authors: Andreas Kies
+ * Vlad Lungu
*
* 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
- * Vlad Lungu
- *
*/
#include
#include
#include
#include
-#include
-
#include
-
#include
-
#include
#include
-#include
#include
#include
#include
#include
#include
#include
+#include
#include
-#include
+#include
+#include
+#include
#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 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 sta2x11_vip {
struct v4l2_device v4l2_dev;
struct video_device *video_dev;
@@ -129,21 +132,27 @@ struct sta2x11_vip {
Re: [PATCH v6 1/2] sta2x11_vip: convert to videobuf2, control framework, file handler
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 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 1/4] v4l: vb2: add prepare/finish callbacks to allocators
This patch adds support for prepare/finish callbacks in VB2 allocators.
These callback are used for buffer flushing.
Signed-off-by: Marek Szyprowski
Acked-by: Laurent Pinchart
Acked-by: Federico Vaga
---
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
Signed-off-by: Federico Vaga
---
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
+ * *
+ * 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
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+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 *buf = buf_priv;
+
[PATCH 2/4] adv7180: remove {query/g_/s_}ctrl
Signed-off-by: Federico Vaga
---
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
Signed-off-by: Federico Vaga
Acked-by: Giancarlo Asnaghi
---
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
- * Vlad Lungu
- *
*/
#include
#include
#include
#include
-#include
-
#include
-
#include
-
#include
#include
-#include
#include
#include
#include
#include
#include
#include
+#include
#include
-#include
+#include
+#include
+#include
#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, f
Re: [PATCH 3/4] videobuf2-dma-streaming: new videobuf2 memory allocator
> 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
> On Thursday, September 13, 2012 3:53 PM Federico Vaga wrote:
> > Signed-off-by: Federico Vaga
>
> 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
> 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
In data giovedì 13 settembre 2012 11:45:31, Jonathan Corbet ha scritto: > On Thu, 13 Sep 2012 17:46:32 +0200 > > Federico Vaga 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/
Update STA2X11 to videobuf2 and control framework
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
Signed-off-by: Federico Vaga
---
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
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
Signed-off-by: Federico Vaga
---
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
Signed-off-by: Federico Vaga
---
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
+ * *
+ * 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
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+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 (!
[PATCH 3/3] [media] sta2x11_vip: convert to videobuf2 and control framework
Signed-off-by: Federico Vaga
Acked-by: Giancarlo Asnaghi
---
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
- * Vlad Lungu
- *
*/
#include
#include
#include
#include
-#include
-
#include
-
#include
-
#include
#include
-#include
#include
#include
#include
#include
#include
#include
+#include
#include
-#include
+#include
#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 sequence;
+ struct vip_buffer *active; /* current active buffer */
+ spinlock_t lock; /* Used in videobuf2 callback */
+
+ unsigned int closing; /* 1 if VIP is closing*/
+ /* Interrupt counters */
+ int tcount, bcount;
int overflow;
- void *mem_spare;
- dma_addr_t dma_spare;
- void *iomem;
+
+ v
Re: [PATCH 1/3] [media] adv7180: remove {query/g_/s_}ctrl
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
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 :
> 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/
[PATCH v2 3/4] sta2x11_vip: convert to videobuf2 and control framework
From: 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
Acked-by: Giancarlo Asnaghi
---
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
- * Vlad Lungu
- *
*/
#include
#include
#include
#include
-#include
-
#include
-
#include
-
#include
#include
-#include
#include
#include
#include
#include
#include
#include
+#include
#include
-#include
+#include
+#include
+#include
#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 v
[PATCH 1/4] v4l: vb2: add prepare/finish callbacks to allocators
This patch adds support for prepare/finish callbacks in VB2 allocators.
These callback are used for buffer flushing.
Signed-off-by: Marek Szyprowski
Acked-by: Laurent Pinchart
Acked-by: Federico Vaga
---
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
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
---
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
+ *
+ * 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
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+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 int vb2_dma_streaming_num_users(vo
[PATCH v2 3/4] sta2x11_vip: convert to videobuf2 and control framework
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
Acked-by: Giancarlo Asnaghi
---
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
- * Vlad Lungu
- *
*/
#include
#include
#include
#include
-#include
-
#include
-
#include
-
#include
#include
-#include
#include
#include
#include
#include
#include
#include
+#include
#include
-#include
+#include
+#include
+#include
#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;
- struc
[PATCH v2 4/4] adv7180: remove {query/g_/s_}ctrl
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
---
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
> > > > + * @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/
Re: [PATCH v2 3/4] sta2x11_vip: convert to videobuf2 and control framework
> > +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
This patch adds support for prepare/finish callbacks in VB2 allocators.
These callback are used for buffer flushing.
Signed-off-by: Marek Szyprowski
Acked-by: Laurent Pinchart
Acked-by: Federico Vaga
---
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
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
---
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
+ *
+ * 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
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+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;
+}
+
+static unsigned int vb2
[PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
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
Acked-by: Giancarlo Asnaghi
---
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
- * Vlad Lungu
- *
*/
#include
#include
#include
#include
-#include
-
#include
-
#include
-
#include
#include
-#include
#include
#include
#include
#include
#include
#include
+#include
#include
-#include
+#include
+#include
+#include
#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;
- 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, tcoun
[PATCH v3 4/4] adv7180: remove {query/g_/s_}ctrl
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
---
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
From: Marek Szyprowski
This patch adds support for prepare/finish callbacks in VB2 allocators.
These callback are used for buffer flushing.
Signed-off-by: Marek Szyprowski
Acked-by: Laurent Pinchart
Acked-by: Federico Vaga
---
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 v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
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
> > Acked-by: Giancarlo Asnaghi
> >
> > [..]
> >
> > /*
> >
> >* 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
> > - * Vlad Lungu
>
> 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
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
* Copyright (C) 2010 WindRiver Systems, Inc.
* authors: Andreas Kies
* Vlad Lungu
> 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
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
> > Ok, I understand. I will write something like this. > > > > * Copyright (C) 2012 ST Microelectronics > > * author: Federico Vaga > > * Copyright (C) 2010 WindRiver Systems, Inc. > > * authors: Andreas Kies > > * Vlad Lungu > > 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
> 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 v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
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
Acked-by: Giancarlo Asnaghi
---
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
* Copyright (C) 2010 WindRiver Systems, Inc.
+ * authors: Andreas Kies
+ * Vlad Lungu
*
* 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
- * Vlad Lungu
- *
*/
#include
#include
#include
#include
-#include
-
#include
-
#include
-
#include
#include
-#include
#include
#include
#include
#include
#include
#include
+#include
#include
-#include
+#include
+#include
+#include
#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 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 a
Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
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/
Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
> 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
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 RFC] spidev.c: add sysfs attributes for SPI configuration
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
> 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
> 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/
DWC2 and/or S3C-HSOTG for STA2X11 board
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
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/
Re: DWC2 and/or S3C-HSOTG for STA2X11 board
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] dwc2/pci.c: add STMICRO vendor and device ID for STA2X11 board
Signed-off-by: Federico Vaga
Acked-by: Giancarlo Asnaghi
---
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/
[RFC] assign some address families for local use
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 Vaga>From 8ce4f2576aa8e95ea22921c31bdffd049460951d Mon Sep 17 00:00:00 2001
From: Federico Vaga
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
---
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 cha
drivers/base/core.c: about device_find_child() function
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()
Signed-off-by: Federico Vaga --- 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
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);
}
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.
Re: [PATCH] base/core.c: improve comment of the function device_find_child()
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 > > --- > > > > 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
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
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()
Signed-off-by: Federico Vaga --- 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()
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 --- 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()
The vio_remove() function uses device_find_child() but it does not drop the reference of the retrieved child. Signed-off-by: Federico Vaga --- 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/
[PATCH] net/core/sock.c: add missing VSOCK string in af_family_*_key_strings
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
---
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/
Re: Update VIP to videobuf2 and control framework
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
> > 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
Signed-off-by: Federico Vaga
Acked-by: Giancarlo Asnaghi
---
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
- * Vlad Lungu
- *
*/
#include
#include
#include
#include
-#include
-
#include
-
#include
-
#include
#include
-#include
#include
#include
#include
#include
#include
#include
+#include
#include
-#include
+#include
+#include
+#include
#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;
+ struct vb2_queue vb_vidq; /* queue maintaned by videobuf2 */
+ struct list_head buffer_list; /* list of buffers */
+ unsigned int sequence;
+ struct vip_buffer *active; /* current active buffer */
+ spinlock_t lock; /* Used in videobuf2 callback */
+
+ /* Interrupt counters */
+ int tcount, bcount;
int overflow;
- void *mem_spa
Re: Update VIP to videobuf2 and control framework
> 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
> > + 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: [PATCH 2/3] [media] videobuf2-dma-streaming: new videobuf2 memory allocator
> 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
> 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/
Re: Update VIP to videobuf2 and control framework
In data mercoledì 1 agosto 2012 07:04:18, Jonathan Corbet ha scritto: > On Wed, 1 Aug 2012 08:41:56 +0200 > > Hans Verkuil 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/
[PATCH RFC] [media] adv7180.c: convert to v4l2 control framework
Signed-off-by: Federico Vaga
---
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
#include
#include
+#include
#include
#include
-#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(&state->mutex);
- if (ret)
- return ret;
-
- switch (ctrl->i
Re: [PATCH RFC] [media] adv7180.c: convert to v4l2 control framework
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
> > > @@ -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
Signed-off-by: Federico Vaga
---
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
#include
#include
+#include
#include
#include
-#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(&state->mutex);
- if (ret)
- return ret;
-
- switch (ctrl->i
[PATCH v6 1/2] sta2x11_vip: convert to videobuf2, control framework, file handler
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
Acked-by: Giancarlo Asnaghi
---
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
* Copyright (C) 2010 WindRiver Systems, Inc.
+ * authors: Andreas Kies
+ * Vlad Lungu
*
* 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
- * Vlad Lungu
- *
*/
#include
#include
#include
#include
-#include
-
#include
-
#include
-
#include
#include
-#include
#include
#include
#include
#include
#include
#include
+#include
#include
-#include
+#include
+#include
+#include
#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 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 sta2x11_vip {
struct v4l2_device v4l2_dev;
struct video_device *video_dev;
@@ -129,21 +132,27 @@ struct sta2x11_vip {
[PATCH v6 2/2] adv7180: remove {query/g_/s_}ctrl
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
---
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 RFC] spidev.c: add sysfs attributes for SPI configuration
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
---
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
#include
#include
+#include
#include
#include
@@ -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_
Re: [GIT PULL FOR v3.5] Move sta2x11_vip to staging
> 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
> 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/
Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
> 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
This is useful when you need to specify specific GFP flags during memory
allocation (e.g. GFP_DMA).
Signed-off-by: Federico Vaga
---
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
#include
-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
#include
+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
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
Acked-by: Giancarlo Asnaghi
---
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
* Copyright (C) 2010 WindRiver Systems, Inc.
+ * authors: Andreas Kies
+ * Vlad Lungu
*
* 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
- * Vlad Lungu
- *
*/
#include
#include
#include
#include
-#include
-
#include
-
#include
-
#include
#include
-#include
#include
#include
#include
#include
#include
#include
+#include
#include
-#include
+#include
+#include
+#include
#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 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 a
[PATCH V4 3/3] adv7180: remove {query/g_/s_}ctrl
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
---
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
> > @@ -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
> 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/
fpga: fpga_mgr_get() buggy ?
Hello,
I believe that this patch
fpga: manager: change api, don't use drvdata
7085e2a94f7df5f419e3cfb2fe809ce6564e9629
is incomplete and buggy.
I completely agree that drvdata should not be used by the FPGA manager
or any other subsystem like that.
What is buggy is the function fpga_mgr_get().
That patch has been done to allow multiple FPGA manager instances to
be linked to the same device (PCI it says). But function
fpga_mgr_get() will return only the first found: what about the
others?
Then, all load kernel-doc comments says:
"This code assumes the caller got the mgr pointer from
of_fpga_mgr_get() or fpga_mgr_get()"
but that function does not allow me to get, for instance, the second
FPGA manager on my card.
Since, thanks to this patch I'm actually the creator of the
fpga_manager structure, I do not need to use fpga_mgr_get() to
retrieve that data structure.
Despite this, I believe we still need to increment the module
reference counter (which is done by fpga_mgr_get()).
We can fix this function by just replacing the argument from 'device'
to 'fpga_manager' (the one returned by create() ). Alternatively, we
can add an 'owner' field in "struct fpga_manager_ops" and 'get' it
when we use it. Or again, just an 'owner' argument in the create()
function. I'm proposing these alternatives because I'm not sure that
this is correct:
if (!try_module_get(dev->parent->driver->owner))
What if the device does not have a driver? Do we consider the
following a valid use case?
probe(struct device *dev) {
struct device *mydev;
mydev->parent = dev;
device_register(mydev);
fpga_mrg_create(mydev, );
}
thanks :)
Re: fpga: fpga_mgr_get() buggy ?
Hi Alan,
inline comments
On Friday, 22 June 2018 04:07:41 CEST Alan Tull wrote:
> On Thu, Jun 21, 2018 at 8:13 AM, Federico Vaga
> wrote:
>
> Hi Federico,
>
> Thanks for the analysis. I'll probably not be able to look into
> this very much until next week. A few notes below.
>
> > Hello,
> >
> > I believe that this patch
> >
> > fpga: manager: change api, don't use drvdata
> > 7085e2a94f7df5f419e3cfb2fe809ce6564e9629
> >
> > is incomplete and buggy.
> >
> > I completely agree that drvdata should not be used by the FPGA
> > manager or any other subsystem like that.
> >
> > What is buggy is the function fpga_mgr_get().
> > That patch has been done to allow multiple FPGA manager instances
> > to be linked to the same device (PCI it says). But function
> > fpga_mgr_get() will return only the first found: what about the
> > others?
>
> I was thinking it was going to be one manager per device which makes
> sense if the device corresponds to a single FPGA. But I could see
> that there could be valid use cases that had more than one FPGA
> such as on a PCI card.
Here a practical example where we have 2 FPGAs on the same card
https://www.ohwr.org/projects/svec/wiki/wiki
> > Then, all load kernel-doc comments says:
> >
> > "This code assumes the caller got the mgr pointer from
> > of_fpga_mgr_get() or fpga_mgr_get()"
> >
> > but that function does not allow me to get, for instance, the
> > second FPGA manager on my card.
> >
> > Since, thanks to this patch I'm actually the creator of the
> > fpga_manager structure, I do not need to use fpga_mgr_get() to
> > retrieve that data structure.
> > Despite this, I believe we still need to increment the module
> > reference counter (which is done by fpga_mgr_get()).
> >
> > We can fix this function by just replacing the argument from
> > 'device' to 'fpga_manager' (the one returned by create() ).
>
> At first thought, that's what I'd want.
>
> > Alternatively, we
> > can add an 'owner' field in "struct fpga_manager_ops" and 'get' it
> > when we use it. Or again, just an 'owner' argument in the create()
> > function.
>
> It seems like we shouldn't have to do that.
Why?
> > I'm proposing these alternatives because I'm not sure that
> >
> > this is correct:
> > if (!try_module_get(dev->parent->driver->owner))
> >
> > What if the device does not have a driver? Do we consider the
> > following a valid use case?
> >
> >
> > probe(struct device *dev) {
> >
> > struct device *mydev;
> >
> > mydev->parent = dev;
> > device_register(mydev);
> > fpga_mrg_create(mydev, );
> >
> > }
>
> When would you want to do that?
Not sure when, I'm in the middle of some other development and I
stumbled into this issue. But of course I can do it ... at some point
:)
> Alan
>
> > thanks :)
> >
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe
> > linux-fpga" in the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] i2c:ocores: do not handle IRQ if IF is not set
If the Interrupt Flag (IF) is not set, we should not handle the IRQ:
- the line can be shared with other devices
- it can be a spurious interrupt
To avoid reading twice the status register, the ocores_process() function
expects it to be read by the caller.
Signed-off-by: Federico Vaga
---
drivers/i2c/busses/i2c-ocores.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 98c0ef74882b..274d6eb22a2c 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -139,10 +139,9 @@ static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
return i2c->getreg(i2c, reg);
}
-static void ocores_process(struct ocores_i2c *i2c)
+static void ocores_process(struct ocores_i2c *i2c, u8 stat)
{
struct i2c_msg *msg = i2c->msg;
- u8 stat = oc_getreg(i2c, OCI2C_STATUS);
if ((i2c->state == STATE_DONE) || (i2c->state == STATE_ERROR)) {
/* stop has been sent */
@@ -209,9 +208,13 @@ static void ocores_process(struct ocores_i2c *i2c)
static irqreturn_t ocores_isr(int irq, void *dev_id)
{
struct ocores_i2c *i2c = dev_id;
+ u8 stat = oc_getreg(i2c, OCI2C_STATUS);
unsigned long flags;
int ret;
+ if (!(stat & OCI2C_STAT_IF))
+ return IRQ_NONE;
+
/*
* We need to protect i2c against a timeout event (see ocores_xfer())
* If we cannot take this lock, it means that we are already in
@@ -222,7 +225,7 @@ static irqreturn_t ocores_isr(int irq, void *dev_id)
if (!ret)
return IRQ_HANDLED;
- ocores_process(i2c);
+ ocores_process(i2c, stat);
spin_unlock_irqrestore(&i2c->xfer_lock, flags);
--
2.15.0
[PATCH 3/3] i2c:ocores: add polling interface
This driver assumes that an interrupt line is always available for
the I2C master. This is not always the case and this patch adds support
for a polling version based on workqueue.
Signed-off-by: Federico Vaga
---
drivers/i2c/busses/i2c-ocores.c | 94 ++---
1 file changed, 79 insertions(+), 15 deletions(-)
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 274d6eb22a2c..0dad1a512ef5 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -13,6 +13,7 @@
*/
#include
+#include
#include
#include
#include
@@ -26,14 +27,19 @@
#include
#include
#include
+#include
+
+#define OCORES_FLAG_POLL BIT(0)
struct ocores_i2c {
void __iomem *base;
u32 reg_shift;
u32 reg_io_width;
+ unsigned long flags;
wait_queue_head_t wait;
struct i2c_adapter adap;
struct i2c_msg *msg;
+ struct work_struct xfer_work;
int pos;
int nmsgs;
int state; /* see STATE_ */
@@ -166,8 +172,9 @@ static void ocores_process(struct ocores_i2c *i2c, u8 stat)
oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
return;
}
- } else
+ } else {
msg->buf[i2c->pos++] = oc_getreg(i2c, OCI2C_DATA);
+ }
/* end of msg? */
if (i2c->pos == msg->len) {
@@ -232,6 +239,50 @@ static irqreturn_t ocores_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
+
+/**
+ * It waits until is possible to process some data
+ * @i2c: ocores I2C device instance
+ *
+ * This is used when the device is in polling mode (interrupts disabled).
+ * It sleeps for the time necessary to send 8bits (one transfer over
+ * the I2C bus), then it permanently ping the ip-core until is possible
+ * to process data. The idea is that we sleep for most of the time at the
+ * beginning because we are sure that the ip-core is not ready yet.
+ */
+static void ocores_poll_wait(struct ocores_i2c *i2c)
+{
+ int sleep_min = (8/i2c->bus_clock_khz) * 1000; /* us for 8bits */
+ u8 loop_on;
+
+ usleep_range(sleep_min, sleep_min + 10);
+ if (i2c->state == STATE_DONE || i2c->state == STATE_ERROR)
+ loop_on = OCI2C_STAT_BUSY;
+ else
+ loop_on = OCI2C_STAT_TIP;
+ while (oc_getreg(i2c, OCI2C_STATUS) & loop_on)
+ ;
+}
+
+
+/**
+ * It implements the polling logic
+ * @work: work instance descriptor
+ *
+ * Here we try to re-use as much as possible from the IRQ logic
+ */
+static void ocores_work(struct work_struct *work)
+{
+ struct ocores_i2c *i2c = container_of(work,
+ struct ocores_i2c, xfer_work);
+ irqreturn_t ret;
+
+ do {
+ ocores_poll_wait(i2c);
+ ret = ocores_isr(-1, i2c);
+ } while (ret != IRQ_NONE);
+}
+
static int ocores_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
{
struct ocores_i2c *i2c = i2c_get_adapdata(adap);
@@ -245,6 +296,9 @@ static int ocores_xfer(struct i2c_adapter *adap, struct
i2c_msg *msgs, int num)
oc_setreg(i2c, OCI2C_DATA, i2c_8bit_addr_from_msg(i2c->msg));
oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_START);
+ if (i2c->flags & OCORES_FLAG_POLL)
+ schedule_work(&i2c->xfer_work);
+
if (wait_event_timeout(i2c->wait, (i2c->state == STATE_ERROR) ||
(i2c->state == STATE_DONE), HZ)) {
return (i2c->state == STATE_DONE) ? num : -EIO;
@@ -264,7 +318,8 @@ static int ocores_init(struct device *dev, struct
ocores_i2c *i2c)
u8 ctrl = oc_getreg(i2c, OCI2C_CONTROL);
/* make sure the device is disabled */
- oc_setreg(i2c, OCI2C_CONTROL, ctrl & ~(OCI2C_CTRL_EN|OCI2C_CTRL_IEN));
+ ctrl &= ~(OCI2C_CTRL_EN|OCI2C_CTRL_IEN);
+ oc_setreg(i2c, OCI2C_CONTROL, ctrl);
prescale = (i2c->ip_clock_khz / (5 * i2c->bus_clock_khz)) - 1;
prescale = clamp(prescale, 0, 0x);
@@ -277,12 +332,16 @@ static int ocores_init(struct device *dev, struct
ocores_i2c *i2c)
return -EINVAL;
}
+
oc_setreg(i2c, OCI2C_PRELOW, prescale & 0xff);
oc_setreg(i2c, OCI2C_PREHIGH, prescale >> 8);
/* Init the device */
oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_IACK);
- oc_setreg(i2c, OCI2C_CONTROL, ctrl | OCI2C_CTRL_IEN | OCI2C_CTRL_EN);
+ ctrl |= OCI2C_CTRL_EN;
+ if (i2c->flags != OCORES_FLAG_POLL)
+ ctrl |= OCI2C_CTRL_IEN;
+ oc_setreg(i2c, OCI2C_CONTROL, ctrl);
return 0;
}
@@ -439,10 +498,6 @@ static int ocores_i2c_probe(struct platform_device *pdev)
int ret;
int i;
- irq = platform_get_irq(pdev, 0);
- if (irq < 0)
- return irq;
-
i2c = devm_kzalloc(&am
Re: i2c:ocores: fixes and polling mechanism
Hello, sorry to disturb you all but after one month and a half I never received any comment about this patch set and I fear it ended up in a forgotten corner. I would like to know if someone is considering it or not. Thanks :) On Monday, June 25, 2018 6:13:00 PM CEST Federico Vaga wrote: > The first two patches fix what I believe are bugs. > > The third patch add a polling mechanism for those systems where > interrupts are not available. > > All these patches have been tested on a system without interrupt, this > means that I used my third patch to validate also the other two. > I would be nice if someone can run verify this also on other system, > perhaps with interrupts. If you consider it a useful information, I'm not > using devicetree for this installation. -- Federico Vaga [BE-CO-HT]
Re: [PATCH 1/2] fpga: Document when fpga_blah_free functions should be used
Hi Alan,
have you considered the possibility of having something like devm_fpga_[mgr|
bridge|region]_[create|free]() ? Like this, it will be obvious that 'struct
fpga_mgr' will be released automatically without reading any comment (but the
comment is still good), and you use devm_*_free() only to handle error
conditions.
On Wednesday, July 25, 2018 8:15:13 PM CEST Alan Tull wrote:
> Clarify when fpga_(mgr|bridge|region)_free functions should be used.
> The class's dev_release will handle cleanup when the device is released
> so once the mgr/brige/region has been successfully registered, it
> would be a bug to call fpga_(mgr|bridge|region)_free.
>
> Signed-off-by: Alan Tull
> Suggested-by: Florian Fainelli
> Suggested-by: Federico Vaga
> ---
> drivers/fpga/fpga-bridge.c | 10 +-
> drivers/fpga/fpga-mgr.c| 10 +-
> drivers/fpga/fpga-region.c | 10 +-
> 3 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/fpga/fpga-bridge.c b/drivers/fpga/fpga-bridge.c
> index 24b8f98..528d2149 100644
> --- a/drivers/fpga/fpga-bridge.c
> +++ b/drivers/fpga/fpga-bridge.c
> @@ -379,7 +379,11 @@ EXPORT_SYMBOL_GPL(fpga_bridge_create);
>
> /**
> * fpga_bridge_free - free a fpga bridge and its id
> - * @bridge: FPGA bridge struct created by fpga_bridge_create
> + * @bridge: FPGA bridge struct created by fpga_bridge_create()
> + *
> + * Free a FPGA bridge. This function should only be called for
> + * freeing a bridge that has not been registered yet (such as in error
> + * paths in a probe function).
> */
> void fpga_bridge_free(struct fpga_bridge *bridge)
> {
> @@ -414,6 +418,10 @@ EXPORT_SYMBOL_GPL(fpga_bridge_register);
> /**
> * fpga_bridge_unregister - unregister and free a fpga bridge
> * @bridge: FPGA bridge struct created by fpga_bridge_create
> + *
> + * Unregister the bridge device. The class's dev_release will handle
> + * freeing the bridge struct when the device is released so don't
> + * call fpga_bridge_free() after calling fpga_bridge_unregister().
> */
> void fpga_bridge_unregister(struct fpga_bridge *bridge)
> {
> diff --git a/drivers/fpga/fpga-mgr.c b/drivers/fpga/fpga-mgr.c
> index a41b07e..9632cbd 100644
> --- a/drivers/fpga/fpga-mgr.c
> +++ b/drivers/fpga/fpga-mgr.c
> @@ -619,7 +619,11 @@ EXPORT_SYMBOL_GPL(fpga_mgr_create);
>
> /**
> * fpga_mgr_free - deallocate a FPGA manager
> - * @mgr: fpga manager struct created by fpga_mgr_create
> + * @mgr: fpga manager struct created by fpga_mgr_create()
> + *
> + * Free a FPGA manager struct. This function should only be called
> + * for freeing a manager that has not been registered yet (such as in
> + * error paths in a probe function).
> */
> void fpga_mgr_free(struct fpga_manager *mgr)
> {
> @@ -663,6 +667,10 @@ EXPORT_SYMBOL_GPL(fpga_mgr_register);
> /**
> * fpga_mgr_unregister - unregister and free a FPGA manager
> * @mgr: fpga manager struct
> + *
> + * Unregister the manager device. The class's dev_release will handle
> + * freeing the manager struct when the device is released so don't
> + * call fpga_mgr_free() after calling fpga_mgr_unregister().
> */
> void fpga_mgr_unregister(struct fpga_manager *mgr)
> {
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> index 0d65220..7335fa9 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -231,7 +231,11 @@ EXPORT_SYMBOL_GPL(fpga_region_create);
>
> /**
> * fpga_region_free - free a struct fpga_region
> - * @region: FPGA region created by fpga_region_create
> + * @region: FPGA region created by fpga_region_create()
> + *
> + * Free a FPGA region struct. This function should only be called for
> + * freeing a region that has not been registered yet (such as in error
> + * paths in a probe function).
> */
> void fpga_region_free(struct fpga_region *region)
> {
> @@ -255,6 +259,10 @@ EXPORT_SYMBOL_GPL(fpga_region_register);
> /**
> * fpga_region_unregister - unregister and free a FPGA region
> * @region: FPGA region
> + *
> + * Unregister the region device. The class's dev_release will handle
> + * freeing the region so don't call fpga_region_free() after calling
> + * fpga_region_unregister().
> */
> void fpga_region_unregister(struct fpga_region *region)
> {
Re: fpga: fpga_mgr_free usage
On Wednesday, July 25, 2018 6:33:44 PM CEST Alan Tull wrote: > On Wed, Jul 11, 2018 at 10:59 AM, Alan Tull wrote: > > On Wed, Jul 11, 2018 at 7:38 AM, Federico Vaga > > wrote: > > > > Hi Federico, > > > >> Hi Alan, > >> > >> I have another point that I would like to discuss. It is about the > >> usage of 'fpga_mgr_free()' which does not look like consistent. > >> > >> This function, according to the current implementation, can be used by > >> an FPGA manager user and it is used by the FPGA manager itself on > >> device release. This means that the user can only use this function if > >> fpga_mgr_register() fails (to clean up), otherwise the user must NOT > >> use this function, otherwise we most likely get an oops (NULL or > >> invalid pointer). The example here is correct, this is what we should > >> do: > >> > >> https://www.kernel.org/doc/html/latest/driver-api/fpga/fpga-mgr.html > >> > >> But I suggest to document it better or prevent this type of mistakes > >> from happening. Following a couple of proposals > >> > >> -- > >> 1. > >> Document it better. This is easy, in the fpga_mgr_free() kernel-doc > >> comment we explain that the use of this function must be limited to > >> clean up the memory on a registration failure. If an FPGA manager has > >> been successfully registered then it will be freed by the framework > >> itself. > > As I was researching this, I remembered why I implemented it this way. > See below for that explanation. > > It looks like I'm going to switch to option 1 here and add more > documentation for both fpga_mgr_free() and fpga_mgr_unregister(). > Note that fpga_mgr_unregister() already says that that it frees the > manager, and the usage example already does the right thing, but I'll > add more words to really beat the message in. > > >> But still, this does not prevent an oops from happening. > >> -- > >> 2. > >> Remove the fpga_mgr_free() from fpga_mgr_dev_release() and ask the > >> user to free the manager when necessary. > >> > >> This makes the usage consistent: the user creates and destroy its own > >> objects. This is also consistent with our other discussion where we > >> said, among the other things, that the module that uses the FPGA > >> manager can the owner of the fpga_manager data structure. > > > > You're not the first to complain about this. I think I'll err on the > > side of consistency and implement your option 2 here. > > > > Alan > > If you write a class or create a device, the kernel wants a release > function and will give a warning if you leave it out. The warning is > "Device 'fpga0' does not have a release() function, it is broken and > must be fixed." and comes from drivers/base/core.c. True, but that function can be empty (in other words, it does nothing) and option 2 can be implemented as well without warnings. I do not think is that bad, for example if I allocate everything with devm_* probably I will not have much to do in my release() function. Anyway, I do not have strong technical arguments in favor of option 1 or 2. > I will add some more documentation to make it clear that once a a > mgr/bridge/region has been registered, the cleanup will be handled > automatically when the device goes away. Until the > fpga_(mgr|bridge|region)_register succeeds, the caller still needs to > do cleanup. > > I did find one bug while looking at this. I'll post some patches. > > Full message was: > root@cyclone5:~# rmmod socfpga > [ 48.206235] fpga_manager fpga0: fpga_mgr_unregister Altera SOCFPGA > FPGA Manager > [ 48.213677] [ cut here ] > [ 48.218312] WARNING: CPU: 1 PID: 1369 at > /home/atull/repos/linux-socfpga/drivers/base/core.c:895 > device_release+0x9c/0xa0 > [ 48.229293] Device 'fpga0' does not have a release() function, it > is broken and must be fixed. > [ 48.237904] Modules linked in: socfpga(-) altera_hps2fpga fpga_mgr > fpga_bridge [last unloaded: fpga_region] > [ 48.247659] CPU: 1 PID: 1369 Comm: rmmod Not tainted > 4.18.0-rc5-next-20180717-00012-ge5f548e-dirty #3 > [ 48.256843] Hardware name: Altera SOCFPGA > [ 48.260858] [] (unwind_backtrace) from [] > (show_stack+0x20/0x24) > [ 48.268582] [] (show_stack) from [] > (dump_stack+0x8c/0xa0) > [ 48.275786] [] (dump_stack) from [] > (__warn+0x104/0x11c) [ 48.282810] [] (__warn) from > [] > (warn_slowpath_fmt+0x54/0x70) > [ 48.290269] [] (warn
Re: fpga: fpga_mgr_get() buggy ?
Hi Alan, Thanks for your time, comments below On Wednesday, July 18, 2018 9:47:24 PM CEST Alan Tull wrote: > On Thu, Jun 28, 2018 at 2:50 AM, Federico Vaga wrote: > > On Wednesday, 27 June 2018 23:23:07 CEST Alan Tull wrote: > >> On Wed, Jun 27, 2018 at 4:25 AM, Federico Vaga > > > > wrote: > >> > Hi Alan, > >> > > >> > On Tuesday, 26 June 2018 23:00:46 CEST Alan Tull wrote: > >> >> On Fri, Jun 22, 2018 at 2:53 AM, Federico Vaga > >> >> wrote: > >> >> > >> >> Hi Federico, > >> >> > >> >> >> > What is buggy is the function fpga_mgr_get(). > >> >> >> > That patch has been done to allow multiple FPGA manager > >> >> >> > instances > >> >> >> > to be linked to the same device (PCI it says). But function > >> >> >> > fpga_mgr_get() will return only the first found: what about > >> >> >> > the > >> >> >> > others? > > Looking at this further, no code change is needed in the FPGA API to > support multiple managers. A FPGA manager driver is a self-contained > platform driver. The PCI driver for a board that contains multiple > FPGAs should create a platform device for each manager and save each > of those devs in its pdata. > > >> >> >> > Then, all load kernel-doc comments says: > >> >> >> > > >> >> >> > "This code assumes the caller got the mgr pointer from > >> >> >> > of_fpga_mgr_get() or fpga_mgr_get()" > >> >> >> > > >> >> >> > but that function does not allow me to get, for instance, > >> >> >> > the > >> >> >> > second FPGA manager on my card. > > fpga_mgr_get() will do what you want if your PCI driver creates a > platform device per FPGA manager as mentioned above. > > >> >> >> > Since, thanks to this patch I'm actually the creator of the > >> >> >> > fpga_manager structure, I do not need to use fpga_mgr_get() > >> >> >> > to > >> >> >> > retrieve that data structure. > > The creator of the FPGA mgr structure is the low level FPGA manager > driver, not the PCIe driver. In my case it is. It's a bit like where SPI driver is the low level FPGA manager driver for the xilinx-spi.c. But if I understand what you mean, I should always have a platform_driver just for the FPGA manager even if it has a 1:1 relation with its carrier? In other words, I write two drivers: - one for the FPGA manager - one for the PCI device that then register the FPGA manager driver In my case the FPGA programmed is also the PCIe bridge (GN4124). Probably I can do it anyway, because the part dedicated to FPGA programming is quite independent from the rest (don't remember all details) > >> >> >> > Despite this, I believe we still need to increment the > >> >> >> > module > >> >> >> > reference counter (which is done by fpga_mgr_get()). > > This is only done in the probe function of a FPGA region driver. > > >> >> >> > We can fix this function by just replacing the argument from > >> >> >> > 'device' to 'fpga_manager' (the one returned by create() ). > >> >> >> > >> >> >> At first thought, that's what I'd want. > >> >> >> > >> >> >> > Alternatively, we > >> >> >> > can add an 'owner' field in "struct fpga_manager_ops" and > >> >> >> > 'get' > >> >> >> > it > >> >> >> > when we use it. Or again, just an 'owner' argument in the > >> >> >> > create() > >> >> >> > function. > >> >> >> > >> >> >> It seems like we shouldn't have to do that. > >> >> > > >> >> > Why? > >> >> > >> >> OK yes, I agree; the kernel has a lot of examples of doing this. > >> >> > >> >> I'll have to play with it, I'll probably add the owner arg to the > >> >> create function, add owner to struct fpga_manager, and save. > >> > > >> > I have two though about this. > >> > > >> > 1. file_operation like approach. The onwer is associated to th
[PATCH 1/2] doc:process: add links where missing
Some documents are refering to others without links. With this patch I add those missing links. This patch affects only documents under process/ and labels where necessary. Signed-off-by: Federico Vaga --- Documentation/admin-guide/devices.rst| 1 + Documentation/dev-tools/coccinelle.rst | 2 ++ Documentation/doc-guide/sphinx.rst | 2 ++ Documentation/driver-api/pm/devices.rst | 2 ++ Documentation/process/4.Coding.rst | 3 ++- Documentation/process/5.Posting.rst | 23 +++- Documentation/process/8.Conclusion.rst | 7 +++--- Documentation/process/changes.rst| 2 +- Documentation/process/coding-style.rst | 2 +- Documentation/process/howto.rst | 11 +- Documentation/process/management-style.rst | 5 +++-- Documentation/process/submitting-drivers.rst | 8 --- 12 files changed, 42 insertions(+), 26 deletions(-) diff --git a/Documentation/admin-guide/devices.rst b/Documentation/admin-guide/devices.rst index 7fadc05330dd..d41671aeaef0 100644 --- a/Documentation/admin-guide/devices.rst +++ b/Documentation/admin-guide/devices.rst @@ -1,3 +1,4 @@ +.. _admin_devices: Linux allocated devices (4.x+ version) == diff --git a/Documentation/dev-tools/coccinelle.rst b/Documentation/dev-tools/coccinelle.rst index aa14f05cabb1..00a3409b0c28 100644 --- a/Documentation/dev-tools/coccinelle.rst +++ b/Documentation/dev-tools/coccinelle.rst @@ -4,6 +4,8 @@ .. highlight:: none +.. _devtools_coccinelle: + Coccinelle == diff --git a/Documentation/doc-guide/sphinx.rst b/Documentation/doc-guide/sphinx.rst index f0796daa95b4..760ab722483b 100644 --- a/Documentation/doc-guide/sphinx.rst +++ b/Documentation/doc-guide/sphinx.rst @@ -1,3 +1,5 @@ +.. _sphinx: + Introduction diff --git a/Documentation/driver-api/pm/devices.rst b/Documentation/driver-api/pm/devices.rst index 1128705a5731..090c151aa86b 100644 --- a/Documentation/driver-api/pm/devices.rst +++ b/Documentation/driver-api/pm/devices.rst @@ -6,6 +6,8 @@ .. |struct wakeup_source| replace:: :c:type:`struct wakeup_source ` .. |struct device| replace:: :c:type:`struct device ` +.. _driverapi_pm_devices: + == Device Power Management Basics == diff --git a/Documentation/process/4.Coding.rst b/Documentation/process/4.Coding.rst index eb4b185d168c..cfe264889447 100644 --- a/Documentation/process/4.Coding.rst +++ b/Documentation/process/4.Coding.rst @@ -315,7 +315,8 @@ variety of potential coding problems; it can also propose fixes for those problems. Quite a few "semantic patches" for the kernel have been packaged under the scripts/coccinelle directory; running "make coccicheck" will run through those semantic patches and report on any problems found. See -Documentation/dev-tools/coccinelle.rst for more information. +:ref:`Documentation/dev-tools/coccinelle.rst ` +for more information. Other kinds of portability errors are best found by compiling your code for other architectures. If you do not happen to have an S/390 system or a diff --git a/Documentation/process/5.Posting.rst b/Documentation/process/5.Posting.rst index c418c5d6cae4..4213e580f273 100644 --- a/Documentation/process/5.Posting.rst +++ b/Documentation/process/5.Posting.rst @@ -9,9 +9,10 @@ kernel. Unsurprisingly, the kernel development community has evolved a set of conventions and procedures which are used in the posting of patches; following them will make life much easier for everybody involved. This document will attempt to cover these expectations in reasonable detail; -more information can also be found in the files process/submitting-patches.rst, -process/submitting-drivers.rst, and process/submit-checklist.rst in the kernel -documentation directory. +more information can also be found in the files +:ref:`Documentation/process/submitting-patches.rst `, +:ref:`Documentation/process/submitting-drivers.rst ` +and :ref:`Documentation/process/submit-checklist.rst `. When to post @@ -198,8 +199,10 @@ pass it to diff with the "-X" option. The tags mentioned above are used to describe how various developers have been associated with the development of this patch. They are described in -detail in the process/submitting-patches.rst document; what follows here is a -brief summary. Each of these lines has the format: +detail in +the :ref:`Documentation/process/submitting-patches.rst ` +document; what follows here is a brief summary. Each of these lines has +the format: :: @@ -210,8 +213,8 @@ The tags in common use are: - Signed-off-by: this is a developer's certification that he or she has the right to submit the patch for inclusion into the kernel. It is an agreement to the Developer's Certificate of Origin, the full text of - which can be found in Documentation/process/sub
fpga: fpga_mgr_free usage
Hi Alan, I have another point that I would like to discuss. It is about the usage of 'fpga_mgr_free()' which does not look like consistent. This function, according to the current implementation, can be used by an FPGA manager user and it is used by the FPGA manager itself on device release. This means that the user can only use this function if fpga_mgr_register() fails (to clean up), otherwise the user must NOT use this function, otherwise we most likely get an oops (NULL or invalid pointer). The example here is correct, this is what we should do: https://www.kernel.org/doc/html/latest/driver-api/fpga/fpga-mgr.html But I suggest to document it better or prevent this type of mistakes from happening. Following a couple of proposals -- 1. Document it better. This is easy, in the fpga_mgr_free() kernel-doc comment we explain that the use of this function must be limited to clean up the memory on a registration failure. If an FPGA manager has been successfully registered then it will be freed by the framework itself. But still, this does not prevent an oops from happening. -- 2. Remove the fpga_mgr_free() from fpga_mgr_dev_release() and ask the user to free the manager when necessary. This makes the usage consistent: the user creates and destroy its own objects. This is also consistent with our other discussion where we said, among the other things, that the module that uses the FPGA manager can the owner of the fpga_manager data structure. -- 3. Not sure how, but perhaps we can be able to understand if we can safely continue to run fpga_mgr_free() or we should stop. --
i2c:ocores: fixes and polling mechanism
The first two patches fix what I believe are bugs. The third patch add a polling mechanism for those systems where interrupts are not available. All these patches have been tested on a system without interrupt, this means that I used my third patch to validate also the other two. I would be nice if someone can run verify this also on other system, perhaps with interrupts. If you consider it a useful information, I'm not using devicetree for this installation.
[PATCH 1/3] i2c:ocores: stop transfer on timeout
Detecting a timeout is ok, but we also need to assert a STOP command on
the bus in order to prevent it from generating interrupts when there are
no on going transfers.
Example: very long transmission.
1. ocores_xfer: START a transfer
2. ocores_isr : handle byte by byte the transfer
3. ocores_xfer: goes in timeout [[bugfix here]]
4. ocores_xfer: return to I2C subsystem and to the I2C driver
5. I2C driver : it may clean up the i2c_msg memory
6. ocores_isr : receives another interrupt (pending bytes to be
transferred) but the i2c_msg memory is invalid now
So, since the transfer was too long, we have to detect the timeout and
STOP the transfer.
Another point is that we have a critical region here. When handling the
timeout condition we may have a running IRQ handler. For this reason I
introduce a spinlock. In the IRQ handler we can just use trylock because
if the lock is taken is because we are in timeout, so there is no need to
process the IRQ.
Signed-off-by: Federico Vaga
---
drivers/i2c/busses/i2c-ocores.c | 28 ++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 88444ef74943..98c0ef74882b 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
struct ocores_i2c {
void __iomem *base;
@@ -36,6 +37,7 @@ struct ocores_i2c {
int pos;
int nmsgs;
int state; /* see STATE_ */
+ spinlock_t xfer_lock;
struct clk *clk;
int ip_clock_khz;
int bus_clock_khz;
@@ -207,15 +209,30 @@ static void ocores_process(struct ocores_i2c *i2c)
static irqreturn_t ocores_isr(int irq, void *dev_id)
{
struct ocores_i2c *i2c = dev_id;
+ unsigned long flags;
+ int ret;
+
+ /*
+* We need to protect i2c against a timeout event (see ocores_xfer())
+* If we cannot take this lock, it means that we are already in
+* timeout, so it's pointless to handle this interrupt because we
+* are going to abort the current transfer.
+*/
+ ret = spin_trylock_irqsave(&i2c->xfer_lock, flags);
+ if (!ret)
+ return IRQ_HANDLED;
ocores_process(i2c);
+ spin_unlock_irqrestore(&i2c->xfer_lock, flags);
+
return IRQ_HANDLED;
}
static int ocores_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
{
struct ocores_i2c *i2c = i2c_get_adapdata(adap);
+ unsigned long flags;
i2c->msg = msgs;
i2c->pos = 0;
@@ -226,10 +243,15 @@ static int ocores_xfer(struct i2c_adapter *adap, struct
i2c_msg *msgs, int num)
oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_START);
if (wait_event_timeout(i2c->wait, (i2c->state == STATE_ERROR) ||
- (i2c->state == STATE_DONE), HZ))
+ (i2c->state == STATE_DONE), HZ)) {
return (i2c->state == STATE_DONE) ? num : -EIO;
- else
+ } else {
+ spin_lock_irqsave(&i2c->xfer_lock, flags);
+ i2c->state = STATE_ERROR;
+ oc_setreg(i2c, OCI2C_CMD, OCI2C_CMD_STOP);
+ spin_unlock_irqrestore(&i2c->xfer_lock, flags);
return -ETIMEDOUT;
+ }
}
static int ocores_init(struct device *dev, struct ocores_i2c *i2c)
@@ -422,6 +444,8 @@ static int ocores_i2c_probe(struct platform_device *pdev)
if (!i2c)
return -ENOMEM;
+ spin_lock_init(&i2c->xfer_lock);
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
i2c->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(i2c->base))
--
2.15.0
Re: fpga: fpga_mgr_get() buggy ?
Hi Alan,
On Tuesday, 26 June 2018 23:00:46 CEST Alan Tull wrote:
> On Fri, Jun 22, 2018 at 2:53 AM, Federico Vaga
> wrote:
>
> Hi Federico,
>
> >> > What is buggy is the function fpga_mgr_get().
> >> > That patch has been done to allow multiple FPGA manager
> >> > instances
> >> > to be linked to the same device (PCI it says). But function
> >> > fpga_mgr_get() will return only the first found: what about the
> >> > others?
>
> I've had more time with this, I agree with you. I didn't intend to
> limit us to one manager per parent device.
>
> >> > Then, all load kernel-doc comments says:
> >> >
> >> > "This code assumes the caller got the mgr pointer from
> >> > of_fpga_mgr_get() or fpga_mgr_get()"
> >> >
> >> > but that function does not allow me to get, for instance, the
> >> > second FPGA manager on my card.
> >> >
> >> > Since, thanks to this patch I'm actually the creator of the
> >> > fpga_manager structure, I do not need to use fpga_mgr_get() to
> >> > retrieve that data structure.
> >> > Despite this, I believe we still need to increment the module
> >> > reference counter (which is done by fpga_mgr_get()).
> >> >
> >> > We can fix this function by just replacing the argument from
> >> > 'device' to 'fpga_manager' (the one returned by create() ).
> >>
> >> At first thought, that's what I'd want.
> >>
> >> > Alternatively, we
> >> > can add an 'owner' field in "struct fpga_manager_ops" and 'get'
> >> > it
> >> > when we use it. Or again, just an 'owner' argument in the
> >> > create()
> >> > function.
> >>
> >> It seems like we shouldn't have to do that.
> >
> > Why?
>
> OK yes, I agree; the kernel has a lot of examples of doing this.
>
> I'll have to play with it, I'll probably add the owner arg to the
> create function, add owner to struct fpga_manager, and save.
I have two though about this.
1. file_operation like approach. The onwer is associated to the FPGA
manager operation. Whenever the FPGA manager wants to use an operation
it get/put the module owner of these operations (because this is what
we need to protect). With this the user is not directly involved, read
it as less code, less things to care about. And probably there is no
need for fpga_manager_get().
2. fpga_manager onwer, we can still play the game above but
conceptually, from the user point of view, I see it like the driver
that creates the fpga_manager instance becomes the owner of it. I
think that this is not true, the fpga_manager structure is completely
used by the FPGA manager module and the driver use it as a token to
access the FPGA manager API. I hope my point is clear :)
I'm more for the solution 1.
> >> > I'm proposing these alternatives because I'm not sure that
> >> >
> >> > this is correct:
> >> > if (!try_module_get(dev->parent->driver->owner))
> >> >
> >> > What if the device does not have a driver? Do we consider the
> >> > following a valid use case?
> >> >
> >> >
> >> > probe(struct device *dev) {
> >> >
> >> > struct device *mydev;
> >> >
> >> > mydev->parent = dev;
> >> > device_register(mydev);
> >> > fpga_mrg_create(mydev, );
> >> >
> >> > }
>
> Sure
>
> >> When would you want to do that?
> >
> > Not sure when, I'm in the middle of some other development and I
> > stumbled into this issue. But of course I can do it ... at some
> > point>
> > :)
>
> I was meaning to ask something else.
I see, you meant the example about the "virtual device" without
driver. I do not have a true example, but this is a possible case I
think we should support it or not (check this on register())
> I don't mind writing this and would be interested in your review/
> feedback. Thanks again for seeing this and for the thoughtful
> analysis.
I'm here for any feedback/review :)
Re: fpga: fpga_mgr_get() buggy ?
On Wednesday, 27 June 2018 23:23:07 CEST Alan Tull wrote: > On Wed, Jun 27, 2018 at 4:25 AM, Federico Vaga wrote: > > Hi Alan, > > > > On Tuesday, 26 June 2018 23:00:46 CEST Alan Tull wrote: > >> On Fri, Jun 22, 2018 at 2:53 AM, Federico Vaga > >> wrote: > >> > >> Hi Federico, > >> > >> >> > What is buggy is the function fpga_mgr_get(). > >> >> > That patch has been done to allow multiple FPGA manager > >> >> > instances > >> >> > to be linked to the same device (PCI it says). But function > >> >> > fpga_mgr_get() will return only the first found: what about > >> >> > the > >> >> > others? > >> > >> I've had more time with this, I agree with you. I didn't intend > >> to > >> limit us to one manager per parent device. > >> > >> >> > Then, all load kernel-doc comments says: > >> >> > > >> >> > "This code assumes the caller got the mgr pointer from > >> >> > of_fpga_mgr_get() or fpga_mgr_get()" > >> >> > > >> >> > but that function does not allow me to get, for instance, > >> >> > the > >> >> > second FPGA manager on my card. > >> >> > > >> >> > Since, thanks to this patch I'm actually the creator of the > >> >> > fpga_manager structure, I do not need to use fpga_mgr_get() > >> >> > to > >> >> > retrieve that data structure. > >> >> > Despite this, I believe we still need to increment the > >> >> > module > >> >> > reference counter (which is done by fpga_mgr_get()). > >> >> > > >> >> > We can fix this function by just replacing the argument from > >> >> > 'device' to 'fpga_manager' (the one returned by create() ). > >> >> > >> >> At first thought, that's what I'd want. > >> >> > >> >> > Alternatively, we > >> >> > can add an 'owner' field in "struct fpga_manager_ops" and > >> >> > 'get' > >> >> > it > >> >> > when we use it. Or again, just an 'owner' argument in the > >> >> > create() > >> >> > function. > >> >> > >> >> It seems like we shouldn't have to do that. > >> > > >> > Why? > >> > >> OK yes, I agree; the kernel has a lot of examples of doing this. > >> > >> I'll have to play with it, I'll probably add the owner arg to the > >> create function, add owner to struct fpga_manager, and save. > > > > I have two though about this. > > > > 1. file_operation like approach. The onwer is associated to the > > FPGA manager operation. Whenever the FPGA manager wants to use an > > operation it get/put the module owner of these operations > > (because this is what we need to protect). With this the user is > > not directly involved, read it as less code, less things to care > > about. And probably there is no need for fpga_manager_get(). > > How does try_module_get fit into this scheme? I think this proposal > #1 is missing the point of what the reference count increment is > meant to do. Or am I misunderstanding? How does that keep the > module from being unloaded 1/3 way through programming a FPGA? > IIUC you are saying that the reference count would be incremented > before doing the manager ops .write_init, then decremented again > afterwards, incremented before each call to .write, decremented > afterwards, then the same for .write_complete. I'm not saying to do module_get/put just around the mops->XXX(): it's too much. Just where you have this comment: "This code assumes the caller got the mgr pointer from of_fpga_mgr_get() or fpga_mgr_get()" Because, currently, it's here where we do module_get() Most mops are invoked within a set of static functions which are called only by few exported functions. I'm suggesting to do module_get/put in those exported function at the beginning (get) and and the end (put) because we know that within these functions, here and there, we will use mops which are owned by a different module. We do not want the module owner of the mops to disappear while someone is doing fpga_mgr_load(). For example, inside fpga_mgr_load() we use most of the mops, so we 'get' the module at the begi
[PATCH 1/5] doc: typos and minor fixes
Signed-off-by: Federico Vaga --- Documentation/doc-guide/kernel-doc.rst| 2 +- Documentation/doc-guide/parse-headers.rst | 4 ++-- Documentation/doc-guide/sphinx.rst| 4 ++-- Documentation/index.rst | 2 +- Documentation/sphinx/parse-headers.pl | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst index 0268335414ce..d322367f70d7 100644 --- a/Documentation/doc-guide/kernel-doc.rst +++ b/Documentation/doc-guide/kernel-doc.rst @@ -262,7 +262,7 @@ comment block. The kernel-doc data structure comments describe each member of the structure, in order, with the ``@member:`` descriptions. The ``@member:`` descriptions must -begin on the very next line following the opening brief function description +begin on the very next line following the opening brief structure description line, with no intervening blank comment lines. The ``@member:`` descriptions may span multiple lines. The continuation lines may contain indentation. diff --git a/Documentation/doc-guide/parse-headers.rst b/Documentation/doc-guide/parse-headers.rst index 96a0423d5dba..cbfbd1748f8f 100644 --- a/Documentation/doc-guide/parse-headers.rst +++ b/Documentation/doc-guide/parse-headers.rst @@ -32,7 +32,7 @@ SYNOPSIS \ **parse_headers.pl**\ [] [] -Where can be: --debug, --help or --man. +Where can be: --debug, --usage or --help. OPTIONS @@ -133,7 +133,7 @@ For both statements, \ **type**\ can be either one of the following: \ **symbol**\ - The ignore or replace statement will apply to the name of enum statements + The ignore or replace statement will apply to the name of enum values at C_FILE. For replace statements, \ **new_value**\ will automatically use :c:type: diff --git a/Documentation/doc-guide/sphinx.rst b/Documentation/doc-guide/sphinx.rst index a2417633fdd8..8d789a117a4f 100644 --- a/Documentation/doc-guide/sphinx.rst +++ b/Documentation/doc-guide/sphinx.rst @@ -28,7 +28,7 @@ The ReST markups currently used by the Documentation/ files are meant to be built with ``Sphinx`` version 1.3 or upper. If you're desiring to build PDF outputs, it is recommended to use version 1.4.6 or upper. -There's a script that checks for the Spinx requirements. Please see +There's a script that checks for the Sphinx requirements. Please see :ref:`sphinx-pre-install` for further details. Most distributions are shipped with Sphinx, but its toolchain is fragile, @@ -266,7 +266,7 @@ some additional features: * row-span: with the role ``rspan`` a cell can be extended through additional rows -* auto span rightmost cell of a table row over the missing cells on the right +* auto-span: rightmost cell of a table row over the missing cells on the right side of that table-row. With Option ``:fill-cells:`` this behavior can changed from *auto span* to *auto fill*, which automatically inserts (empty) cells instead of spanning the last cell. diff --git a/Documentation/index.rst b/Documentation/index.rst index cb7f1ba5b3b1..331da87c82c8 100644 --- a/Documentation/index.rst +++ b/Documentation/index.rst @@ -33,7 +33,7 @@ the kernel interface as seen by application developers. .. toctree:: :maxdepth: 2 - userspace-api/index + userspace-api/index Introduction to kernel development diff --git a/Documentation/sphinx/parse-headers.pl b/Documentation/sphinx/parse-headers.pl index a958d8b5e99d..27117194d41f 100755 --- a/Documentation/sphinx/parse-headers.pl +++ b/Documentation/sphinx/parse-headers.pl @@ -344,7 +344,7 @@ enums and defines and create cross-references to a Sphinx book. B [] [] -Where can be: --debug, --help or --man. +Where can be: --debug, --help or --usage. =head1 OPTIONS -- 2.14.3
[PATCH 2/5] doc: add chapter labels
The idea is to make it easier to create references (doc-guide does the same). Signed-off-by: Federico Vaga --- Documentation/index.rst| 2 ++ Documentation/kernel-hacking/index.rst | 2 ++ 2 files changed, 4 insertions(+) diff --git a/Documentation/index.rst b/Documentation/index.rst index 331da87c82c8..8f11fccb9744 100644 --- a/Documentation/index.rst +++ b/Documentation/index.rst @@ -3,6 +3,8 @@ You can adapt this file completely to your liking, but it should at least contain the root `toctree` directive. +.. _linux_doc: + The Linux Kernel documentation == diff --git a/Documentation/kernel-hacking/index.rst b/Documentation/kernel-hacking/index.rst index fcb0eda3cca3..f53027652290 100644 --- a/Documentation/kernel-hacking/index.rst +++ b/Documentation/kernel-hacking/index.rst @@ -1,3 +1,5 @@ +.. _kernel_hacking: + = Kernel Hacking Guides = -- 2.14.3
doc: Italian translation
Ciao Jonathan, here the doc-guide translated in Italian. This set of patches includes some minor changes to the main one. The idea of this first set of patches is also to adjust the structure and our expectations. We tried to translate everything in **Italian**; which means that we avoided imported English words wherever there is the possibility to use the Italian ones. Of course, this is not always possible, or worst doing the translation make it less clear, for example the word "file". Another point. I noticed that the disclaimer for other translations is in English, so I did the same. But actually shouldn't be in Italian, since that page will be read by Italian speakers?
[PATCH 3/5] doc: add Italian translation skeleton
Signed-off-by: Federico Vaga Signed-off-by: Alessia Mantegazza --- Documentation/index.rst| 8 ++ .../translations/it_IT/disclaimer-ita.rst | 11 +++ Documentation/translations/it_IT/index.rst | 101 + 3 files changed, 120 insertions(+) create mode 100644 Documentation/translations/it_IT/disclaimer-ita.rst create mode 100644 Documentation/translations/it_IT/index.rst diff --git a/Documentation/index.rst b/Documentation/index.rst index 8f11fccb9744..e6075bdacbbd 100644 --- a/Documentation/index.rst +++ b/Documentation/index.rst @@ -113,6 +113,14 @@ Japanese translations translations/ja_JP/index +Italian translations +- + +.. toctree:: + :maxdepth: 1 + + translations/it_IT/index + Indices and tables == diff --git a/Documentation/translations/it_IT/disclaimer-ita.rst b/Documentation/translations/it_IT/disclaimer-ita.rst new file mode 100644 index ..0c439117ed7f --- /dev/null +++ b/Documentation/translations/it_IT/disclaimer-ita.rst @@ -0,0 +1,11 @@ +:orphan: + +.. note:: + This document is maintained by Federico Vaga . + If you find any difference between this document and the original file or a + problem with the translation, please contact the maintainer of this file. + +.. warning:: + The purpose of this file is to be easier to read and understand for Italian + speakers and is not intended as a fork. So, if you have any comments or + updates for this file please try to update the original English file first. diff --git a/Documentation/translations/it_IT/index.rst b/Documentation/translations/it_IT/index.rst new file mode 100644 index ..e688c06fd9a4 --- /dev/null +++ b/Documentation/translations/it_IT/index.rst @@ -0,0 +1,101 @@ +.. _it_linux_doc: + +=== +Traduzione italiana +=== + +L'obiettivo di questa traduzione è di rendere più facile la lettura e +la comprensione per chi preferisce leggere in lingua italiana. +Tenete presente che la documentazione di riferimento rimane comunque +quella in lingua inglese: :ref:`linux_doc` + +Questa traduzione cerca di essere il più fedele possibile all'originale ma +è ovvio che alcune frasi vadano trasformate: non aspettatevi una traduzione +letterale. Quando possibile, si eviteranno gli inglesismi ed al loro posto +verranno utilizzate le corrispettive parole italiane. + +Se notate che la traduzione non è più aggiornata potete contattare +direttamente il manutentore della traduzione italiana. + +Se notate che la documentazione contiene errori o dimenticanze, allora +verificate la documentazione di riferimento in lingua inglese. Se il problema +è presente anche nella documentazione di riferimento, contattate il suo +manutentore. Se avete problemi a scrivere in inglese, potete comunque +riportare il problema al manutentore della traduzione italiana. + +Manutentore della traduzione italiana: Federico Vaga + +La documentazione del kernel Linux +== + +Questo è il livello principale della documentazione del kernel in +lingua italiana. La traduzione è incompleta, noterete degli avvisi +che vi segnaleranno la mancanza di una traduzione o di un gruppo di +traduzioni. + +Più in generale, la documentazione, come il kernel stesso, sono in +costante sviluppo; particolarmente vero in quanto stiamo lavorando +alla riorganizzazione della documentazione in modo più coerente. +I miglioramenti alla documentazione sono sempre i benvenuti; per cui, +se vuoi aiutare, iscriviti alla lista di discussione linux-doc presso +vger.kernel.org. + +Documentazione per gli utenti +- + +I seguenti manuali sono scritti per gli *utenti* del kernel - ovvero, +coloro che cercano di farlo funzionare in modo ottimale su un dato sistema + +.. warning:: + +TODO ancora da tradurre + +Documentazione per gli sviluppatori di applicazioni +--- + +Il manuale delle API verso lo spazio utente è una collezione di documenti +che descrivono le interfacce del kernel viste dagli sviluppatori +di applicazioni. + +.. warning:: + +TODO ancora da tradurre + + +Introduzione allo sviluppo del kernel +- + +Questi manuali contengono informazioni su come contribuire allo sviluppo +del kernel. +Attorno al kernel Linux gira una comunità molto grande con migliaia di +sviluppatori che contribuiscono ogni anno. Come in ogni grande comunità, +sapere come le cose vengono fatte renderà il processo di integrazione delle +vostre modifiche molto più semplice + +.. warning:: + +TODO ancora da tradurre + +Documentazione della API del kernel +--- + +Questi manuali forniscono dettagli su come funzionano i sottosistemi del +kernel dal punto di vista degli sviluppatori del kernel. Molte delle +informazioni contenute in questi manuali sono prese direttamente
[PATCH 4/5] doc:it_IT: add doc-guide translation
Signed-off-by: Federico Vaga
Signed-off-by: Alessia Mantegazza
---
.../translations/it_IT/doc-guide/hello.dot | 3 +
.../translations/it_IT/doc-guide/index.rst | 24 ++
.../translations/it_IT/doc-guide/kernel-doc.rst| 402 ++
.../translations/it_IT/doc-guide/parse-headers.rst | 196 +
.../translations/it_IT/doc-guide/sphinx.rst| 455 +
.../translations/it_IT/doc-guide/svg_image.svg | 1 +
Documentation/translations/it_IT/index.rst | 8 +
7 files changed, 1089 insertions(+)
create mode 100644 Documentation/translations/it_IT/doc-guide/hello.dot
create mode 100644 Documentation/translations/it_IT/doc-guide/index.rst
create mode 100644 Documentation/translations/it_IT/doc-guide/kernel-doc.rst
create mode 100644 Documentation/translations/it_IT/doc-guide/parse-headers.rst
create mode 100644 Documentation/translations/it_IT/doc-guide/sphinx.rst
create mode 12 Documentation/translations/it_IT/doc-guide/svg_image.svg
diff --git a/Documentation/translations/it_IT/doc-guide/hello.dot
b/Documentation/translations/it_IT/doc-guide/hello.dot
new file mode 100644
index ..eee1e5864b3e
--- /dev/null
+++ b/Documentation/translations/it_IT/doc-guide/hello.dot
@@ -0,0 +1,3 @@
+graph G {
+ Ciao -- Mondo
+}
diff --git a/Documentation/translations/it_IT/doc-guide/index.rst
b/Documentation/translations/it_IT/doc-guide/index.rst
new file mode 100644
index ..7a6562b547ee
--- /dev/null
+++ b/Documentation/translations/it_IT/doc-guide/index.rst
@@ -0,0 +1,24 @@
+.. include:: ../disclaimer-ita.rst
+
+.. note:: Per leggere la documentazione originale in inglese:
+ :ref:`Documentation/doc-guide/index.rst `
+
+.. _it_doc_guide:
+
+==
+Come scrivere la documentazione del kernel
+==
+
+.. toctree::
+ :maxdepth: 1
+
+ sphinx.rst
+ kernel-doc.rst
+ parse-headers.rst
+
+.. only:: subproject and html
+
+ Indices
+ ===
+
+ * :ref:`genindex`
diff --git a/Documentation/translations/it_IT/doc-guide/kernel-doc.rst
b/Documentation/translations/it_IT/doc-guide/kernel-doc.rst
new file mode 100644
index ..77f6ec7aa38d
--- /dev/null
+++ b/Documentation/translations/it_IT/doc-guide/kernel-doc.rst
@@ -0,0 +1,402 @@
+.. include:: ../disclaimer-ita.rst
+
+.. note:: Per leggere la documentazione originale in inglese:
+ :ref:`Documentation/doc-guide/index.rst `
+
+Includere i commenti di tipo kernel-doc
+===
+
+I sorgenti del kernel Linux possono contenere commenti strutturati per
+la documentazione, oppure commenti di tipo kernel-doc che descrivono
+le funzioni, i tipi e la struttura del codice. I commenti di documentazione
+possono essere aggiunti ad un qualsiasi documento reStructuredtext
+utilizzando l'apposita estensione Sphinx per le direttive kernel-doc.
+
+Le direttive kernel-doc sono nel formato::
+
+ .. kernel-doc:: source
+ :option:
+
+Il campo *source* è il percorso ad un file sorgente, relativo alla cartella
+principale dei sorgenti del kernel. La direttiva supporta le seguenti opzioni:
+
+
+export: *[source-pattern ...]*
+ Include la documentazione per tutte le funzioni presenti nel file sorgente
+ (*source*) che sono state esportate utilizzando ``EXPORT_SYMBOL`` o
+ ``EXPORT_SYMBOL_GPL`` in *source* o in qualsiasi altro *source-pattern*
+ specificato.
+
+ Il campo *source-patter* è utile quando i commenti kernel-doc sono stati
+ scritti nei file d'intestazione, mentre ``EXPORT_SYMBOL`` e
+ ``EXPORT_SYMBOL_GPL`` si trovano vicino alla definizione delle funzioni.
+
+ Esempi::
+
+.. kernel-doc:: lib/bitmap.c
+ :export:
+
+.. kernel-doc:: include/net/mac80211.h
+ :export: net/mac80211/*.c
+
+internal: *[source-pattern ...]*
+ Include la documentazione per tutte le funzioni ed i tipi presenti nel file
+ sorgente (*source*) che **non** sono stati esportati utilizzando
+ ``EXPORT_SYMBOL`` o ``EXPORT_SYMBOL_GPL`` né in *source* né in qualsiasi
+ altro *source-pattern* specificato.
+
+ Esempio::
+
+.. kernel-doc:: drivers/gpu/drm/i915/intel_audio.c
+ :internal:
+
+doc: *title*
+ Include la documentazione per il paragrafo ``DOC:`` identificato dal titolo
+ (*title*) all'interno del file sorgente (*source*). Gli spazi in *title* sono
+ permessi; non virgolettate *title*. Il campo *title* è utilizzato per
+ identificare un paragrafo e per questo non viene incluso nella documentazione
+ finale. Verificate d'avere l'intestazione appropriata nei documenti
+ reStructuredText.
+
+ Esempio::
+
+.. kernel-doc:: drivers/gpu/drm/i915/intel_audio.c
+ :doc: High Definition Audio over HDMI and Display Port
+
+functions: *function* *[...]*
+ Dal file sorgente (*source*) include la documentazione per le funzioni
elencate
+ (*function*).
+
+ Esempio::
+
+.. kernel-doc:: lib/bi
[PATCH] i2c: ocores: update HDL sources URL
The URL is broken. This patch fix it Signed-off-by: Federico Vaga --- drivers/i2c/busses/i2c-ocores.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c index 8c42ca7107b2..14c5f6bbfd95 100644 --- a/drivers/i2c/busses/i2c-ocores.c +++ b/drivers/i2c/busses/i2c-ocores.c @@ -1,6 +1,6 @@ /* * i2c-ocores.c: I2C bus driver for OpenCores I2C controller - * (http://www.opencores.org/projects.cgi/web/i2c/overview). + * (https://opencores.org/project/i2c?do=projects&download=i2c). * * Peter Korsgaard * -- 2.15.0
