RE: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
I was completely unaware of that feature. I hunch that many drivers are incapable of dealing with an unbind while they are still open. Hmm, maybe older drivers... Anythig hotpluggable (USB, PCI, etc) should be in a better shape because they expect to be yanked at any time. Whim and prayer ??? David ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
Hi Ira,
On Tue, Feb 08, 2011 at 03:37:46PM -0800, Ira W. Snyder wrote:
This driver allows userspace to access the data processing FPGAs on the
OVRO CARMA board. It has two modes of operation:
Thank you for making the changes, some more comments below.
+
+#define inode_to_dev(inode) container_of(inode-i_cdev, struct fpga_device,
cdev)
+
Does not seem to be used.
+/*
+ * Data Buffer Allocation Helpers
+ */
+
+static int data_map_buffer(struct device *dev, struct data_buf *buf)
+{
+ return videobuf_dma_map(dev, buf-vb);
+}
+
+static void data_unmap_buffer(struct device *dev, struct data_buf *buf)
+{
+ videobuf_dma_unmap(dev, buf-vb);
+}
Why can't we all videobuf_dma_map() and videobuf_dma_unmap() directly?
What these helpers supposed to solve?
+static int data_alloc_buffers(struct fpga_device *priv)
+{
+ struct data_buf *buf;
+ int i, ret;
+
+ for (i = 0; i MAX_DATA_BUFS; i++) {
+
+ /* allocate a buffer */
+ buf = data_alloc_buffer(priv-bufsize);
+ if (!buf)
+ continue;
break?
+
+ /* map it for DMA */
+ ret = data_map_buffer(priv-dev, buf);
+ if (ret) {
+ data_free_buffer(buf);
+ continue;
and here as well?
+ }
+
+ /* add it to the list of free buffers */
+ list_add_tail(buf-entry, priv-free);
+ priv-num_buffers++;
+ }
+
+ /* Make sure we allocated the minimum required number of buffers */
+ if (priv-num_buffers MIN_DATA_BUFS) {
+ dev_err(priv-dev, Unable to allocate enough data buffers\n);
+ data_free_buffers(priv);
+ return -ENOMEM;
+ }
+
+ /* Warn if we are running in a degraded state, but do not fail */
+ if (priv-num_buffers MAX_DATA_BUFS) {
+ dev_warn(priv-dev, Unable to allocate one second worth of
+ buffers, using %d buffers instead\n, i);
The latest style is not break strings even if they exceed 80 column
limit to make sure they are easily greppable.
+static void data_dma_cb(void *data)
+{
+ struct fpga_device *priv = data;
+ struct data_buf *buf;
+ unsigned long flags;
+
+ spin_lock_irqsave(priv-lock, flags);
+
+ /* clear the FPGA status and re-enable interrupts */
+ data_enable_interrupts(priv);
+
+ /* If the inflight list is empty, we've got a bug */
+ BUG_ON(list_empty(priv-inflight));
+
+ /* Grab the first buffer from the inflight list */
+ buf = list_first_entry(priv-inflight, struct data_buf, entry);
+ list_del_init(buf-entry);
+
+ /* Add it to the used list */
+ list_add_tail(buf-entry, priv-used);
or
list_move_tail(buf-entry, priv-used);
+
+static irqreturn_t data_irq(int irq, void *dev_id)
+{
+ struct fpga_device *priv = dev_id;
+ struct data_buf *buf;
+ u32 status;
+ int i;
+
+ /* detect spurious interrupts via FPGA status */
+ for (i = 0; i 4; i++) {
+ status = fpga_read_reg(priv, i, MMAP_REG_STATUS);
+ if (!(status (CORL_DONE | CORL_ERR))) {
+ dev_err(priv-dev, spurious irq detected (FPGA)\n);
+ return IRQ_NONE;
+ }
+ }
+
+ /* detect spurious interrupts via raw IRQ pin readback */
+ status = ioread32be(priv-regs + SYS_IRQ_INPUT_DATA);
+ if (status IRQ_CORL_DONE) {
+ dev_err(priv-dev, spurious irq detected (IRQ)\n);
+ return IRQ_NONE;
+ }
+
+ spin_lock(priv-lock);
+
+ /* hide the interrupt by switching the IRQ driver to GPIO */
+ data_disable_interrupts(priv);
+
+ /* Check that we actually have a free buffer */
+ if (list_empty(priv-free)) {
+ priv-num_dropped++;
+ data_enable_interrupts(priv);
+ goto out_unlock;
+ }
+
+ buf = list_first_entry(priv-free, struct data_buf, entry);
+ list_del_init(buf-entry);
+
+ /* Check the buffer size */
+ BUG_ON(buf-size != priv-bufsize);
+
+ /* Submit a DMA transfer to get the correlation data */
+ if (data_submit_dma(priv, buf)) {
+ dev_err(priv-dev, Unable to setup DMA transfer\n);
+ list_add_tail(buf-entry, priv-free);
+ data_enable_interrupts(priv);
+ goto out_unlock;
+ }
+
+ /* DMA setup succeeded, GO!!! */
+ list_add_tail(buf-entry, priv-inflight);
+ dma_async_memcpy_issue_pending(priv-chan);
+
+out_unlock:
+ spin_unlock(priv-lock);
+ return IRQ_HANDLED;
+}
Hmm, I wonder if we could simplify this a bit by using list_move()...
bool submitted = false;
...
if (list_empty(priv-free)) {
priv-num_dropped++;
goto out;
}
buf = list_first_entry(priv-free, struct data_buf,
RE: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
This driver allows userspace to access the data processing FPGAs on the OVRO CARMA board. It has two modes of operation: 1) random access This allows users to poke any DATA-FPGA registers by using mmap to map the address region directly into their memory map. I needed something similar, but used pread() and pwrite() to request the transfers. While this does require a system call per transfer, it allows the driver to use dma (if available) to speed up the request. In my case doing single cycle transfers would be too slow. David ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Wed, Feb 09, 2011 at 04:30:23PM -, David Laight wrote: This driver allows userspace to access the data processing FPGAs on the OVRO CARMA board. It has two modes of operation: 1) random access This allows users to poke any DATA-FPGA registers by using mmap to map the address region directly into their memory map. I needed something similar, but used pread() and pwrite() to request the transfers. While this does require a system call per transfer, it allows the driver to use dma (if available) to speed up the request. In my case doing single cycle transfers would be too slow. We initially started with a read()/write() interface for individual register reads and writes, just like you describe. It turned out that mmap was plenty fast for our use. I made the decision to ditch all of the extra code needed to setup and execute the DMA for the much simpler mmap code. In our case, going all the way through the DMA engine code just to transfer 4 bytes is overkill. The local bus is already quite fast, and we can increase the clock speed if needed. Ira ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Wed, Feb 09, 2011 at 12:33:25AM -0800, Dmitry Torokhov wrote:
Hi Ira,
On Tue, Feb 08, 2011 at 03:37:46PM -0800, Ira W. Snyder wrote:
This driver allows userspace to access the data processing FPGAs on the
OVRO CARMA board. It has two modes of operation:
Thank you for making the changes, some more comments below.
+
+#define inode_to_dev(inode) container_of(inode-i_cdev, struct
fpga_device, cdev)
+
Does not seem to be used.
Leftovers from earlier versions, will remove.
+/*
+ * Data Buffer Allocation Helpers
+ */
+
+static int data_map_buffer(struct device *dev, struct data_buf *buf)
+{
+ return videobuf_dma_map(dev, buf-vb);
+}
+
+static void data_unmap_buffer(struct device *dev, struct data_buf *buf)
+{
+ videobuf_dma_unmap(dev, buf-vb);
+}
Why can't we all videobuf_dma_map() and videobuf_dma_unmap() directly?
What these helpers supposed to solve?
The helpers were useful in the past. Now they are not. Will change.
+static int data_alloc_buffers(struct fpga_device *priv)
+{
+ struct data_buf *buf;
+ int i, ret;
+
+ for (i = 0; i MAX_DATA_BUFS; i++) {
+
+ /* allocate a buffer */
+ buf = data_alloc_buffer(priv-bufsize);
+ if (!buf)
+ continue;
break?
+
+ /* map it for DMA */
+ ret = data_map_buffer(priv-dev, buf);
+ if (ret) {
+ data_free_buffer(buf);
+ continue;
and here as well?
Yep, break is fine also.
+ }
+
+ /* add it to the list of free buffers */
+ list_add_tail(buf-entry, priv-free);
+ priv-num_buffers++;
+ }
+
+ /* Make sure we allocated the minimum required number of buffers */
+ if (priv-num_buffers MIN_DATA_BUFS) {
+ dev_err(priv-dev, Unable to allocate enough data buffers\n);
+ data_free_buffers(priv);
+ return -ENOMEM;
+ }
+
+ /* Warn if we are running in a degraded state, but do not fail */
+ if (priv-num_buffers MAX_DATA_BUFS) {
+ dev_warn(priv-dev, Unable to allocate one second worth of
+ buffers, using %d buffers instead\n, i);
The latest style is not break strings even if they exceed 80 column
limit to make sure they are easily greppable.
I usually just follow checkpatch warnings. I'll combine the strings onto
one line.
+static void data_dma_cb(void *data)
+{
+ struct fpga_device *priv = data;
+ struct data_buf *buf;
+ unsigned long flags;
+
+ spin_lock_irqsave(priv-lock, flags);
+
+ /* clear the FPGA status and re-enable interrupts */
+ data_enable_interrupts(priv);
+
+ /* If the inflight list is empty, we've got a bug */
+ BUG_ON(list_empty(priv-inflight));
+
+ /* Grab the first buffer from the inflight list */
+ buf = list_first_entry(priv-inflight, struct data_buf, entry);
+ list_del_init(buf-entry);
+
+ /* Add it to the used list */
+ list_add_tail(buf-entry, priv-used);
or
list_move_tail(buf-entry, priv-used);
Using list_move_tail() didn't occur to me. I'll change it.
+
+static irqreturn_t data_irq(int irq, void *dev_id)
+{
+ struct fpga_device *priv = dev_id;
+ struct data_buf *buf;
+ u32 status;
+ int i;
+
+ /* detect spurious interrupts via FPGA status */
+ for (i = 0; i 4; i++) {
+ status = fpga_read_reg(priv, i, MMAP_REG_STATUS);
+ if (!(status (CORL_DONE | CORL_ERR))) {
+ dev_err(priv-dev, spurious irq detected (FPGA)\n);
+ return IRQ_NONE;
+ }
+ }
+
+ /* detect spurious interrupts via raw IRQ pin readback */
+ status = ioread32be(priv-regs + SYS_IRQ_INPUT_DATA);
+ if (status IRQ_CORL_DONE) {
+ dev_err(priv-dev, spurious irq detected (IRQ)\n);
+ return IRQ_NONE;
+ }
+
+ spin_lock(priv-lock);
+
+ /* hide the interrupt by switching the IRQ driver to GPIO */
+ data_disable_interrupts(priv);
+
+ /* Check that we actually have a free buffer */
+ if (list_empty(priv-free)) {
+ priv-num_dropped++;
+ data_enable_interrupts(priv);
+ goto out_unlock;
+ }
+
+ buf = list_first_entry(priv-free, struct data_buf, entry);
+ list_del_init(buf-entry);
+
+ /* Check the buffer size */
+ BUG_ON(buf-size != priv-bufsize);
+
+ /* Submit a DMA transfer to get the correlation data */
+ if (data_submit_dma(priv, buf)) {
+ dev_err(priv-dev, Unable to setup DMA transfer\n);
+ list_add_tail(buf-entry, priv-free);
+ data_enable_interrupts(priv);
+ goto out_unlock;
+ }
+
+ /* DMA setup succeeded, GO!!! */
+ list_add_tail(buf-entry, priv-inflight);
+ dma_async_memcpy_issue_pending(priv-chan);
+
+out_unlock:
+
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Wed, Feb 09, 2011 at 09:35:32AM -0800, Ira W. Snyder wrote:
On Wed, Feb 09, 2011 at 12:33:25AM -0800, Dmitry Torokhov wrote:
+
+ /* Warn if we are running in a degraded state, but do not fail */
+ if (priv-num_buffers MAX_DATA_BUFS) {
+ dev_warn(priv-dev, Unable to allocate one second worth of
+ buffers, using %d buffers instead\n, i);
The latest style is not break strings even if they exceed 80 column
limit to make sure they are easily greppable.
I usually just follow checkpatch warnings. I'll combine the strings onto
one line.
Does it still warn? I think they tried to fix it so it recognizes long
messages.
OTOH, we can't have more than 1 in-flight buffer, can we? Should
we even have a list?
This looks like a good change. I didn't know about list_move_tail()
before you mentioned it. Good catch.
You are correct that it is not possible to have more than one in flight
buffer at the same time. It was previously possible in an earlier
version of the driver. That was before I was forced to come up with the
ugly IRQ disabling scheme due to the hardware design.
What would you replace the inflight list with? A struct data_buf
*inflight; in the priv structure?
Yes. Then one would not worry of it is safe to always mark first element
of in-flight list as complete
Have you considered enabling the device when someone opens the device
node and closing it when last user drops off?
Yes. Unfortunately it is not possible. Blame my userspace software
engineers, who refused this model of operation.
I must meet the requirement that the device must remain open while the
DATA-FPGAs are reconfigured. This means that the FPGAs are completely
powered down, and a new FPGA bitfile is loaded into them.
After a reconfiguration, the data buffer size may change.
The userspace coders were willing to use a sysfs file to enable/disable
reading from the device, but they were not willing to open/close the
device each time. It was the best compromise they would accept.
I'm not happy about it either.
I see.
+
+/*
+ * FPGA Realtime Data Character Device
+ */
+
+static int data_open(struct inode *inode, struct file *filp)
+{
+ /*
+ * The miscdevice layer puts our struct miscdevice into the
+ * filp-private_data field. We use this to find our private
+ * data and then overwrite it with our own private structure.
+ */
+ struct fpga_device *priv = container_of(filp-private_data,
+ struct fpga_device, miscdev);
+ struct fpga_reader *reader;
+ int ret;
+
+ /* allocate private data */
+ reader = kzalloc(sizeof(*reader), GFP_KERNEL);
+ if (!reader)
+ return -ENOMEM;
+
+ reader-priv = priv;
+ reader-buf = NULL;
+
+ filp-private_data = reader;
+ ret = nonseekable_open(inode, filp);
+ if (ret) {
+ dev_err(priv-dev, nonseekable-open failed\n);
+ kfree(reader);
+ return ret;
+ }
+
I wonder if the device should allow only exclusive open... Unless you
distribute copies of data between all readers.
I wish to allow multiple different processes to mmap() the device. This
requires open(), followed by mmap(). I only care about a single realtime
data reader (which calls read()). Splitting the two use cases into two
drivers seemed like a big waste of time and effort. I have no idea how
to accomplish a single read()er while allowing multiple mmap()ers.
I see.
+ return 0;
+}
+
+static int data_release(struct inode *inode, struct file *filp)
+{
+ struct fpga_reader *reader = filp-private_data;
+
+ /* free the per-reader structure */
+ data_free_buffer(reader-buf);
+ kfree(reader);
+ filp-private_data = NULL;
+ return 0;
+}
+
+static ssize_t data_read(struct file *filp, char __user *ubuf, size_t
count,
+ loff_t *f_pos)
+{
+ struct fpga_reader *reader = filp-private_data;
+ struct fpga_device *priv = reader-priv;
+ struct list_head *used = priv-used;
+ struct data_buf *dbuf;
+ size_t avail;
+ void *data;
+ int ret;
+
+ /* check if we already have a partial buffer */
+ if (reader-buf) {
+ dbuf = reader-buf;
+ goto have_buffer;
+ }
+
+ spin_lock_irq(priv-lock);
+
+ /* Block until there is at least one buffer on the used list */
+ while (list_empty(used)) {
I believe we should stop and return error when device is disabled so the
condition should be:
while (!reader-buf list_empty() priv-enabled)
The requirement is that the device stay open during reconfiguration.
This provides for that. Readers just block for as long as the device is
not producing data.
OK, you still need to make sure you do not touch free/used buffer while
device is disabled. Also, you need to kick readers if
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Wed, Feb 09, 2011 at 10:27:40AM -0800, Dmitry Torokhov wrote: [ snip stuff I've already fixed in the next version ] The requirement is that the device stay open during reconfiguration. This provides for that. Readers just block for as long as the device is not producing data. OK, you still need to make sure you do not touch free/used buffer while device is disabled. Also, you need to kick readers if you unbind the driver, so maybe a new flag priv-exists should be introduced and checked. I don't understand what you mean by kick readers if you unbind the driver. The kernel automatically increases the refcount on a module when a process is using the module. This shows up in the Used by column of lsmod's output. The kernel will not let you rmmod a module with a non-zero refcount. You cannot get into the situation where you have rmmod'ed the module and a reader is still blocking in read()/poll(). Thanks for the review. A v6 is coming right up. Ira ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Wed, Feb 09, 2011 at 03:35:45PM -0800, Ira W. Snyder wrote: On Wed, Feb 09, 2011 at 10:27:40AM -0800, Dmitry Torokhov wrote: [ snip stuff I've already fixed in the next version ] The requirement is that the device stay open during reconfiguration. This provides for that. Readers just block for as long as the device is not producing data. OK, you still need to make sure you do not touch free/used buffer while device is disabled. Also, you need to kick readers if you unbind the driver, so maybe a new flag priv-exists should be introduced and checked. I don't understand what you mean by kick readers if you unbind the driver. The kernel automatically increases the refcount on a module when a process is using the module. This shows up in the Used by column of lsmod's output. The kernel will not let you rmmod a module with a non-zero refcount. You cannot get into the situation where you have rmmod'ed the module and a reader is still blocking in read()/poll(). However you can still unbind the driver from the device by writing into driver's sysfs 'unbind' attribute. See drivers/base/bus.c::driver_unbind(). Thanks. -- Dmitry ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Wed, Feb 09, 2011 at 03:42:31PM -0800, Dmitry Torokhov wrote: On Wed, Feb 09, 2011 at 03:35:45PM -0800, Ira W. Snyder wrote: On Wed, Feb 09, 2011 at 10:27:40AM -0800, Dmitry Torokhov wrote: [ snip stuff I've already fixed in the next version ] The requirement is that the device stay open during reconfiguration. This provides for that. Readers just block for as long as the device is not producing data. OK, you still need to make sure you do not touch free/used buffer while device is disabled. Also, you need to kick readers if you unbind the driver, so maybe a new flag priv-exists should be introduced and checked. I don't understand what you mean by kick readers if you unbind the driver. The kernel automatically increases the refcount on a module when a process is using the module. This shows up in the Used by column of lsmod's output. The kernel will not let you rmmod a module with a non-zero refcount. You cannot get into the situation where you have rmmod'ed the module and a reader is still blocking in read()/poll(). However you can still unbind the driver from the device by writing into driver's sysfs 'unbind' attribute. See drivers/base/bus.c::driver_unbind(). I was completely unaware of that feature. I hunch that many drivers are incapable of dealing with an unbind while they are still open. Matter of fact, I don't see how this can EVER be safe. The driver core automatically calls the data_of_remove() routine while there are still blocked readers. This kfree()s the private data structure, which contains the suggested priv-exists flag. What happens if the memory allocator re-allocates that memory to a different driver before the reader process is woken up to check the priv-exists flag? The only way to solve this is to count the number of open()s and close()s, and block the unbind until all users have close()d the device. Thanks, Ira ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Wed, Feb 09, 2011 at 04:10:55PM -0800, Ira W. Snyder wrote: On Wed, Feb 09, 2011 at 03:42:31PM -0800, Dmitry Torokhov wrote: On Wed, Feb 09, 2011 at 03:35:45PM -0800, Ira W. Snyder wrote: On Wed, Feb 09, 2011 at 10:27:40AM -0800, Dmitry Torokhov wrote: [ snip stuff I've already fixed in the next version ] The requirement is that the device stay open during reconfiguration. This provides for that. Readers just block for as long as the device is not producing data. OK, you still need to make sure you do not touch free/used buffer while device is disabled. Also, you need to kick readers if you unbind the driver, so maybe a new flag priv-exists should be introduced and checked. I don't understand what you mean by kick readers if you unbind the driver. The kernel automatically increases the refcount on a module when a process is using the module. This shows up in the Used by column of lsmod's output. The kernel will not let you rmmod a module with a non-zero refcount. You cannot get into the situation where you have rmmod'ed the module and a reader is still blocking in read()/poll(). However you can still unbind the driver from the device by writing into driver's sysfs 'unbind' attribute. See drivers/base/bus.c::driver_unbind(). I was completely unaware of that feature. I hunch that many drivers are incapable of dealing with an unbind while they are still open. Hmm, maybe older drivers... Anythig hotpluggable (USB, PCI, etc) should be in a better shape because they expect to be yanked at any time. Matter of fact, I don't see how this can EVER be safe. The driver core automatically calls the data_of_remove() routine while there are still blocked readers. This kfree()s the private data structure, which contains the suggested priv-exists flag. What happens if the memory allocator re-allocates that memory to a different driver before the reader process is woken up to check the priv-exists flag? The only way to solve this is to count the number of open()s and close()s, and block the unbind until all users have close()d the device. Yes, you can kick readers and wait, or you can refcount that private structure and have readers grab a reference when they open your device and drop it in their fops-release() method. Your remove() should also drop reference instead of doing kfree() outright. Thanks. -- Dmitry ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Mon, Feb 07, 2011 at 11:33:10PM -0800, Dmitry Torokhov wrote: Hi Ira, On Mon, Feb 07, 2011 at 03:23:40PM -0800, Ira W. Snyder wrote: This driver allows userspace to access the data processing FPGAs on the OVRO CARMA board. It has two modes of operation: 1) random access This allows users to poke any DATA-FPGA registers by using mmap to map the address region directly into their memory map. 2) correlation dumping When correlating, the DATA-FPGA's have special requirements for getting the data out of their memory before the next correlation. This nominally happens at 64Hz (every 15.625ms). If the data is not dumped before the next correlation, data is lost. The data dumping driver handles buffering up to 1 second worth of correlation data from the FPGAs. This lowers the realtime scheduling requirements for the userspace process reading the device. Kind of a fly-by review but it looks like the locking in the driver needs work. Hi Dmitry, Thanks for the review. I have a few comments inline below. Signed-off-by: Ira W. Snyder i...@ovro.caltech.edu --- drivers/misc/Kconfig|1 + drivers/misc/Makefile |1 + drivers/misc/carma/Kconfig |9 + drivers/misc/carma/Makefile |1 + drivers/misc/carma/carma-fpga.c | 1446 +++ 5 files changed, 1458 insertions(+), 0 deletions(-) create mode 100644 drivers/misc/carma/Kconfig create mode 100644 drivers/misc/carma/Makefile create mode 100644 drivers/misc/carma/carma-fpga.c diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 4d073f1..f457f14 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -457,5 +457,6 @@ source drivers/misc/eeprom/Kconfig source drivers/misc/cb710/Kconfig source drivers/misc/iwmc3200top/Kconfig source drivers/misc/ti-st/Kconfig +source drivers/misc/carma/Kconfig endif # MISC_DEVICES diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 98009cc..2c1610e 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -42,3 +42,4 @@ obj-$(CONFIG_ARM_CHARLCD) += arm-charlcd.o obj-$(CONFIG_PCH_PHUB) += pch_phub.o obj-y += ti-st/ obj-$(CONFIG_AB8500_PWM) += ab8500-pwm.o +obj-y += carma/ diff --git a/drivers/misc/carma/Kconfig b/drivers/misc/carma/Kconfig new file mode 100644 index 000..4be183f --- /dev/null +++ b/drivers/misc/carma/Kconfig @@ -0,0 +1,9 @@ +config CARMA_FPGA + tristate CARMA DATA-FPGA Access Driver + depends on FSL_SOC PPC_83xx MEDIA_SUPPORT HAS_DMA FSL_DMA + select VIDEOBUF_DMA_SG + default n + help + Say Y here to include support for communicating with the data + processing FPGAs on the OVRO CARMA board. + diff --git a/drivers/misc/carma/Makefile b/drivers/misc/carma/Makefile new file mode 100644 index 000..0b69fa7 --- /dev/null +++ b/drivers/misc/carma/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_CARMA_FPGA) += carma-fpga.o diff --git a/drivers/misc/carma/carma-fpga.c b/drivers/misc/carma/carma-fpga.c new file mode 100644 index 000..52620b3 --- /dev/null +++ b/drivers/misc/carma/carma-fpga.c @@ -0,0 +1,1446 @@ +/* + * CARMA DATA-FPGA Access Driver + * + * Copyright (c) 2009-2010 Ira W. Snyder i...@ovro.caltech.edu + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +/* + * FPGA Memory Dump Format + * + * FPGA #0 control registers (32 x 32-bit words) + * FPGA #1 control registers (32 x 32-bit words) + * FPGA #2 control registers (32 x 32-bit words) + * FPGA #3 control registers (32 x 32-bit words) + * SYSFPGA control registers (32 x 32-bit words) + * FPGA #0 correlation array (NUM_CORL0 correlation blocks) + * FPGA #1 correlation array (NUM_CORL1 correlation blocks) + * FPGA #2 correlation array (NUM_CORL2 correlation blocks) + * FPGA #3 correlation array (NUM_CORL3 correlation blocks) + * + * Each correlation array consists of: + * + * Correlation Data (2 x NUM_LAGSn x 32-bit words) + * Pipeline Metadata (2 x NUM_METAn x 32-bit words) + * Quantization Counters (2 x NUM_QCNTn x 32-bit words) + * + * The NUM_CORLn, NUM_LAGSn, NUM_METAn, and NUM_QCNTn values come from + * the FPGA configuration registers. They do not change once the FPGA's + * have been programmed, they only change on re-programming. + */ + +/* + * Basic Description: + * + * This driver is used to capture correlation spectra off of the four data + * processing FPGAs. The FPGAs are often reprogrammed at runtime, therefore + * this driver supports dynamic
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Tue, Feb 08, 2011 at 09:20:46AM -0800, Ira W. Snyder wrote: +static DEVICE_ATTR(enable, S_IWUGO | S_IRUGO, data_en_show, data_en_set); Are all of these really needed or most of them are for debug? Most are for debugging. They have proved useful a few times in production to track down bugs. File mode should probably not be world writable. (checkpatch.pl should warn you about this now btw) Dave ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Tue, Feb 08, 2011 at 09:20:46AM -0800, Ira W. Snyder wrote:
On Mon, Feb 07, 2011 at 11:33:10PM -0800, Dmitry Torokhov wrote:
+static void data_free_buffer(struct device *dev, struct data_buf *buf)
+{
+ /* It is ok to free a NULL buffer */
+ if (!buf)
+ return;
+
+ /* Make sure the buffer is not on any list */
+ list_del_init(buf-entry);
And what happens if it is? Should it be WARN_ON(!list_empty()) instead?
This was only defensive programming. Everywhere this function is called,
the buffer has already been removed from the list.
I am concerned as sometimes defencive programming is the sign that we
arenot quite sure how the code works. I believe defensive programming
should be used when providing library-like code, not in local cases.
+
+ list_for_each_entry_safe(buf, tmp, priv-free, entry) {
+ list_del_init(buf-entry);
+ spin_unlock_irq(priv-lock);
+ data_free_buffer(priv-dev, buf);
+ spin_lock_irq(priv-lock);
+ }
This is messed up. If there is concurrent access to the free list then
it is not safe to continue iterating list after releasing the lock, you
need to do:
spin_lock_irq(priv-lock);
while (!list_empty(priv-free)) {
buf = list_first_entry(priv-free, struct data_buf, entry);
list_del_init(buf-entry);
spin_unlock_irq(priv-lock);
data_free_buffer(priv-dev, buf);
spin_lock_irq(priv-lock);
}
BUT, the function is only called when you disable (or fail to enable) device
which, at this point, should be quiesced, thus all this locking is not
really needed.
Correct.
I thought it would be clearer to reviewers if I always used the lock to
protect a data structure, even when it isn't technically needed.
No, locks should protect wehat needs to be protected. The rest just
muddles water.
+
+ spin_lock_irq(priv-lock);
+ while (!list_empty(list)) {
+ spin_unlock_irq(priv-lock);
+
+ ret = wait_event_interruptible(priv-wait, list_empty(list));
+ if (ret)
+ return -ERESTARTSYS;
+
+ spin_lock_irq(priv-lock);
+ }
+ spin_unlock_irq(priv-lock);
Locking is not needed - if you disable interrupyts what would put more
stuff on the list?
The locking is definitely needed.
You've missed a critical piece of information. There are *two* devices
we are interacting with here, and BOTH generate interrupts.
No, I did not miss this fact. The point is that when we get to this code
the device _putting_ items on wauiting list is stopped and we only need
to wait for the list to drain. Nobody puts more stuff on it. You can
check fir list_empty() condition without locking.
And if someone _is_ putting more stuff on the list - you are screwed
since list may become non-empty the moment you release the lock.
+
+static ssize_t data_num_buffers_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct fpga_device *priv = dev_get_drvdata(dev);
+ unsigned int num;
+
+ spin_lock_irq(priv-lock);
+ num = priv-num_buffers;
+ spin_unlock_irq(priv-lock);
This spin lock is pointless, priv-num_buffers might be already changed
here, you can't guarantee that you show accurate data.
Correct, I know this. I just wanted to protect the data structure at all
points of use in the driver.
Protect from what? integer reads are guaranteed to be complete and you
are not concerned with missing updates as information is obsolete the
moment you release trhe lock.
Would an atomic_t be better for this, or
should I just remove the locking completely?
Just remove the locking.
+
+ if (mutex_lock_interruptible(priv-mutex))
+ return -ERESTARTSYS;
Why don't
error = mutex_lock_interruptible(priv-mutex);
if (error)
return error;
- do not clobber perfectly valid error codes.
That's what the Linux Device Drivers 3rd Edition book does. See page
112. I will change it to fix the return code.
LDD3 is quite old by now...
+
+static struct attribute *data_sysfs_attrs[] = {
+ dev_attr_num_buffers.attr,
+ dev_attr_buffer_size.attr,
+ dev_attr_num_inflight.attr,
+ dev_attr_num_free.attr,
+ dev_attr_num_used.attr,
+ dev_attr_num_dropped.attr,
+ dev_attr_enable.attr,
+ NULL,
+};
Are all of these really needed or most of them are for debug?
Most are for debugging. They have proved useful a few times in
production to track down bugs.
Consider switching over to debugfs then. Or, if you believe the device
is sufficiently bug-free just cut the code.
+
+static ssize_t data_read(struct file *filp, char __user *ubuf, size_t
count,
+ loff_t *f_pos)
+{
+ struct fpga_reader *reader = filp-private_data;
+ struct fpga_device *priv =
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Tue, Feb 08, 2011 at 09:50:29AM -0800, Dmitry Torokhov wrote:
On Tue, Feb 08, 2011 at 09:20:46AM -0800, Ira W. Snyder wrote:
On Mon, Feb 07, 2011 at 11:33:10PM -0800, Dmitry Torokhov wrote:
+static void data_free_buffer(struct device *dev, struct data_buf *buf)
+{
+ /* It is ok to free a NULL buffer */
+ if (!buf)
+ return;
+
+ /* Make sure the buffer is not on any list */
+ list_del_init(buf-entry);
And what happens if it is? Should it be WARN_ON(!list_empty()) instead?
This was only defensive programming. Everywhere this function is called,
the buffer has already been removed from the list.
I am concerned as sometimes defencive programming is the sign that we
arenot quite sure how the code works. I believe defensive programming
should be used when providing library-like code, not in local cases.
Ok.
+
+ list_for_each_entry_safe(buf, tmp, priv-free, entry) {
+ list_del_init(buf-entry);
+ spin_unlock_irq(priv-lock);
+ data_free_buffer(priv-dev, buf);
+ spin_lock_irq(priv-lock);
+ }
This is messed up. If there is concurrent access to the free list then
it is not safe to continue iterating list after releasing the lock, you
need to do:
spin_lock_irq(priv-lock);
while (!list_empty(priv-free)) {
buf = list_first_entry(priv-free, struct data_buf, entry);
list_del_init(buf-entry);
spin_unlock_irq(priv-lock);
data_free_buffer(priv-dev, buf);
spin_lock_irq(priv-lock);
}
BUT, the function is only called when you disable (or fail to enable)
device
which, at this point, should be quiesced, thus all this locking is not
really needed.
Correct.
I thought it would be clearer to reviewers if I always used the lock to
protect a data structure, even when it isn't technically needed.
No, locks should protect wehat needs to be protected. The rest just
muddles water.
Ok.
+
+ spin_lock_irq(priv-lock);
+ while (!list_empty(list)) {
+ spin_unlock_irq(priv-lock);
+
+ ret = wait_event_interruptible(priv-wait,
list_empty(list));
+ if (ret)
+ return -ERESTARTSYS;
+
+ spin_lock_irq(priv-lock);
+ }
+ spin_unlock_irq(priv-lock);
Locking is not needed - if you disable interrupyts what would put more
stuff on the list?
The locking is definitely needed.
You've missed a critical piece of information. There are *two* devices
we are interacting with here, and BOTH generate interrupts.
No, I did not miss this fact. The point is that when we get to this code
the device _putting_ items on wauiting list is stopped and we only need
to wait for the list to drain. Nobody puts more stuff on it. You can
check fir list_empty() condition without locking.
And if someone _is_ putting more stuff on the list - you are screwed
since list may become non-empty the moment you release the lock.
Ok, I understand what you mean now. You are correct, nothing else can
add things to the list. Thanks for clarifying this for me. :)
+
+static ssize_t data_num_buffers_show(struct device *dev,
+struct device_attribute *attr,
char *buf)
+{
+ struct fpga_device *priv = dev_get_drvdata(dev);
+ unsigned int num;
+
+ spin_lock_irq(priv-lock);
+ num = priv-num_buffers;
+ spin_unlock_irq(priv-lock);
This spin lock is pointless, priv-num_buffers might be already changed
here, you can't guarantee that you show accurate data.
Correct, I know this. I just wanted to protect the data structure at all
points of use in the driver.
Protect from what? integer reads are guaranteed to be complete and you
are not concerned with missing updates as information is obsolete the
moment you release trhe lock.
Would an atomic_t be better for this, or
should I just remove the locking completely?
Just remove the locking.
Ok.
+
+ if (mutex_lock_interruptible(priv-mutex))
+ return -ERESTARTSYS;
Why don't
error = mutex_lock_interruptible(priv-mutex);
if (error)
return error;
- do not clobber perfectly valid error codes.
That's what the Linux Device Drivers 3rd Edition book does. See page
112. I will change it to fix the return code.
LDD3 is quite old by now...
I know, but it is still the best written reference I have. Reviewers
like yourself are better, but I can't look up your advice in a book. :)
I'll return the error code.
+
+static struct attribute *data_sysfs_attrs[] = {
+ dev_attr_num_buffers.attr,
+
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
On Tue, Feb 08, 2011 at 11:11:44AM -0800, Ira W. Snyder wrote: On Tue, Feb 08, 2011 at 09:50:29AM -0800, Dmitry Torokhov wrote: On Tue, Feb 08, 2011 at 09:20:46AM -0800, Ira W. Snyder wrote: Go back and re-think my loop. This is a common idiom straight of out LDD3 pages 153-154. You should note that it is only possible to exit the loop with the lock held AND !list_empty(used). The lock protects the used list, and therefore, there must be a buffer on the list. No, because you are woken up while not holding the lock so another reader is free to take it off the list. Correct. But then I go around the loop and check list_empty() again before exiting the loop. The list MUST NOT be empty before the loop will terminate. Yes, you are right, I competely missed the fact that we'd loop around and check the condition again. I'll go grab another coffee now. -- Dmitry ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
This driver allows userspace to access the data processing FPGAs on the OVRO CARMA board. It has two modes of operation: 1) random access This allows users to poke any DATA-FPGA registers by using mmap to map the address region directly into their memory map. 2) correlation dumping When correlating, the DATA-FPGA's have special requirements for getting the data out of their memory before the next correlation. This nominally happens at 64Hz (every 15.625ms). If the data is not dumped before the next correlation, data is lost. The data dumping driver handles buffering up to 1 second worth of correlation data from the FPGAs. This lowers the realtime scheduling requirements for the userspace process reading the device. Signed-off-by: Ira W. Snyder i...@ovro.caltech.edu --- drivers/misc/Kconfig|1 + drivers/misc/Makefile |1 + drivers/misc/carma/Kconfig |9 + drivers/misc/carma/Makefile |1 + drivers/misc/carma/carma-fpga.c | 1396 +++ 5 files changed, 1408 insertions(+), 0 deletions(-) create mode 100644 drivers/misc/carma/Kconfig create mode 100644 drivers/misc/carma/Makefile create mode 100644 drivers/misc/carma/carma-fpga.c diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index cc8e49d..93cf1e6 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -457,5 +457,6 @@ source drivers/misc/eeprom/Kconfig source drivers/misc/cb710/Kconfig source drivers/misc/iwmc3200top/Kconfig source drivers/misc/ti-st/Kconfig +source drivers/misc/carma/Kconfig endif # MISC_DEVICES diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 98009cc..2c1610e 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -42,3 +42,4 @@ obj-$(CONFIG_ARM_CHARLCD) += arm-charlcd.o obj-$(CONFIG_PCH_PHUB) += pch_phub.o obj-y += ti-st/ obj-$(CONFIG_AB8500_PWM) += ab8500-pwm.o +obj-y += carma/ diff --git a/drivers/misc/carma/Kconfig b/drivers/misc/carma/Kconfig new file mode 100644 index 000..4be183f --- /dev/null +++ b/drivers/misc/carma/Kconfig @@ -0,0 +1,9 @@ +config CARMA_FPGA + tristate CARMA DATA-FPGA Access Driver + depends on FSL_SOC PPC_83xx MEDIA_SUPPORT HAS_DMA FSL_DMA + select VIDEOBUF_DMA_SG + default n + help + Say Y here to include support for communicating with the data + processing FPGAs on the OVRO CARMA board. + diff --git a/drivers/misc/carma/Makefile b/drivers/misc/carma/Makefile new file mode 100644 index 000..0b69fa7 --- /dev/null +++ b/drivers/misc/carma/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_CARMA_FPGA) += carma-fpga.o diff --git a/drivers/misc/carma/carma-fpga.c b/drivers/misc/carma/carma-fpga.c new file mode 100644 index 000..4ea473a --- /dev/null +++ b/drivers/misc/carma/carma-fpga.c @@ -0,0 +1,1396 @@ +/* + * CARMA DATA-FPGA Access Driver + * + * Copyright (c) 2009-2011 Ira W. Snyder i...@ovro.caltech.edu + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +/* + * FPGA Memory Dump Format + * + * FPGA #0 control registers (32 x 32-bit words) + * FPGA #1 control registers (32 x 32-bit words) + * FPGA #2 control registers (32 x 32-bit words) + * FPGA #3 control registers (32 x 32-bit words) + * SYSFPGA control registers (32 x 32-bit words) + * FPGA #0 correlation array (NUM_CORL0 correlation blocks) + * FPGA #1 correlation array (NUM_CORL1 correlation blocks) + * FPGA #2 correlation array (NUM_CORL2 correlation blocks) + * FPGA #3 correlation array (NUM_CORL3 correlation blocks) + * + * Each correlation array consists of: + * + * Correlation Data (2 x NUM_LAGSn x 32-bit words) + * Pipeline Metadata (2 x NUM_METAn x 32-bit words) + * Quantization Counters (2 x NUM_QCNTn x 32-bit words) + * + * The NUM_CORLn, NUM_LAGSn, NUM_METAn, and NUM_QCNTn values come from + * the FPGA configuration registers. They do not change once the FPGA's + * have been programmed, they only change on re-programming. + */ + +/* + * Basic Description: + * + * This driver is used to capture correlation spectra off of the four data + * processing FPGAs. The FPGAs are often reprogrammed at runtime, therefore + * this driver supports dynamic enable/disable of capture while the device + * remains open. + * + * The nominal capture rate is 64Hz (every 15.625ms). To facilitate this fast + * capture rate, all buffers are pre-allocated to avoid any potentially long + * running memory allocations while capturing. + * + * There are three lists which are used to keep track of the different states + * of data buffers. + * + * 1) free list + * This list holds all empty data buffers which are ready to receive data. + * + * 2) inflight list + * This list holds data
[PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
This driver allows userspace to access the data processing FPGAs on the OVRO CARMA board. It has two modes of operation: 1) random access This allows users to poke any DATA-FPGA registers by using mmap to map the address region directly into their memory map. 2) correlation dumping When correlating, the DATA-FPGA's have special requirements for getting the data out of their memory before the next correlation. This nominally happens at 64Hz (every 15.625ms). If the data is not dumped before the next correlation, data is lost. The data dumping driver handles buffering up to 1 second worth of correlation data from the FPGAs. This lowers the realtime scheduling requirements for the userspace process reading the device. Signed-off-by: Ira W. Snyder i...@ovro.caltech.edu --- drivers/misc/Kconfig|1 + drivers/misc/Makefile |1 + drivers/misc/carma/Kconfig |9 + drivers/misc/carma/Makefile |1 + drivers/misc/carma/carma-fpga.c | 1446 +++ 5 files changed, 1458 insertions(+), 0 deletions(-) create mode 100644 drivers/misc/carma/Kconfig create mode 100644 drivers/misc/carma/Makefile create mode 100644 drivers/misc/carma/carma-fpga.c diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 4d073f1..f457f14 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -457,5 +457,6 @@ source drivers/misc/eeprom/Kconfig source drivers/misc/cb710/Kconfig source drivers/misc/iwmc3200top/Kconfig source drivers/misc/ti-st/Kconfig +source drivers/misc/carma/Kconfig endif # MISC_DEVICES diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 98009cc..2c1610e 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -42,3 +42,4 @@ obj-$(CONFIG_ARM_CHARLCD) += arm-charlcd.o obj-$(CONFIG_PCH_PHUB) += pch_phub.o obj-y += ti-st/ obj-$(CONFIG_AB8500_PWM) += ab8500-pwm.o +obj-y += carma/ diff --git a/drivers/misc/carma/Kconfig b/drivers/misc/carma/Kconfig new file mode 100644 index 000..4be183f --- /dev/null +++ b/drivers/misc/carma/Kconfig @@ -0,0 +1,9 @@ +config CARMA_FPGA + tristate CARMA DATA-FPGA Access Driver + depends on FSL_SOC PPC_83xx MEDIA_SUPPORT HAS_DMA FSL_DMA + select VIDEOBUF_DMA_SG + default n + help + Say Y here to include support for communicating with the data + processing FPGAs on the OVRO CARMA board. + diff --git a/drivers/misc/carma/Makefile b/drivers/misc/carma/Makefile new file mode 100644 index 000..0b69fa7 --- /dev/null +++ b/drivers/misc/carma/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_CARMA_FPGA) += carma-fpga.o diff --git a/drivers/misc/carma/carma-fpga.c b/drivers/misc/carma/carma-fpga.c new file mode 100644 index 000..52620b3 --- /dev/null +++ b/drivers/misc/carma/carma-fpga.c @@ -0,0 +1,1446 @@ +/* + * CARMA DATA-FPGA Access Driver + * + * Copyright (c) 2009-2010 Ira W. Snyder i...@ovro.caltech.edu + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +/* + * FPGA Memory Dump Format + * + * FPGA #0 control registers (32 x 32-bit words) + * FPGA #1 control registers (32 x 32-bit words) + * FPGA #2 control registers (32 x 32-bit words) + * FPGA #3 control registers (32 x 32-bit words) + * SYSFPGA control registers (32 x 32-bit words) + * FPGA #0 correlation array (NUM_CORL0 correlation blocks) + * FPGA #1 correlation array (NUM_CORL1 correlation blocks) + * FPGA #2 correlation array (NUM_CORL2 correlation blocks) + * FPGA #3 correlation array (NUM_CORL3 correlation blocks) + * + * Each correlation array consists of: + * + * Correlation Data (2 x NUM_LAGSn x 32-bit words) + * Pipeline Metadata (2 x NUM_METAn x 32-bit words) + * Quantization Counters (2 x NUM_QCNTn x 32-bit words) + * + * The NUM_CORLn, NUM_LAGSn, NUM_METAn, and NUM_QCNTn values come from + * the FPGA configuration registers. They do not change once the FPGA's + * have been programmed, they only change on re-programming. + */ + +/* + * Basic Description: + * + * This driver is used to capture correlation spectra off of the four data + * processing FPGAs. The FPGAs are often reprogrammed at runtime, therefore + * this driver supports dynamic enable/disable of capture while the device + * remains open. + * + * The nominal capture rate is 64Hz (every 15.625ms). To facilitate this fast + * capture rate, all buffers are pre-allocated to avoid any potentially long + * running memory allocations while capturing. + * + * There are three lists which are used to keep track of the different states + * of data buffers. + * + * 1) free list + * This list holds all empty data buffers which are ready to receive data. + * + * 2) inflight list + * This list holds data
Re: [PATCH 1/2] misc: add CARMA DATA-FPGA Access Driver
Hi Ira, On Mon, Feb 07, 2011 at 03:23:40PM -0800, Ira W. Snyder wrote: This driver allows userspace to access the data processing FPGAs on the OVRO CARMA board. It has two modes of operation: 1) random access This allows users to poke any DATA-FPGA registers by using mmap to map the address region directly into their memory map. 2) correlation dumping When correlating, the DATA-FPGA's have special requirements for getting the data out of their memory before the next correlation. This nominally happens at 64Hz (every 15.625ms). If the data is not dumped before the next correlation, data is lost. The data dumping driver handles buffering up to 1 second worth of correlation data from the FPGAs. This lowers the realtime scheduling requirements for the userspace process reading the device. Kind of a fly-by review but it looks like the locking in the driver needs work. Signed-off-by: Ira W. Snyder i...@ovro.caltech.edu --- drivers/misc/Kconfig|1 + drivers/misc/Makefile |1 + drivers/misc/carma/Kconfig |9 + drivers/misc/carma/Makefile |1 + drivers/misc/carma/carma-fpga.c | 1446 +++ 5 files changed, 1458 insertions(+), 0 deletions(-) create mode 100644 drivers/misc/carma/Kconfig create mode 100644 drivers/misc/carma/Makefile create mode 100644 drivers/misc/carma/carma-fpga.c diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index 4d073f1..f457f14 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -457,5 +457,6 @@ source drivers/misc/eeprom/Kconfig source drivers/misc/cb710/Kconfig source drivers/misc/iwmc3200top/Kconfig source drivers/misc/ti-st/Kconfig +source drivers/misc/carma/Kconfig endif # MISC_DEVICES diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 98009cc..2c1610e 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -42,3 +42,4 @@ obj-$(CONFIG_ARM_CHARLCD) += arm-charlcd.o obj-$(CONFIG_PCH_PHUB) += pch_phub.o obj-y+= ti-st/ obj-$(CONFIG_AB8500_PWM) += ab8500-pwm.o +obj-y+= carma/ diff --git a/drivers/misc/carma/Kconfig b/drivers/misc/carma/Kconfig new file mode 100644 index 000..4be183f --- /dev/null +++ b/drivers/misc/carma/Kconfig @@ -0,0 +1,9 @@ +config CARMA_FPGA + tristate CARMA DATA-FPGA Access Driver + depends on FSL_SOC PPC_83xx MEDIA_SUPPORT HAS_DMA FSL_DMA + select VIDEOBUF_DMA_SG + default n + help + Say Y here to include support for communicating with the data + processing FPGAs on the OVRO CARMA board. + diff --git a/drivers/misc/carma/Makefile b/drivers/misc/carma/Makefile new file mode 100644 index 000..0b69fa7 --- /dev/null +++ b/drivers/misc/carma/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_CARMA_FPGA) += carma-fpga.o diff --git a/drivers/misc/carma/carma-fpga.c b/drivers/misc/carma/carma-fpga.c new file mode 100644 index 000..52620b3 --- /dev/null +++ b/drivers/misc/carma/carma-fpga.c @@ -0,0 +1,1446 @@ +/* + * CARMA DATA-FPGA Access Driver + * + * Copyright (c) 2009-2010 Ira W. Snyder i...@ovro.caltech.edu + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +/* + * FPGA Memory Dump Format + * + * FPGA #0 control registers (32 x 32-bit words) + * FPGA #1 control registers (32 x 32-bit words) + * FPGA #2 control registers (32 x 32-bit words) + * FPGA #3 control registers (32 x 32-bit words) + * SYSFPGA control registers (32 x 32-bit words) + * FPGA #0 correlation array (NUM_CORL0 correlation blocks) + * FPGA #1 correlation array (NUM_CORL1 correlation blocks) + * FPGA #2 correlation array (NUM_CORL2 correlation blocks) + * FPGA #3 correlation array (NUM_CORL3 correlation blocks) + * + * Each correlation array consists of: + * + * Correlation Data (2 x NUM_LAGSn x 32-bit words) + * Pipeline Metadata (2 x NUM_METAn x 32-bit words) + * Quantization Counters (2 x NUM_QCNTn x 32-bit words) + * + * The NUM_CORLn, NUM_LAGSn, NUM_METAn, and NUM_QCNTn values come from + * the FPGA configuration registers. They do not change once the FPGA's + * have been programmed, they only change on re-programming. + */ + +/* + * Basic Description: + * + * This driver is used to capture correlation spectra off of the four data + * processing FPGAs. The FPGAs are often reprogrammed at runtime, therefore + * this driver supports dynamic enable/disable of capture while the device + * remains open. + * + * The nominal capture rate is 64Hz (every 15.625ms). To facilitate this fast + * capture rate, all buffers are pre-allocated to avoid any potentially long + * running memory
