[PATCH 0/3] mmap() for vme_user and CR/CSR fix for master
Dear all, mmap() on VME bridge devices is a feature present in many existing Linux and UNIX drivers. The proposed solution follows the approach so that mmap() offsets are coherent with read() and write() offsets, which seems to be the only manner compatible with the current vme_user API. It also ends up adding vme_master_mmap call to the VME subsystem API. On one hand this might come useful for a board driver that will be able to expose one of its windows to the user that way. On the other hand it introduces dependency on vma_area_struct into vme.h, which doesn't seem very right. Also I'm bringing up a bugfix by Martyn Welch that was published on the list, but for some reason didn't make it to the kernel. Cheers, Dmitry Dmitry Kalinkin (2): staging: vme: use image mutex for ioctl() staging: vme: mmap() support for vme_user Martyn Welch (1): vme: tsi148: Master windows support USERx and CR/CSR accesses, not slaves drivers/staging/vme/devices/vme_user.c | 94 -- drivers/vme/bridges/vme_tsi148.c | 11 ++-- drivers/vme/vme.c | 26 ++ include/linux/vme.h| 1 + 4 files changed, 120 insertions(+), 12 deletions(-) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] vme: tsi148: Master windows support USERx and CR/CSR accesses, not slaves
From: Martyn Welch The tsi148 driver is registering the slave images as supporting the "USER" access modes and CR/CSR access mode rather than the master images as it should. Remove the incorrect case entries for these modes from the tsi148_slave_set() function, stop registering slave_images as supporting these modes and instead register master windows as supporting these modes. Signed-off-by: Martyn Welch Acked-by: Dmitry Kalinkin --- drivers/vme/bridges/vme_tsi148.c | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c index e07cfa8..895c2a3 100644 --- a/drivers/vme/bridges/vme_tsi148.c +++ b/drivers/vme/bridges/vme_tsi148.c @@ -587,11 +587,6 @@ static int tsi148_slave_set(struct vme_slave_resource *image, int enabled, granularity = 0x1; addr |= TSI148_LCSR_ITAT_AS_A64; break; - case VME_CRCSR: - case VME_USER1: - case VME_USER2: - case VME_USER3: - case VME_USER4: default: dev_err(tsi148_bridge->parent, "Invalid address space\n"); return -EINVAL; @@ -2471,7 +2466,8 @@ static int tsi148_probe(struct pci_dev *pdev, const struct pci_device_id *id) master_image->locked = 0; master_image->number = i; master_image->address_attr = VME_A16 | VME_A24 | VME_A32 | - VME_A64; + VME_A64 | VME_CRCSR | VME_USER1 | VME_USER2 | + VME_USER3 | VME_USER4; master_image->cycle_attr = VME_SCT | VME_BLT | VME_MBLT | VME_2eVME | VME_2eSST | VME_2eSSTB | VME_2eSST160 | VME_2eSST267 | VME_2eSST320 | VME_SUPER | VME_USER | @@ -2500,8 +2496,7 @@ static int tsi148_probe(struct pci_dev *pdev, const struct pci_device_id *id) slave_image->locked = 0; slave_image->number = i; slave_image->address_attr = VME_A16 | VME_A24 | VME_A32 | - VME_A64 | VME_CRCSR | VME_USER1 | VME_USER2 | - VME_USER3 | VME_USER4; + VME_A64; slave_image->cycle_attr = VME_SCT | VME_BLT | VME_MBLT | VME_2eVME | VME_2eSST | VME_2eSSTB | VME_2eSST160 | VME_2eSST267 | VME_2eSST320 | VME_SUPER | VME_USER | -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: vme: mmap() support for vme_user
We also make sure that user won't be able to reconfigure the window while it is
mmap'ed.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 85 ++
drivers/vme/vme.c | 26 +++
include/linux/vme.h| 1 +
3 files changed, 112 insertions(+)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 8731838..b7fe1f0 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -17,6 +17,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include
#include
#include
#include
@@ -99,6 +100,7 @@ struct image_desc {
struct device *device; /* Sysfs device */
struct vme_resource *resource; /* VME resource */
int users; /* Number of current users */
+ int mmap_count; /* Number of current mmap's */
};
static struct image_desc image[VME_DEVS];
@@ -134,6 +136,10 @@ static ssize_t vme_user_write(struct file *, const char
__user *, size_t,
loff_t *);
static loff_t vme_user_llseek(struct file *, loff_t, int);
static long vme_user_unlocked_ioctl(struct file *, unsigned int, unsigned
long);
+static int vme_user_mmap(struct file *file, struct vm_area_struct *vma);
+
+static void vme_user_vm_open(struct vm_area_struct *vma);
+static void vme_user_vm_close(struct vm_area_struct *vma);
static int vme_user_match(struct vme_dev *);
static int vme_user_probe(struct vme_dev *);
@@ -147,6 +153,17 @@ static const struct file_operations vme_user_fops = {
.llseek = vme_user_llseek,
.unlocked_ioctl = vme_user_unlocked_ioctl,
.compat_ioctl = vme_user_unlocked_ioctl,
+ .mmap = vme_user_mmap,
+};
+
+struct vme_user_vma_priv {
+ unsigned int minor;
+ atomic_t refcnt;
+};
+
+static const struct vm_operations_struct vme_user_vm_ops = {
+ .open = vme_user_vm_open,
+ .close = vme_user_vm_close,
};
@@ -488,6 +505,11 @@ static int vme_user_ioctl(struct inode *inode, struct file
*file,
case VME_SET_MASTER:
+ if (image[minor].mmap_count != 0) {
+ pr_warn("Can't adjust mapped window\n");
+ return -EPERM;
+ }
+
copied = copy_from_user(&master, argp, sizeof(master));
if (copied != 0) {
pr_warn("Partial copy from userspace\n");
@@ -564,6 +586,69 @@ vme_user_unlocked_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
return ret;
}
+static void vme_user_vm_open(struct vm_area_struct *vma)
+{
+ struct vme_user_vma_priv *vma_priv = vma->vm_private_data;
+
+ atomic_inc(&vma_priv->refcnt);
+}
+
+static void vme_user_vm_close(struct vm_area_struct *vma)
+{
+ struct vme_user_vma_priv *vma_priv = vma->vm_private_data;
+ unsigned int minor = vma_priv->minor;
+
+ if (!atomic_dec_and_test(&vma_priv->refcnt))
+ return;
+
+ mutex_lock(&image[minor].mutex);
+ image[minor].mmap_count--;
+ mutex_unlock(&image[minor].mutex);
+
+ kfree(vma_priv);
+}
+
+static int vme_user_master_mmap(unsigned int minor, struct vm_area_struct *vma)
+{
+ int err;
+ struct vme_user_vma_priv *vma_priv;
+
+ mutex_lock(&image[minor].mutex);
+
+ err = vme_master_mmap(image[minor].resource, vma);
+ if (err) {
+ mutex_unlock(&image[minor].mutex);
+ return err;
+ }
+
+ vma_priv = kmalloc(sizeof(struct vme_user_vma_priv), GFP_KERNEL);
+ if (vma_priv == NULL) {
+ mutex_unlock(&image[minor].mutex);
+ return -ENOMEM;
+ }
+
+ vma_priv->minor = minor;
+ atomic_set(&vma_priv->refcnt, 1);
+ vma->vm_ops = &vme_user_vm_ops;
+ vma->vm_private_data = vma_priv;
+
+ image[minor].mmap_count++;
+
+ mutex_unlock(&image[minor].mutex);
+
+ return 0;
+}
+
+static int vme_user_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ unsigned int minor = MINOR(file->f_dentry->d_inode->i_rdev);
+
+ if (type[minor] == MASTER_MINOR)
+ return vme_user_master_mmap(minor, vma);
+
+ return -ENODEV;
+}
+
/*
* Unallocate a previously allocated buffer
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 7516030..3dc62b4 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -609,6 +609,32 @@ unsigned int vme_master_rmw(struct vme_resource *resource,
unsigned int mask,
}
EXPORT_SYMBOL(vme_master_rmw);
+int vme_master_mmap(struct vme_resource *resource, struct vm_area_struct *vma)
+{
+ struct vme_master_resource *image;
+ phys_addr_t phys_addr;
+
[PATCH 1/3] staging: vme: use image mutex for ioctl()
This implements more granular locking in vme_user_ioctl() by using separate
locks for each devfs device.
This also provides a synchronization between vme_user_read(), vme_user_write()
and vme_user_ioctl().
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 8b1f533..8731838 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -41,7 +41,6 @@
#include "vme_user.h"
-static DEFINE_MUTEX(vme_user_mutex);
static const char driver_name[] = "vme_user";
static int bus[VME_USER_BUS_MAX];
@@ -555,10 +554,12 @@ static long
vme_user_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
int ret;
+ struct inode *inode = file_inode(file);
+ unsigned int minor = MINOR(inode->i_rdev);
- mutex_lock(&vme_user_mutex);
- ret = vme_user_ioctl(file_inode(file), file, cmd, arg);
- mutex_unlock(&vme_user_mutex);
+ mutex_lock(&image[minor].mutex);
+ ret = vme_user_ioctl(inode, file, cmd, arg);
+ mutex_unlock(&image[minor].mutex);
return ret;
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] vme: Fix integer overflow checking in vme_check_window()
> On 30 Sep 2017, at 07:27, Dan Carpenter wrote:
>
> The controversial part of this patch is that I've changed it so we now
> prevent integer overflows for VME_USER types and before we didn't. I
> view it as kernel-hardening. I looked at a couple places that used
> VME_USER types and they seemed pretty suspicious so I'm pretty sure
> preventing overflows here is a good idea.
>
> The most common problem which this function is for cases like VME_A16
> where we don't put an upper bound on "size" so you could have "size" set
> to U64_MAX and a valid vme_base would overflow the "vme_base + size"
> into the valid range as well.
>
> In the VME_A64 case, the integer overflow checking doesn't work because
> "U64_MAX + 1" has an integer overflow and it's just a complicated way of
> saying zero. That VME_A64 case is sort of interesting as well because
> there is a VME_A64_MAX define which is set to "U64_MAX + 1". The
> compiler will never let anyone use it since it can't be stored in a u64
> variable... With my patch it's now limited to just U64_MAX.
>
> Anyway, I put one integer overflow check at the start of the function
> and deleted all existing checks.
>
> Signed-off-by: Dan Carpenter
Acked-by: Dmitry Kalinkin
>
> diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
> index 6a3ead42aba8..5b4c898d7509 100644
> --- a/drivers/vme/vme.c
> +++ b/drivers/vme/vme.c
> @@ -208,29 +208,27 @@ int vme_check_window(u32 aspace, unsigned long long
> vme_base,
> {
> int retval = 0;
>
> + if (vme_base + size < size)
> + return -EINVAL;
> +
> switch (aspace) {
> case VME_A16:
> - if (((vme_base + size) > VME_A16_MAX) ||
> - (vme_base > VME_A16_MAX))
> + if (vme_base + size > VME_A16_MAX)
> retval = -EFAULT;
> break;
> case VME_A24:
> - if (((vme_base + size) > VME_A24_MAX) ||
> - (vme_base > VME_A24_MAX))
> + if (vme_base + size > VME_A24_MAX)
> retval = -EFAULT;
> break;
> case VME_A32:
> - if (((vme_base + size) > VME_A32_MAX) ||
> - (vme_base > VME_A32_MAX))
> + if (vme_base + size > VME_A32_MAX)
> retval = -EFAULT;
> break;
> case VME_A64:
> - if ((size != 0) && (vme_base > U64_MAX + 1 - size))
> - retval = -EFAULT;
> + /* The VME_A64_MAX limit is actually U64_MAX + 1 */
> break;
> case VME_CRCSR:
> - if (((vme_base + size) > VME_CRCSR_MAX) ||
> - (vme_base > VME_CRCSR_MAX))
> + if (vme_base + size > VME_CRCSR_MAX)
> retval = -EFAULT;
> break;
> case VME_USER1:
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/6] vme: DMA improvements
The first item in this submission documents previously introduced
vme_master_mmap() call. Following, there are three fixes for the tsi148
driver's DMA. There was one bug that rendered it imposible to use DMA
lists with more than one item. The other was related to the place where
dma_map_single was called on the first DMA descriptor in the DMA list. The
last bug was about DMA transfer not stopping at interruption by signal,
which is a possible DoS attack vector. I also made an attempt to fix the
same issue in the ca91cx42 driver. I don't have access to this hardware to
test, so this is based only on my understanding of the datasheet (checked
ca91c042's errata as well).
A new /sys/bus/vme/dma0 device with a new ioctl for making DMA transfers
was introduced in vme_user driver. The logic of the function is similar to
the one found in similar existing drivers.
One question that I had while implementing this feature was whether we
should keep vme_dma_attr objects until vme_dma_list_exec() call. API
doesn't specify this, the existing vme bridge drivers copy all information
from attributes during vme_dma_list_add(). So for simplicity this
implementation frees vme_dma_attr's before vme_dma_list_exec() is done.
A simple test against AVM16 board displays speeds around 45 MiB/s for
A32/D32 reads for both BLT and MBLT (with MBLT being slightly faster).
Dmitry Kalinkin (6):
Documentation: mention vme_master_mmap() in VME API
vme: tsi148: fix DMA lists longer that one item
vme: tsi148: fix first DMA item mapping
vme: stop DMA transfer on interruption
staging: vme_user: refactor llseek to switch(){}
staging: vme_user: provide DMA functionality
Documentation/vme_api.txt | 6 ++
drivers/staging/vme/devices/vme_user.c | 188 +++--
drivers/staging/vme/devices/vme_user.h | 11 ++
drivers/vme/bridges/vme_ca91cx42.c | 15 ++-
drivers/vme/bridges/vme_tsi148.c | 42 +---
5 files changed, 237 insertions(+), 25 deletions(-)
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/6] Documentation: mention vme_master_mmap() in VME API
Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- Documentation/vme_api.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/vme_api.txt b/Documentation/vme_api.txt index ffe6e22..ca5b827 100644 --- a/Documentation/vme_api.txt +++ b/Documentation/vme_api.txt @@ -171,6 +171,12 @@ This functions by reading the offset, applying the mask. If the bits selected in the mask match with the values of the corresponding bits in the compare field, the value of swap is written the specified offset. +Parts of a VME window can be mapped into user space memory using the following +function: + + int vme_master_mmap(struct vme_resource *resource, + struct vm_area_struct *vma) + Slave windows = -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/6] vme: tsi148: fix first DMA item mapping
This moves DMA mapping of the first list element to vme_list_add, the
same place where other elements mappings occur. This prevents extra
mapping or over-unmapping in the cases when vme_list_exec is called more
or less than one time respectively.
Also adds dma_mapping_error check.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/vme/bridges/vme_tsi148.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index 1be4136..03b0755 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -1833,17 +1833,21 @@ static int tsi148_dma_list_add(struct vme_dma_list
*list,
/* Add to list */
list_add_tail(&entry->list, &list->entries);
+ entry->dma_handle = dma_map_single(tsi148_bridge->parent,
+ &entry->descriptor,
+ sizeof(struct tsi148_dma_descriptor), DMA_TO_DEVICE);
+ if (dma_mapping_error(tsi148_bridge->parent, entry->dma_handle)) {
+ dev_err(tsi148_bridge->parent, "DMA mapping error\n");
+ retval = -EINVAL;
+ goto err_dma;
+ }
+
/* Fill out previous descriptors "Next Address" */
if (entry->list.prev != &list->entries) {
- prev = list_entry(entry->list.prev, struct tsi148_dma_entry,
- list);
- /* We need the bus address for the pointer */
- entry->dma_handle = dma_map_single(tsi148_bridge->parent,
- &entry->descriptor,
- sizeof(struct tsi148_dma_descriptor), DMA_TO_DEVICE);
-
reg_split((unsigned long long)entry->dma_handle, &address_high,
&address_low);
+ prev = list_entry(entry->list.prev, struct tsi148_dma_entry,
+ list);
prev->descriptor.dnlau = cpu_to_be32(address_high);
prev->descriptor.dnlal = cpu_to_be32(address_low);
@@ -1851,6 +1855,7 @@ static int tsi148_dma_list_add(struct vme_dma_list *list,
return 0;
+err_dma:
err_dest:
err_source:
err_align:
@@ -1921,10 +1926,6 @@ static int tsi148_dma_list_exec(struct vme_dma_list
*list)
entry = list_first_entry(&list->entries, struct tsi148_dma_entry,
list);
- entry->dma_handle = dma_map_single(tsi148_bridge->parent,
- &entry->descriptor,
- sizeof(struct tsi148_dma_descriptor), DMA_TO_DEVICE);
-
mutex_unlock(&ctrlr->mtx);
reg_split(entry->dma_handle, &bus_addr_high, &bus_addr_low);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/6] staging: vme_user: refactor llseek to switch(){}
This makes vme_user_llseek ignore all minors that don't have llseek
implementation.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 19ba749..da828f4 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -430,15 +430,17 @@ static loff_t vme_user_llseek(struct file *file, loff_t
off, int whence)
size_t image_size;
loff_t res;
- if (minor == CONTROL_MINOR)
- return -EINVAL;
-
- mutex_lock(&image[minor].mutex);
- image_size = vme_get_size(image[minor].resource);
- res = fixed_size_llseek(file, off, whence, image_size);
- mutex_unlock(&image[minor].mutex);
+ switch (type[minor]) {
+ case MASTER_MINOR:
+ case SLAVE_MINOR:
+ mutex_lock(&image[minor].mutex);
+ image_size = vme_get_size(image[minor].resource);
+ res = fixed_size_llseek(file, off, whence, image_size);
+ mutex_unlock(&image[minor].mutex);
+ return res;
+ }
- return res;
+ return -EINVAL;
}
/*
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/6] vme: tsi148: fix DMA lists longer that one item
DMA lists on tsi148 weren't processed further than the first item because of the broken logic. This regression was introduced in: ac1a4f2caf7b071 "Staging: VME: Ensure TSI148 link list descriptors..." Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- drivers/vme/bridges/vme_tsi148.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c index 895c2a3..1be4136 100644 --- a/drivers/vme/bridges/vme_tsi148.c +++ b/drivers/vme/bridges/vme_tsi148.c @@ -1844,8 +1844,8 @@ static int tsi148_dma_list_add(struct vme_dma_list *list, reg_split((unsigned long long)entry->dma_handle, &address_high, &address_low); - entry->descriptor.dnlau = cpu_to_be32(address_high); - entry->descriptor.dnlal = cpu_to_be32(address_low); + prev->descriptor.dnlau = cpu_to_be32(address_high); + prev->descriptor.dnlal = cpu_to_be32(address_low); } -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/6] vme: stop DMA transfer on interruption
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/vme/bridges/vme_ca91cx42.c | 15 +--
drivers/vme/bridges/vme_tsi148.c | 15 +--
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/vme/bridges/vme_ca91cx42.c
b/drivers/vme/bridges/vme_ca91cx42.c
index 18078ec..ffb8e91 100644
--- a/drivers/vme/bridges/vme_ca91cx42.c
+++ b/drivers/vme/bridges/vme_ca91cx42.c
@@ -1192,7 +1192,7 @@ static int ca91cx42_dma_list_exec(struct vme_dma_list
*list)
{
struct vme_dma_resource *ctrlr;
struct ca91cx42_dma_entry *entry;
- int retval = 0;
+ int retval;
dma_addr_t bus_addr;
u32 val;
struct device *dev;
@@ -1245,9 +1245,19 @@ static int ca91cx42_dma_list_exec(struct vme_dma_list
*list)
iowrite32(val, bridge->base + DGCS);
- wait_event_interruptible(bridge->dma_queue,
+ retval = wait_event_interruptible(bridge->dma_queue,
ca91cx42_dma_busy(ctrlr->parent));
+ if (retval) {
+ val = ioread32(bridge->base + DGCS);
+ iowrite32(val | CA91CX42_DGCS_STOP_REQ, bridge->base + DGCS);
+ /* Wait for the operation to abort */
+ wait_event(bridge->dma_queue,
+ ca91cx42_dma_busy(ctrlr->parent));
+ retval = -EINTR;
+ goto exit;
+ }
+
/*
* Read status register, this register is valid until we kick off a
* new transfer.
@@ -1261,6 +1271,7 @@ static int ca91cx42_dma_list_exec(struct vme_dma_list
*list)
val = ioread32(bridge->base + DCTL);
}
+exit:
/* Remove list from running list */
mutex_lock(&ctrlr->mtx);
list_del(&list->list);
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index 03b0755..f6c455e 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -1892,7 +1892,7 @@ static int tsi148_dma_busy(struct vme_bridge
*tsi148_bridge, int channel)
static int tsi148_dma_list_exec(struct vme_dma_list *list)
{
struct vme_dma_resource *ctrlr;
- int channel, retval = 0;
+ int channel, retval;
struct tsi148_dma_entry *entry;
u32 bus_addr_high, bus_addr_low;
u32 val, dctlreg = 0;
@@ -1942,9 +1942,19 @@ static int tsi148_dma_list_exec(struct vme_dma_list
*list)
iowrite32be(dctlreg | TSI148_LCSR_DCTL_DGO, bridge->base +
TSI148_LCSR_DMA[channel] + TSI148_LCSR_OFFSET_DCTL);
- wait_event_interruptible(bridge->dma_queue[channel],
+ retval = wait_event_interruptible(bridge->dma_queue[channel],
tsi148_dma_busy(ctrlr->parent, channel));
+ if (retval) {
+ iowrite32be(dctlreg | TSI148_LCSR_DCTL_ABT, bridge->base +
+ TSI148_LCSR_DMA[channel] + TSI148_LCSR_OFFSET_DCTL);
+ /* Wait for the operation to abort */
+ wait_event(bridge->dma_queue[channel],
+ tsi148_dma_busy(ctrlr->parent, channel));
+ retval = -EINTR;
+ goto exit;
+ }
+
/*
* Read status register, this register is valid until we kick off a
* new transfer.
@@ -1957,6 +1967,7 @@ static int tsi148_dma_list_exec(struct vme_dma_list *list)
retval = -EIO;
}
+exit:
/* Remove list from running list */
mutex_lock(&ctrlr->mtx);
list_del(&list->list);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/6] staging: vme_user: provide DMA functionality
This introduces a new dma device that provides a single ioctl call that
provides DMA read and write functionality to the user space.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 174 -
drivers/staging/vme/devices/vme_user.h | 11 +++
2 files changed, 183 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index da828f4..aca1e0b 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -79,13 +79,14 @@ static unsigned int bus_num;
* We shall support 4 masters and 4 slaves with this driver.
*/
#define VME_MAJOR 221 /* VME Major Device Number */
-#define VME_DEVS 9 /* Number of dev entries */
+#define VME_DEVS 10 /* Number of dev entries */
#define MASTER_MINOR 0
#define MASTER_MAX 3
#define SLAVE_MINOR4
#define SLAVE_MAX 7
#define CONTROL_MINOR 8
+#define DMA_MINOR 9
#define PCI_BUF_SIZE 0x2 /* Size of one slave image buffer */
@@ -125,7 +126,7 @@ static const int type[VME_DEVS] = { MASTER_MINOR,
MASTER_MINOR,
MASTER_MINOR, MASTER_MINOR,
SLAVE_MINOR,SLAVE_MINOR,
SLAVE_MINOR,SLAVE_MINOR,
- CONTROL_MINOR
+ CONTROL_MINOR, DMA_MINOR
};
@@ -443,6 +444,145 @@ static loff_t vme_user_llseek(struct file *file, loff_t
off, int whence)
return -EINVAL;
}
+static int vme_user_sg_to_dma_list(const struct vme_dma_op *dma_op,
+ struct sg_table *sgt, int sg_count, struct vme_dma_list *dma_list)
+{
+ ssize_t pos = 0;
+ struct scatterlist *sg;
+ int i, ret;
+
+ for_each_sg(sgt->sgl, sg, sg_count, i) {
+ struct vme_dma_attr *pci_attr, *vme_attr, *dest, *src;
+ dma_addr_t hw_address = sg_dma_address(sg);
+ unsigned int hw_len = sg_dma_len(sg);
+
+ vme_attr = vme_dma_vme_attribute(dma_op->vme_addr + pos,
+ dma_op->aspace, dma_op->cycle, dma_op->dwidth);
+ if (!vme_attr)
+ return -ENOMEM;
+
+ pci_attr = vme_dma_pci_attribute(hw_address);
+ if (!pci_attr) {
+ vme_dma_free_attribute(vme_attr);
+ return -ENOMEM;
+ }
+
+ if (dma_op->write) {
+ dest = vme_attr;
+ src = pci_attr;
+ } else {
+ dest = pci_attr;
+ src = vme_attr;
+ }
+
+ ret = vme_dma_list_add(dma_list, src, dest, hw_len);
+
+ /*
+* XXX VME API doesn't mention whether we should keep
+* attributes around
+*/
+ vme_dma_free_attribute(vme_attr);
+ vme_dma_free_attribute(pci_attr);
+
+ if (ret)
+ return ret;
+
+ pos += hw_len;
+ }
+
+ WARN_ON(pos != dma_op->count);
+
+ return 0;
+}
+
+static ssize_t vme_user_dma_ioctl(unsigned int minor,
+ const struct vme_dma_op *dma_op)
+{
+ unsigned int offset = offset_in_page(dma_op->buf_vaddr);
+ unsigned long nr_pages;
+ enum dma_data_direction dir;
+ struct vme_dma_list *dma_list;
+ struct sg_table *sgt = NULL;
+ struct page **pages = NULL;
+ long got_pages;
+ int ret, sg_count;
+
+ /* Overflow check for nr_pages */
+ if (dma_op->count > U32_MAX - 2 * PAGE_SIZE)
+ return -EINVAL;
+
+ /* Prevent WARN from dma_map_sg */
+ if (dma_op->count == 0)
+ return 0;
+
+ nr_pages = (offset + dma_op->count + PAGE_SIZE - 1) >> PAGE_SHIFT;
+ dir = dma_op->write ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
+
+ pages = kmalloc_array(nr_pages, sizeof(pages[0]), GFP_KERNEL);
+ if (!pages) {
+ ret = -ENOMEM;
+ goto free;
+ }
+
+ sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+ if (!sgt) {
+ ret = -ENOMEM;
+ goto free;
+ }
+
+ dma_list = vme_new_dma_list(image[minor].resource);
+ if (!dma_list) {
+ ret = -ENOMEM;
+ goto free;
+ }
+
+ got_pages = get_user_pages_fast(dma_op->buf_vaddr, nr_pages,
+ !dma_op->write, pages);
+ if (got_pages != nr_pages) {
+ pr_debug("Not all pages were pinned\n");
+ ret = (got_pages < 0) ? got_pages : -EFAULT;
+ goto release_pages;
+ }
+
+ ret = sg_alloc_table_from_pages(sgt, pages, nr_pages
Re: [PATCH 6/6] staging: vme_user: provide DMA functionality
On Tue, May 19, 2015 at 12:18 PM, Dan Carpenter
wrote:
> On Mon, May 18, 2015 at 09:56:33PM +0300, Dmitry Kalinkin wrote:
>>
>> + for_each_sg(sgt->sgl, sg, sg_count, i) {
>> + struct vme_dma_attr *pci_attr, *vme_attr, *dest, *src;
>> + dma_addr_t hw_address = sg_dma_address(sg);
>> + unsigned int hw_len = sg_dma_len(sg);
>> +
>> + vme_attr = vme_dma_vme_attribute(dma_op->vme_addr + pos,
> ^^
>
> ->vme_addr comes from the user and we don't seem to have done any
> validation that it's correct. This addition can overflow. How is this
> safe? (This is not a rhetorical question, I am a newbie in this).
>
This expression calculates address on the VME bus, where we already have
full access. There shouldn't have security implications. Such transfer will
most likely wrap or cause DMA transfer error (gives EIO).
We could add an additional check:
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index d8aa03b..cb0fc63 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -515,6 +515,11 @@ static ssize_t vme_user_dma_ioctl(unsigned int minor,
if (dma_op->count == 0)
return 0;
+ ret = vme_check_window(dma_op->aspace, dma_op->vme_addr,
+ dma_op->count);
+ if (ret)
+ return ret;
+
nr_pages = (offset + dma_op->count + PAGE_SIZE - 1) >> PAGE_SHIFT;
dir = dma_op->write ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 6bab2c4..50cabc3 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -177,7 +177,7 @@ size_t vme_get_size(struct vme_resource *resource)
}
EXPORT_SYMBOL(vme_get_size);
-static int vme_check_window(u32 aspace, unsigned long long vme_base,
+int vme_check_window(u32 aspace, unsigned long long vme_base,
unsigned long long size)
{
int retval = 0;
@@ -199,10 +199,8 @@ static int vme_check_window(u32 aspace, unsigned long long
vme_base,
retval = -EFAULT;
break;
case VME_A64:
- /*
-* Any value held in an unsigned long long can be used as the
-* base
-*/
+ if (vme_base > VME_A64_MAX - size)
+ retval = -EFAULT;
break;
case VME_CRCSR:
if (((vme_base + size) > VME_CRCSR_MAX) ||
@@ -223,6 +221,7 @@ static int vme_check_window(u32 aspace, unsigned long long
vme_base,
return retval;
}
+EXPORT_SYMBOL(vme_check_window);
/*
* Request a slave image with specific attributes, return some unique
diff --git a/include/linux/vme.h b/include/linux/vme.h
index 79242e9..cfff272 100644
--- a/include/linux/vme.h
+++ b/include/linux/vme.h
@@ -120,6 +120,8 @@ void vme_free_consistent(struct vme_resource *, size_t,
void *,
dma_addr_t);
size_t vme_get_size(struct vme_resource *);
+int vme_check_window(u32 aspace, unsigned long long vme_base,
+ unsigned long long size);
struct vme_resource *vme_slave_request(struct vme_dev *, u32, u32);
int vme_slave_set(struct vme_resource *, int, unsigned long long,
>> + nr_pages = (offset + dma_op->count + PAGE_SIZE - 1) >> PAGE_SHIFT;
>> + dir = dma_op->write ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
>> +
>> + pages = kmalloc_array(nr_pages, sizeof(pages[0]), GFP_KERNEL);
>
> This lets the user try allocate huge ammounts of RAM. Is there no
> reasonable max size we can use?
>
We could limit operation size by 64 MiB and do an partial read for any bigger
request.
> This file is all indented poorly, but these patches adds a bunch of new
> ones so they make a bad situation worse.
>
>got_pages = get_user_pages_fast(dma_op->buf_vaddr, nr_pages,
>!dma_op->write, pages);
>
> You sometimes might have to use spaces to make things align correctly.
>
>got_pages = some_fake_name(dma_op->buf_vaddr, nr_pages,
> !dma_op->write, pages);
>
> [tab][tab][tab][tab][space][space][space][space]!dma_op->write, pages);
Will fix this.
Thank you.
Cheers,
Dmitry
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 00/16] vme DMA and user space driver improvements
The first item in this submission documents previously introduced
vme_master_mmap() call. Following, there are three fixes for the tsi148
driver's DMA. There was one bug that rendered it imposible to use DMA
lists with more than one item. The other was related to the place where
dma_map_single was called on the first DMA descriptor in the DMA list. The
last bug was about DMA transfer not stopping at interruption by signal,
which is a possible DoS attack vector. I also made an attempt to fix the
same issue in the ca91cx42 driver. I don't have access to this hardware to
test, so this is based only on my understanding of the datasheet (checked
ca91c042's errata as well).
A new /sys/bus/vme/dma0 device with a new ioctl for making DMA transfers
was introduced in vme_user driver. The logic of the function is similar to
the one found in existing drivers.
One question that I had while implementing this feature was whether we
should keep vme_dma_attr objects until vme_dma_list_exec() call. API
doesn't specify this, the existing vme bridge drivers copy all information
from attributes during vme_dma_list_add(). So for simplicity this
implementation frees vme_dma_attr's before vme_dma_list_exec() is done.
A simple test against AVM16 board displays speeds around 45 MiB/s for
A32/D32 reads for both BLT and MBLT (with MBLT being slightly faster).
Changes in v2 [patches 1-6, now 1-5 and 8]:
* vme_addr check for vme_user DMA
* limit on DMA operation length in vme_user
* reorder dma_op ioctl struct to omit __packed attribute
* change dma_op->write into dma_op->dir
* use vme_check_window assure DMA operation correctness
New changes include vme_user code cleanup, a couple of ca91cx42 fixes
(again, tested for compilation only).
I also propose a change to export some of VME subsytem related constants
to the user space. These can be useful if vme_user is to go into the kernel.
Also, email
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2012-July/029084.html
mentions that we probably can now get rid of this comment:
> /* XXX We do not want to push aspace, cycle and width
> * to userspace as they are
> */
Dmitry Kalinkin (16):
Documentation: mention vme_master_mmap() in VME API
vme: tsi148: fix DMA lists longer that one item
vme: tsi148: fix first DMA item mapping
vme: stop DMA transfer on interruption
staging: vme_user: refactor llseek to switch(){}
vme: check for A64 overflow in vme_check_window()
vme: export vme_check_window()
staging: vme_user: provide DMA functionality
vme: ca91cx42: return error code on DMA error
vme: ca91cx42: fix LM_CTL address mask
staging: vme_user: remove unused counters
staging: vme_user: remove forward declarations
staging: vme_user: remove open/release
staging: vme_user: remove buf_unalloc helper
vme: tsi148: depend on HAS_DMA for Kconfig
vme: provide uapi header
Documentation/vme_api.txt | 6 +
drivers/staging/vme/devices/vme_user.c | 454 +++--
drivers/staging/vme/devices/vme_user.h | 11 +
drivers/vme/bridges/Kconfig| 2 +-
drivers/vme/bridges/vme_ca91cx42.c | 18 +-
drivers/vme/bridges/vme_ca91cx42.h | 2 +-
drivers/vme/bridges/vme_tsi148.c | 42 +--
drivers/vme/vme.c | 11 +-
include/linux/vme.h| 56 +---
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/vme.h | 56
11 files changed, 394 insertions(+), 265 deletions(-)
create mode 100644 include/uapi/linux/vme.h
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 01/16] Documentation: mention vme_master_mmap() in VME API
Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- Documentation/vme_api.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/vme_api.txt b/Documentation/vme_api.txt index ffe6e22..ca5b827 100644 --- a/Documentation/vme_api.txt +++ b/Documentation/vme_api.txt @@ -171,6 +171,12 @@ This functions by reading the offset, applying the mask. If the bits selected in the mask match with the values of the corresponding bits in the compare field, the value of swap is written the specified offset. +Parts of a VME window can be mapped into user space memory using the following +function: + + int vme_master_mmap(struct vme_resource *resource, + struct vm_area_struct *vma) + Slave windows = -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 06/16] vme: check for A64 overflow in vme_check_window()
Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- drivers/vme/vme.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c index 6bab2c4..1b78d27 100644 --- a/drivers/vme/vme.c +++ b/drivers/vme/vme.c @@ -199,10 +199,8 @@ static int vme_check_window(u32 aspace, unsigned long long vme_base, retval = -EFAULT; break; case VME_A64: - /* -* Any value held in an unsigned long long can be used as the -* base -*/ + if ((size != 0) && (vme_base > U64_MAX + 1 - size)) + retval = -EFAULT; break; case VME_CRCSR: if (((vme_base + size) > VME_CRCSR_MAX) || -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 07/16] vme: export vme_check_window()
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/vme/vme.c | 5 +++--
include/linux/vme.h | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 1b78d27..5670891 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -177,8 +177,8 @@ size_t vme_get_size(struct vme_resource *resource)
}
EXPORT_SYMBOL(vme_get_size);
-static int vme_check_window(u32 aspace, unsigned long long vme_base,
- unsigned long long size)
+int vme_check_window(u32 aspace, unsigned long long vme_base,
+unsigned long long size)
{
int retval = 0;
@@ -221,6 +221,7 @@ static int vme_check_window(u32 aspace, unsigned long long
vme_base,
return retval;
}
+EXPORT_SYMBOL(vme_check_window);
/*
* Request a slave image with specific attributes, return some unique
diff --git a/include/linux/vme.h b/include/linux/vme.h
index 79242e9..c013135 100644
--- a/include/linux/vme.h
+++ b/include/linux/vme.h
@@ -120,6 +120,8 @@ void vme_free_consistent(struct vme_resource *, size_t,
void *,
dma_addr_t);
size_t vme_get_size(struct vme_resource *);
+int vme_check_window(u32 aspace, unsigned long long vme_base,
+unsigned long long size);
struct vme_resource *vme_slave_request(struct vme_dev *, u32, u32);
int vme_slave_set(struct vme_resource *, int, unsigned long long,
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 11/16] staging: vme_user: remove unused counters
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 31 ---
1 file changed, 31 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index ca7ea6b..ed374d9 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -107,18 +107,6 @@ struct image_desc {
};
static struct image_desc image[VME_DEVS];
-struct driver_stats {
- unsigned long reads;
- unsigned long writes;
- unsigned long ioctls;
- unsigned long irqs;
- unsigned long berrs;
- unsigned long dmaerrors;
- unsigned long timeouts;
- unsigned long external;
-};
-static struct driver_stats statistics;
-
static struct cdev *vme_user_cdev; /* Character device */
static struct class *vme_user_sysfs_class; /* Sysfs class */
static struct vme_dev *vme_user_bridge;/* Pointer to user
device */
@@ -170,20 +158,6 @@ static const struct vm_operations_struct vme_user_vm_ops =
{
};
-/*
- * Reset all the statistic counters
- */
-static void reset_counters(void)
-{
- statistics.reads = 0;
- statistics.writes = 0;
- statistics.ioctls = 0;
- statistics.irqs = 0;
- statistics.berrs = 0;
- statistics.dmaerrors = 0;
- statistics.timeouts = 0;
-}
-
static int vme_user_open(struct inode *inode, struct file *file)
{
int err;
@@ -631,8 +605,6 @@ static int vme_user_ioctl(struct inode *inode, struct file
*file,
dma_addr_t pci_addr;
void __user *argp = (void __user *)arg;
- statistics.ioctls++;
-
switch (type[minor]) {
case CONTROL_MINOR:
switch (cmd) {
@@ -944,9 +916,6 @@ static int vme_user_probe(struct vme_dev *vdev)
image[i].users = 0;
}
- /* Initialise statistics counters */
- reset_counters();
-
/* Assign major and minor numbers for the driver */
err = register_chrdev_region(MKDEV(VME_MAJOR, 0), VME_DEVS,
driver_name);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 08/16] staging: vme_user: provide DMA functionality
This introduces a new dma device that provides a single ioctl call that
provides DMA read and write functionality to the user space.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 201 -
drivers/staging/vme/devices/vme_user.h | 11 ++
2 files changed, 209 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index da828f4..ca7ea6b 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -79,15 +79,18 @@ static unsigned int bus_num;
* We shall support 4 masters and 4 slaves with this driver.
*/
#define VME_MAJOR 221 /* VME Major Device Number */
-#define VME_DEVS 9 /* Number of dev entries */
+#define VME_DEVS 10 /* Number of dev entries */
#define MASTER_MINOR 0
#define MASTER_MAX 3
#define SLAVE_MINOR4
#define SLAVE_MAX 7
#define CONTROL_MINOR 8
+#define DMA_MINOR 9
-#define PCI_BUF_SIZE 0x2 /* Size of one slave image buffer */
+#define PCI_BUF_SIZE 0x2 /* Size of one slave image buffer */
+
+#define VME_MAX_DMA_LEN0x400 /* Maximal DMA transfer length
*/
/*
* Structure to handle image related parameters.
@@ -125,7 +128,7 @@ static const int type[VME_DEVS] = { MASTER_MINOR,
MASTER_MINOR,
MASTER_MINOR, MASTER_MINOR,
SLAVE_MINOR,SLAVE_MINOR,
SLAVE_MINOR,SLAVE_MINOR,
- CONTROL_MINOR
+ CONTROL_MINOR, DMA_MINOR
};
@@ -443,6 +446,168 @@ static loff_t vme_user_llseek(struct file *file, loff_t
off, int whence)
return -EINVAL;
}
+static int vme_user_sg_to_dma_list(const struct vme_dma_op *dma_op,
+ struct sg_table *sgt,
+ int sg_count, struct vme_dma_list *dma_list)
+{
+ ssize_t pos = 0;
+ struct scatterlist *sg;
+ int i, ret;
+
+ for_each_sg(sgt->sgl, sg, sg_count, i) {
+ struct vme_dma_attr *pci_attr, *vme_attr, *src, *dest;
+ dma_addr_t hw_address = sg_dma_address(sg);
+ unsigned int hw_len = sg_dma_len(sg);
+
+ vme_attr = vme_dma_vme_attribute(dma_op->vme_addr + pos,
+dma_op->aspace,
+dma_op->cycle,
+dma_op->dwidth);
+ if (!vme_attr)
+ return -ENOMEM;
+
+ pci_attr = vme_dma_pci_attribute(hw_address);
+ if (!pci_attr) {
+ vme_dma_free_attribute(vme_attr);
+ return -ENOMEM;
+ }
+
+ switch(dma_op->dir) {
+ case VME_DMA_MEM_TO_VME:
+ src = pci_attr;
+ dest = vme_attr;
+ break;
+ case VME_DMA_VME_TO_MEM:
+ src = vme_attr;
+ dest = pci_attr;
+ break;
+ }
+
+ ret = vme_dma_list_add(dma_list, src, dest, hw_len);
+
+ /*
+* XXX VME API doesn't mention whether we should keep
+* attributes around
+*/
+ vme_dma_free_attribute(vme_attr);
+ vme_dma_free_attribute(pci_attr);
+
+ if (ret)
+ return ret;
+
+ pos += hw_len;
+ }
+
+ return 0;
+}
+
+static enum dma_data_direction vme_dir_to_dma_dir(unsigned vme_dir)
+{
+ switch(vme_dir) {
+ case VME_DMA_VME_TO_MEM:
+ return DMA_FROM_DEVICE;
+ case VME_DMA_MEM_TO_VME:
+ return DMA_TO_DEVICE;
+ }
+
+ return DMA_NONE;
+}
+
+static ssize_t vme_user_dma_ioctl(unsigned int minor,
+ const struct vme_dma_op *dma_op)
+{
+ unsigned int offset = offset_in_page(dma_op->buf_vaddr);
+ unsigned long nr_pages;
+ enum dma_data_direction dir;
+ struct vme_dma_list *dma_list;
+ struct sg_table *sgt = NULL;
+ struct page **pages = NULL;
+ long got_pages;
+ ssize_t count;
+ int retval, sg_count;
+
+ /* Prevent WARN from dma_map_sg */
+ if (dma_op->count == 0)
+ return 0;
+
+ /*
+* This is a voluntary limit to prevent huge allocation for pages
+* array. VME_MAX_DMA_LEN is not a fundamental VME constraint.
+*/
+ count = min_t(size_t, dma_op->count, VME_MAX_DMA_LEN);
+ nr_pages = (offset + count + PAGE_SIZE - 1) >> PAGE_SHIFT;
+
+ dir = vme_dir_to
[PATCHv2 03/16] vme: tsi148: fix first DMA item mapping
This moves DMA mapping of the first list element to vme_list_add, the
same place where other elements mappings occur. This prevents extra
mapping or over-unmapping in the cases when vme_list_exec is called more
or less than one time respectively.
Also adds dma_mapping_error check.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/vme/bridges/vme_tsi148.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index 1be4136..6562348 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -1833,17 +1833,21 @@ static int tsi148_dma_list_add(struct vme_dma_list
*list,
/* Add to list */
list_add_tail(&entry->list, &list->entries);
+ entry->dma_handle = dma_map_single(tsi148_bridge->parent,
+ &entry->descriptor,
+ sizeof(struct tsi148_dma_descriptor), DMA_TO_DEVICE);
+ if (dma_mapping_error(tsi148_bridge->parent, entry->dma_handle)) {
+ dev_err(tsi148_bridge->parent, "DMA mapping error\n");
+ retval = -EINVAL;
+ goto err_dma;
+ }
+
/* Fill out previous descriptors "Next Address" */
if (entry->list.prev != &list->entries) {
- prev = list_entry(entry->list.prev, struct tsi148_dma_entry,
- list);
- /* We need the bus address for the pointer */
- entry->dma_handle = dma_map_single(tsi148_bridge->parent,
- &entry->descriptor,
- sizeof(struct tsi148_dma_descriptor), DMA_TO_DEVICE);
-
reg_split((unsigned long long)entry->dma_handle, &address_high,
&address_low);
+ prev = list_entry(entry->list.prev, struct tsi148_dma_entry,
+ list);
prev->descriptor.dnlau = cpu_to_be32(address_high);
prev->descriptor.dnlal = cpu_to_be32(address_low);
@@ -1851,6 +1855,7 @@ static int tsi148_dma_list_add(struct vme_dma_list *list,
return 0;
+err_dma:
err_dest:
err_source:
err_align:
@@ -1921,10 +1926,6 @@ static int tsi148_dma_list_exec(struct vme_dma_list
*list)
entry = list_first_entry(&list->entries, struct tsi148_dma_entry,
list);
- entry->dma_handle = dma_map_single(tsi148_bridge->parent,
- &entry->descriptor,
- sizeof(struct tsi148_dma_descriptor), DMA_TO_DEVICE);
-
mutex_unlock(&ctrlr->mtx);
reg_split(entry->dma_handle, &bus_addr_high, &bus_addr_low);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 09/16] vme: ca91cx42: return error code on DMA error
Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- drivers/vme/bridges/vme_ca91cx42.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/vme/bridges/vme_ca91cx42.c b/drivers/vme/bridges/vme_ca91cx42.c index e9bd657..f692efc 100644 --- a/drivers/vme/bridges/vme_ca91cx42.c +++ b/drivers/vme/bridges/vme_ca91cx42.c @@ -1269,6 +1269,7 @@ static int ca91cx42_dma_list_exec(struct vme_dma_list *list) dev_err(dev, "ca91c042: DMA Error. DGCS=%08X\n", val); val = ioread32(bridge->base + DCTL); + retval = -EIO; } exit: -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 04/16] vme: stop DMA transfer on interruption
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/vme/bridges/vme_ca91cx42.c | 17 ++---
drivers/vme/bridges/vme_tsi148.c | 15 +--
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/vme/bridges/vme_ca91cx42.c
b/drivers/vme/bridges/vme_ca91cx42.c
index 18078ec..e9bd657 100644
--- a/drivers/vme/bridges/vme_ca91cx42.c
+++ b/drivers/vme/bridges/vme_ca91cx42.c
@@ -1192,7 +1192,7 @@ static int ca91cx42_dma_list_exec(struct vme_dma_list
*list)
{
struct vme_dma_resource *ctrlr;
struct ca91cx42_dma_entry *entry;
- int retval = 0;
+ int retval;
dma_addr_t bus_addr;
u32 val;
struct device *dev;
@@ -1245,8 +1245,18 @@ static int ca91cx42_dma_list_exec(struct vme_dma_list
*list)
iowrite32(val, bridge->base + DGCS);
- wait_event_interruptible(bridge->dma_queue,
- ca91cx42_dma_busy(ctrlr->parent));
+ retval = wait_event_interruptible(bridge->dma_queue,
+ ca91cx42_dma_busy(ctrlr->parent));
+
+ if (retval) {
+ val = ioread32(bridge->base + DGCS);
+ iowrite32(val | CA91CX42_DGCS_STOP_REQ, bridge->base + DGCS);
+ /* Wait for the operation to abort */
+ wait_event(bridge->dma_queue,
+ ca91cx42_dma_busy(ctrlr->parent));
+ retval = -EINTR;
+ goto exit;
+ }
/*
* Read status register, this register is valid until we kick off a
@@ -1261,6 +1271,7 @@ static int ca91cx42_dma_list_exec(struct vme_dma_list
*list)
val = ioread32(bridge->base + DCTL);
}
+exit:
/* Remove list from running list */
mutex_lock(&ctrlr->mtx);
list_del(&list->list);
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index 6562348..fb1e7ad 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -1892,7 +1892,7 @@ static int tsi148_dma_busy(struct vme_bridge
*tsi148_bridge, int channel)
static int tsi148_dma_list_exec(struct vme_dma_list *list)
{
struct vme_dma_resource *ctrlr;
- int channel, retval = 0;
+ int channel, retval;
struct tsi148_dma_entry *entry;
u32 bus_addr_high, bus_addr_low;
u32 val, dctlreg = 0;
@@ -1942,9 +1942,19 @@ static int tsi148_dma_list_exec(struct vme_dma_list
*list)
iowrite32be(dctlreg | TSI148_LCSR_DCTL_DGO, bridge->base +
TSI148_LCSR_DMA[channel] + TSI148_LCSR_OFFSET_DCTL);
- wait_event_interruptible(bridge->dma_queue[channel],
+ retval = wait_event_interruptible(bridge->dma_queue[channel],
tsi148_dma_busy(ctrlr->parent, channel));
+ if (retval) {
+ iowrite32be(dctlreg | TSI148_LCSR_DCTL_ABT, bridge->base +
+ TSI148_LCSR_DMA[channel] + TSI148_LCSR_OFFSET_DCTL);
+ /* Wait for the operation to abort */
+ wait_event(bridge->dma_queue[channel],
+ tsi148_dma_busy(ctrlr->parent, channel));
+ retval = -EINTR;
+ goto exit;
+ }
+
/*
* Read status register, this register is valid until we kick off a
* new transfer.
@@ -1957,6 +1967,7 @@ static int tsi148_dma_list_exec(struct vme_dma_list *list)
retval = -EIO;
}
+exit:
/* Remove list from running list */
mutex_lock(&ctrlr->mtx);
list_del(&list->list);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 05/16] staging: vme_user: refactor llseek to switch(){}
This makes vme_user_llseek ignore all minors that don't have llseek
implementation.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 19ba749..da828f4 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -430,15 +430,17 @@ static loff_t vme_user_llseek(struct file *file, loff_t
off, int whence)
size_t image_size;
loff_t res;
- if (minor == CONTROL_MINOR)
- return -EINVAL;
-
- mutex_lock(&image[minor].mutex);
- image_size = vme_get_size(image[minor].resource);
- res = fixed_size_llseek(file, off, whence, image_size);
- mutex_unlock(&image[minor].mutex);
+ switch (type[minor]) {
+ case MASTER_MINOR:
+ case SLAVE_MINOR:
+ mutex_lock(&image[minor].mutex);
+ image_size = vme_get_size(image[minor].resource);
+ res = fixed_size_llseek(file, off, whence, image_size);
+ mutex_unlock(&image[minor].mutex);
+ return res;
+ }
- return res;
+ return -EINVAL;
}
/*
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 10/16] vme: ca91cx42: fix LM_CTL address mask
Universe II datasheet defines following address space values
for LM_CTL[16:18]
000=A16
001=A24
010=A32
011,100,101=Reserved
110=User1
111=User2
Mask 5<<16 is not the right one for matching [16:18], instead we should
use 7<<16.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
Reported-by: Dan Carpenter
---
drivers/vme/bridges/vme_ca91cx42.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vme/bridges/vme_ca91cx42.h
b/drivers/vme/bridges/vme_ca91cx42.h
index d46b12d..d54119e 100644
--- a/drivers/vme/bridges/vme_ca91cx42.h
+++ b/drivers/vme/bridges/vme_ca91cx42.h
@@ -547,7 +547,7 @@ static const int CA91CX42_LINT_LM[] = { CA91CX42_LINT_LM0,
CA91CX42_LINT_LM1,
#define CA91CX42_LM_CTL_DATA (1<<22)
#define CA91CX42_LM_CTL_SUPR (1<<21)
#define CA91CX42_LM_CTL_NPRIV (1<<20)
-#define CA91CX42_LM_CTL_AS_M (5<<16)
+#define CA91CX42_LM_CTL_AS_M (7<<16)
#define CA91CX42_LM_CTL_AS_A16 0
#define CA91CX42_LM_CTL_AS_A24 (1<<16)
#define CA91CX42_LM_CTL_AS_A32 (1<<17)
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 02/16] vme: tsi148: fix DMA lists longer that one item
DMA lists on tsi148 weren't processed further than the first item because of the broken logic. This regression was introduced in: ac1a4f2caf7b071 "Staging: VME: Ensure TSI148 link list descriptors..." Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- drivers/vme/bridges/vme_tsi148.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c index 895c2a3..1be4136 100644 --- a/drivers/vme/bridges/vme_tsi148.c +++ b/drivers/vme/bridges/vme_tsi148.c @@ -1844,8 +1844,8 @@ static int tsi148_dma_list_add(struct vme_dma_list *list, reg_split((unsigned long long)entry->dma_handle, &address_high, &address_low); - entry->descriptor.dnlau = cpu_to_be32(address_high); - entry->descriptor.dnlal = cpu_to_be32(address_low); + prev->descriptor.dnlau = cpu_to_be32(address_high); + prev->descriptor.dnlal = cpu_to_be32(address_low); } -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 15/16] vme: tsi148: depend on HAS_DMA for Kconfig
Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- drivers/vme/bridges/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vme/bridges/Kconfig b/drivers/vme/bridges/Kconfig index 9331064..f6d8545 100644 --- a/drivers/vme/bridges/Kconfig +++ b/drivers/vme/bridges/Kconfig @@ -9,7 +9,7 @@ config VME_CA91CX42 config VME_TSI148 tristate "Tempe" - depends on VIRT_TO_BUS + depends on HAS_DMA help If you say Y here you get support for the Tundra TSI148 VME bridge chip. -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 14/16] staging: vme_user: remove buf_unalloc helper
buf_unalloc is essentially a vme_free_consistent:
1) image[i].kern_buf is never NULL in buf_alloc call
2) kern_buf, pci_buf and size_buf get zeroed in vme_user_probe anyway
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 31 ---
1 file changed, 4 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index ce4aacf..6b97bd2 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -742,31 +742,6 @@ static const struct file_operations vme_user_fops = {
.mmap = vme_user_mmap,
};
-/*
- * Unallocate a previously allocated buffer
- */
-static void buf_unalloc(int num)
-{
- if (image[num].kern_buf) {
-#ifdef VME_DEBUG
- pr_debug("UniverseII:Releasing buffer at %p\n",
-image[num].pci_buf);
-#endif
-
- vme_free_consistent(image[num].resource, image[num].size_buf,
- image[num].kern_buf, image[num].pci_buf);
-
- image[num].kern_buf = NULL;
- image[num].pci_buf = 0;
- image[num].size_buf = 0;
-
-#ifdef VME_DEBUG
- } else {
- pr_debug("UniverseII: Buffer not allocated\n");
-#endif
- }
-}
-
static int vme_user_match(struct vme_dev *vdev)
{
int i;
@@ -958,7 +933,8 @@ err_master:
err_slave:
while (i > SLAVE_MINOR) {
i--;
- buf_unalloc(i);
+ vme_free_consistent(image[i].resource, image[i].size_buf,
+ image[i].kern_buf, image[i].pci_buf);
vme_slave_free(image[i].resource);
}
err_class:
@@ -990,7 +966,8 @@ static int vme_user_remove(struct vme_dev *dev)
for (i = SLAVE_MINOR; i < (SLAVE_MAX + 1); i++) {
vme_slave_set(image[i].resource, 0, 0, 0, 0, VME_A32, 0);
- buf_unalloc(i);
+ vme_free_consistent(image[i].resource, image[i].size_buf,
+ image[i].kern_buf, image[i].pci_buf);
vme_slave_free(image[i].resource);
}
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 12/16] staging: vme_user: remove forward declarations
Reorder code so that forward declarations are not needed.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 139 ++---
1 file changed, 60 insertions(+), 79 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index ed374d9..2493847 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -119,44 +119,11 @@ static const int type[VME_DEVS] = { MASTER_MINOR,
MASTER_MINOR,
CONTROL_MINOR, DMA_MINOR
};
-
-static int vme_user_open(struct inode *, struct file *);
-static int vme_user_release(struct inode *, struct file *);
-static ssize_t vme_user_read(struct file *, char __user *, size_t, loff_t *);
-static ssize_t vme_user_write(struct file *, const char __user *, size_t,
- loff_t *);
-static loff_t vme_user_llseek(struct file *, loff_t, int);
-static long vme_user_unlocked_ioctl(struct file *, unsigned int, unsigned
long);
-static int vme_user_mmap(struct file *file, struct vm_area_struct *vma);
-
-static void vme_user_vm_open(struct vm_area_struct *vma);
-static void vme_user_vm_close(struct vm_area_struct *vma);
-
-static int vme_user_match(struct vme_dev *);
-static int vme_user_probe(struct vme_dev *);
-static int vme_user_remove(struct vme_dev *);
-
-static const struct file_operations vme_user_fops = {
- .open = vme_user_open,
- .release = vme_user_release,
- .read = vme_user_read,
- .write = vme_user_write,
- .llseek = vme_user_llseek,
- .unlocked_ioctl = vme_user_unlocked_ioctl,
- .compat_ioctl = vme_user_unlocked_ioctl,
- .mmap = vme_user_mmap,
-};
-
struct vme_user_vma_priv {
unsigned int minor;
atomic_t refcnt;
};
-static const struct vm_operations_struct vme_user_vm_ops = {
- .open = vme_user_vm_open,
- .close = vme_user_vm_close,
-};
-
static int vme_user_open(struct inode *inode, struct file *file)
{
@@ -761,6 +728,11 @@ static void vme_user_vm_close(struct vm_area_struct *vma)
kfree(vma_priv);
}
+static const struct vm_operations_struct vme_user_vm_ops = {
+ .open = vme_user_vm_open,
+ .close = vme_user_vm_close,
+};
+
static int vme_user_master_mmap(unsigned int minor, struct vm_area_struct *vma)
{
int err;
@@ -802,6 +774,16 @@ static int vme_user_mmap(struct file *file, struct
vm_area_struct *vma)
return -ENODEV;
}
+static const struct file_operations vme_user_fops = {
+ .open = vme_user_open,
+ .release = vme_user_release,
+ .read = vme_user_read,
+ .write = vme_user_write,
+ .llseek = vme_user_llseek,
+ .unlocked_ioctl = vme_user_unlocked_ioctl,
+ .compat_ioctl = vme_user_unlocked_ioctl,
+ .mmap = vme_user_mmap,
+};
/*
* Unallocate a previously allocated buffer
@@ -828,52 +810,6 @@ static void buf_unalloc(int num)
}
}
-static struct vme_driver vme_user_driver = {
- .name = driver_name,
- .match = vme_user_match,
- .probe = vme_user_probe,
- .remove = vme_user_remove,
-};
-
-
-static int __init vme_user_init(void)
-{
- int retval = 0;
-
- pr_info("VME User Space Access Driver\n");
-
- if (bus_num == 0) {
- pr_err("No cards, skipping registration\n");
- retval = -ENODEV;
- goto err_nocard;
- }
-
- /* Let's start by supporting one bus, we can support more than one
-* in future revisions if that ever becomes necessary.
-*/
- if (bus_num > VME_USER_BUS_MAX) {
- pr_err("Driver only able to handle %d buses\n",
- VME_USER_BUS_MAX);
- bus_num = VME_USER_BUS_MAX;
- }
-
- /*
-* Here we just register the maximum number of devices we can and
-* leave vme_user_match() to allow only 1 to go through to probe().
-* This way, if we later want to allow multiple user access devices,
-* we just change the code in vme_user_match().
-*/
- retval = vme_register_driver(&vme_user_driver, VME_MAX_SLOTS);
- if (retval != 0)
- goto err_reg;
-
- return retval;
-
-err_reg:
-err_nocard:
- return retval;
-}
-
static int vme_user_match(struct vme_dev *vdev)
{
int i;
@@ -,6 +1047,51 @@ static int vme_user_remove(struct vme_dev *dev)
return 0;
}
+static struct vme_driver vme_user_driver = {
+ .name = driver_name,
+ .match = vme_user_match,
+ .probe = vme_user_probe,
+ .remove = vme_user_remove,
+};
+
+static int __init vme_user_init(void)
+{
+ int retval = 0;
+
+ pr_info("VME User Space Access Driver\n");
+
+ if (bus_num == 0) {
+ pr_err("No cards, skipping registration\n");
+
[PATCHv2 16/16] vme: provide uapi header
This separates VME related constants that are a part of both kernel and
user space API into a common uapi header.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
include/linux/vme.h | 54 ++---
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/vme.h | 56 +++
3 files changed, 59 insertions(+), 52 deletions(-)
create mode 100644 include/uapi/linux/vme.h
diff --git a/include/linux/vme.h b/include/linux/vme.h
index c013135..ecfd389 100644
--- a/include/linux/vme.h
+++ b/include/linux/vme.h
@@ -1,6 +1,8 @@
#ifndef _VME_H_
#define _VME_H_
+#include
+
/* Resource Type */
enum vme_resource_type {
VME_MASTER,
@@ -9,47 +11,6 @@ enum vme_resource_type {
VME_LM
};
-/* VME Address Spaces */
-#define VME_A160x1
-#define VME_A240x2
-#defineVME_A32 0x4
-#define VME_A640x8
-#define VME_CRCSR 0x10
-#define VME_USER1 0x20
-#define VME_USER2 0x40
-#define VME_USER3 0x80
-#define VME_USER4 0x100
-
-#define VME_A16_MAX0x1ULL
-#define VME_A24_MAX0x100ULL
-#define VME_A32_MAX0x1ULL
-#define VME_A64_MAX0x1ULL
-#define VME_CRCSR_MAX 0x100ULL
-
-
-/* VME Cycle Types */
-#define VME_SCT0x1
-#define VME_BLT0x2
-#define VME_MBLT 0x4
-#define VME_2eVME 0x8
-#define VME_2eSST 0x10
-#define VME_2eSSTB 0x20
-
-#define VME_2eSST160 0x100
-#define VME_2eSST267 0x200
-#define VME_2eSST320 0x400
-
-#defineVME_SUPER 0x1000
-#defineVME_USER0x2000
-#defineVME_PROG0x4000
-#defineVME_DATA0x8000
-
-/* VME Data Widths */
-#define VME_D8 0x1
-#define VME_D160x2
-#define VME_D320x4
-#define VME_D640x8
-
/* Arbitration Scheduling Modes */
#define VME_R_ROBIN_MODE 0x1
#define VME_PRIORITY_MODE 0x2
@@ -58,17 +19,6 @@ enum vme_resource_type {
#define VME_DMA_PCI(1<<1)
#define VME_DMA_VME(1<<2)
-#define VME_DMA_PATTERN_BYTE (1<<0)
-#define VME_DMA_PATTERN_WORD (1<<1)
-#define VME_DMA_PATTERN_INCREMENT (1<<2)
-
-#define VME_DMA_VME_TO_MEM (1<<0)
-#define VME_DMA_MEM_TO_VME (1<<1)
-#define VME_DMA_VME_TO_VME (1<<2)
-#define VME_DMA_MEM_TO_MEM (1<<3)
-#define VME_DMA_PATTERN_TO_VME (1<<4)
-#define VME_DMA_PATTERN_TO_MEM (1<<5)
-
struct vme_dma_attr {
u32 type;
void *private;
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 1a0006a..ad25e3f 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -439,6 +439,7 @@ header-y += virtio_rng.h
header-y += virtio_scsi.h
header-y += virtio_types.h
header-y += vm_sockets.h
+header-y += vme.h
header-y += vt.h
header-y += wait.h
header-y += wanrouter.h
diff --git a/include/uapi/linux/vme.h b/include/uapi/linux/vme.h
new file mode 100644
index 000..3d49a56
--- /dev/null
+++ b/include/uapi/linux/vme.h
@@ -0,0 +1,56 @@
+#ifndef _UAPI_LINUX_VME
+#define _UAPI_LINUX_VME
+
+/* VME Address Spaces */
+#define VME_A160x1
+#define VME_A240x2
+#define VME_A320x4
+#define VME_A640x8
+#define VME_CRCSR 0x10
+#define VME_USER1 0x20
+#define VME_USER2 0x40
+#define VME_USER3 0x80
+#define VME_USER4 0x100
+
+#define VME_A16_MAX0x1ULL
+#define VME_A24_MAX0x100ULL
+#define VME_A32_MAX0x1ULL
+#define VME_A64_MAX0x1ULL
+#define VME_CRCSR_MAX 0x100ULL
+
+/* VME Cycle Types */
+#define VME_SCT0x1
+#define VME_BLT0x2
+#define VME_MBLT 0x4
+#define VME_2eVME 0x8
+#define VME_2eSST 0x10
+#define VME_2eSSTB 0x20
+
+#define VME_2eSST160 0x100
+#define VME_2eSST267 0x200
+#define VME_2eSST320 0x400
+
+#define VME_SUPER 0x1000
+#define VME_USER 0x2000
+#define VME_PROG 0x4000
+#define VME_DATA 0x8000
+
+/* VME Data Widths */
+#define VME_D8 0x1
+#define VME_D160x2
+#define VME_D320x4
+#define VME_D640x8
+
+/* VME Transfer Directions */
+#define VME_DMA_VME_TO_MEM (1<<0)
+#define VME_DMA_MEM_TO_VME (1<<1)
+#define VME_DMA_VME_TO_VME (1<<2)
+#define VME_DMA_MEM_TO_MEM (1<<3)
+#define VME_DMA_PATTERN_TO_VME (1<<4)
+#define VME_DMA_PATTERN_TO_MEM (1<<5)
+
+#define VME_DMA_PATTERN_BYTE (1<<0)
+#define VME_DMA_PATTERN_WORD (1<<1)
+#define VME_DMA_PATTERN_INCREMENT (1<<2)
+
+#endif
--
1.8.3.1
___
[PATCHv2 13/16] staging: vme_user: remove open/release
Checking for image[minor].resource != NULL is not needed since all
resources are allocated before device is created.
image[minor].users accounting is deleted because it's not being used.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 44 --
1 file changed, 44 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 2493847..ce4aacf 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -102,7 +102,6 @@ struct image_desc {
struct mutex mutex; /* Mutex for locking image */
struct device *device; /* Sysfs device */
struct vme_resource *resource; /* VME resource */
- int users; /* Number of current users */
int mmap_count; /* Number of current mmap's */
};
static struct image_desc image[VME_DEVS];
@@ -125,46 +124,6 @@ struct vme_user_vma_priv {
};
-static int vme_user_open(struct inode *inode, struct file *file)
-{
- int err;
- unsigned int minor = MINOR(inode->i_rdev);
-
- mutex_lock(&image[minor].mutex);
- /* Allow device to be opened if a resource is needed and allocated. */
- if (minor < CONTROL_MINOR && image[minor].resource == NULL) {
- pr_err("No resources allocated for device\n");
- err = -EINVAL;
- goto err_res;
- }
-
- /* Increment user count */
- image[minor].users++;
-
- mutex_unlock(&image[minor].mutex);
-
- return 0;
-
-err_res:
- mutex_unlock(&image[minor].mutex);
-
- return err;
-}
-
-static int vme_user_release(struct inode *inode, struct file *file)
-{
- unsigned int minor = MINOR(inode->i_rdev);
-
- mutex_lock(&image[minor].mutex);
-
- /* Decrement user count */
- image[minor].users--;
-
- mutex_unlock(&image[minor].mutex);
-
- return 0;
-}
-
/*
* We are going ot alloc a page during init per window for small transfers.
* Small transfers will go VME -> buffer -> user space. Larger (more than a
@@ -775,8 +734,6 @@ static int vme_user_mmap(struct file *file, struct
vm_area_struct *vma)
}
static const struct file_operations vme_user_fops = {
- .open = vme_user_open,
- .release = vme_user_release,
.read = vme_user_read,
.write = vme_user_write,
.llseek = vme_user_llseek,
@@ -849,7 +806,6 @@ static int vme_user_probe(struct vme_dev *vdev)
mutex_init(&image[i].mutex);
image[i].device = NULL;
image[i].resource = NULL;
- image[i].users = 0;
}
/* Assign major and minor numbers for the driver */
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 01/16] Documentation: mention vme_master_mmap() in VME API
Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- Documentation/vme_api.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/vme_api.txt b/Documentation/vme_api.txt index ffe6e22..ca5b827 100644 --- a/Documentation/vme_api.txt +++ b/Documentation/vme_api.txt @@ -171,6 +171,12 @@ This functions by reading the offset, applying the mask. If the bits selected in the mask match with the values of the corresponding bits in the compare field, the value of swap is written the specified offset. +Parts of a VME window can be mapped into user space memory using the following +function: + + int vme_master_mmap(struct vme_resource *resource, + struct vm_area_struct *vma) + Slave windows = -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 00/16] vme DMA and user space driver improvements
The first item in this submission documents previously introduced
vme_master_mmap() call. Following, there are three fixes for the tsi148
driver's DMA. There was one bug that rendered it imposible to use DMA
lists with more than one item. The other was related to the place where
dma_map_single was called on the first DMA descriptor in the DMA list. The
last bug was about DMA transfer not stopping at interruption by signal,
which is a possible DoS attack vector. I also made an attempt to fix the
same issue in the ca91cx42 driver. I don't have access to this hardware to
test, so this is based only on my understanding of the datasheet (checked
ca91c042's errata as well).
A new /sys/bus/vme/dma0 device with a new ioctl for making DMA transfers
was introduced in vme_user driver. The logic of the function is similar to
the one found in existing drivers.
One question that I had while implementing this feature was whether we
should keep vme_dma_attr objects until vme_dma_list_exec() call. API
doesn't specify this, the existing vme bridge drivers copy all information
from attributes during vme_dma_list_add(). So for simplicity this
implementation frees vme_dma_attr's before vme_dma_list_exec() is done.
A simple test against AVM16 board displays speeds around 45 MiB/s for
A32/D32 reads for both BLT and MBLT (with MBLT being slightly faster).
Changes in v2 [patches 1-6, now 1-5 and 8]:
* vme_addr check for vme_user DMA
* limit on DMA operation length in vme_user
* reorder dma_op ioctl struct to omit __packed attribute
* change dma_op->write into dma_op->dir
* use vme_check_window assure DMA operation correctness
New changes include vme_user code cleanup, a couple of ca91cx42 fixes
(again, tested for compilation only).
I also propose a change to export some of VME subsytem related constants
to the user space. These can be useful if vme_user is to go into the kernel.
Also, email
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2012-July/029084.html
mentions that we probably can now get rid of this comment:
> /* XXX We do not want to push aspace, cycle and width
> * to userspace as they are
> */
v3 adresses code style problems
Dmitry Kalinkin (16):
Documentation: mention vme_master_mmap() in VME API
vme: tsi148: fix DMA lists longer that one item
vme: tsi148: fix first DMA item mapping
vme: stop DMA transfer on interruption
staging: vme_user: refactor llseek to switch(){}
vme: check for A64 overflow in vme_check_window()
vme: export vme_check_window()
staging: vme_user: provide DMA functionality
vme: ca91cx42: return error code on DMA error
vme: ca91cx42: fix LM_CTL address mask
staging: vme_user: remove unused counters
staging: vme_user: remove forward declarations
staging: vme_user: remove open/release
staging: vme_user: remove buf_unalloc helper
vme: tsi148: depend on HAS_DMA for Kconfig
vme: provide uapi header
Documentation/vme_api.txt | 6 +
drivers/staging/vme/devices/vme_user.c | 454 +++--
drivers/staging/vme/devices/vme_user.h | 11 +
drivers/vme/bridges/Kconfig| 2 +-
drivers/vme/bridges/vme_ca91cx42.c | 18 +-
drivers/vme/bridges/vme_ca91cx42.h | 2 +-
drivers/vme/bridges/vme_tsi148.c | 42 +--
drivers/vme/vme.c | 11 +-
include/linux/vme.h| 56 +---
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/vme.h | 56
11 files changed, 394 insertions(+), 265 deletions(-)
create mode 100644 include/uapi/linux/vme.h
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 07/16] vme: export vme_check_window()
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/vme/vme.c | 5 +++--
include/linux/vme.h | 2 ++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 1b78d27..5670891 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -177,8 +177,8 @@ size_t vme_get_size(struct vme_resource *resource)
}
EXPORT_SYMBOL(vme_get_size);
-static int vme_check_window(u32 aspace, unsigned long long vme_base,
- unsigned long long size)
+int vme_check_window(u32 aspace, unsigned long long vme_base,
+unsigned long long size)
{
int retval = 0;
@@ -221,6 +221,7 @@ static int vme_check_window(u32 aspace, unsigned long long
vme_base,
return retval;
}
+EXPORT_SYMBOL(vme_check_window);
/*
* Request a slave image with specific attributes, return some unique
diff --git a/include/linux/vme.h b/include/linux/vme.h
index 79242e9..c013135 100644
--- a/include/linux/vme.h
+++ b/include/linux/vme.h
@@ -120,6 +120,8 @@ void vme_free_consistent(struct vme_resource *, size_t,
void *,
dma_addr_t);
size_t vme_get_size(struct vme_resource *);
+int vme_check_window(u32 aspace, unsigned long long vme_base,
+unsigned long long size);
struct vme_resource *vme_slave_request(struct vme_dev *, u32, u32);
int vme_slave_set(struct vme_resource *, int, unsigned long long,
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 04/16] vme: stop DMA transfer on interruption
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/vme/bridges/vme_ca91cx42.c | 17 ++---
drivers/vme/bridges/vme_tsi148.c | 15 +--
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/drivers/vme/bridges/vme_ca91cx42.c
b/drivers/vme/bridges/vme_ca91cx42.c
index 18078ec..e9bd657 100644
--- a/drivers/vme/bridges/vme_ca91cx42.c
+++ b/drivers/vme/bridges/vme_ca91cx42.c
@@ -1192,7 +1192,7 @@ static int ca91cx42_dma_list_exec(struct vme_dma_list
*list)
{
struct vme_dma_resource *ctrlr;
struct ca91cx42_dma_entry *entry;
- int retval = 0;
+ int retval;
dma_addr_t bus_addr;
u32 val;
struct device *dev;
@@ -1245,8 +1245,18 @@ static int ca91cx42_dma_list_exec(struct vme_dma_list
*list)
iowrite32(val, bridge->base + DGCS);
- wait_event_interruptible(bridge->dma_queue,
- ca91cx42_dma_busy(ctrlr->parent));
+ retval = wait_event_interruptible(bridge->dma_queue,
+ ca91cx42_dma_busy(ctrlr->parent));
+
+ if (retval) {
+ val = ioread32(bridge->base + DGCS);
+ iowrite32(val | CA91CX42_DGCS_STOP_REQ, bridge->base + DGCS);
+ /* Wait for the operation to abort */
+ wait_event(bridge->dma_queue,
+ ca91cx42_dma_busy(ctrlr->parent));
+ retval = -EINTR;
+ goto exit;
+ }
/*
* Read status register, this register is valid until we kick off a
@@ -1261,6 +1271,7 @@ static int ca91cx42_dma_list_exec(struct vme_dma_list
*list)
val = ioread32(bridge->base + DCTL);
}
+exit:
/* Remove list from running list */
mutex_lock(&ctrlr->mtx);
list_del(&list->list);
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index 6562348..fb1e7ad 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -1892,7 +1892,7 @@ static int tsi148_dma_busy(struct vme_bridge
*tsi148_bridge, int channel)
static int tsi148_dma_list_exec(struct vme_dma_list *list)
{
struct vme_dma_resource *ctrlr;
- int channel, retval = 0;
+ int channel, retval;
struct tsi148_dma_entry *entry;
u32 bus_addr_high, bus_addr_low;
u32 val, dctlreg = 0;
@@ -1942,9 +1942,19 @@ static int tsi148_dma_list_exec(struct vme_dma_list
*list)
iowrite32be(dctlreg | TSI148_LCSR_DCTL_DGO, bridge->base +
TSI148_LCSR_DMA[channel] + TSI148_LCSR_OFFSET_DCTL);
- wait_event_interruptible(bridge->dma_queue[channel],
+ retval = wait_event_interruptible(bridge->dma_queue[channel],
tsi148_dma_busy(ctrlr->parent, channel));
+ if (retval) {
+ iowrite32be(dctlreg | TSI148_LCSR_DCTL_ABT, bridge->base +
+ TSI148_LCSR_DMA[channel] + TSI148_LCSR_OFFSET_DCTL);
+ /* Wait for the operation to abort */
+ wait_event(bridge->dma_queue[channel],
+ tsi148_dma_busy(ctrlr->parent, channel));
+ retval = -EINTR;
+ goto exit;
+ }
+
/*
* Read status register, this register is valid until we kick off a
* new transfer.
@@ -1957,6 +1967,7 @@ static int tsi148_dma_list_exec(struct vme_dma_list *list)
retval = -EIO;
}
+exit:
/* Remove list from running list */
mutex_lock(&ctrlr->mtx);
list_del(&list->list);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 02/16] vme: tsi148: fix DMA lists longer that one item
DMA lists on tsi148 weren't processed further than the first item because of the broken logic. This regression was introduced in: ac1a4f2caf7b071 "Staging: VME: Ensure TSI148 link list descriptors..." Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- drivers/vme/bridges/vme_tsi148.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c index 895c2a3..1be4136 100644 --- a/drivers/vme/bridges/vme_tsi148.c +++ b/drivers/vme/bridges/vme_tsi148.c @@ -1844,8 +1844,8 @@ static int tsi148_dma_list_add(struct vme_dma_list *list, reg_split((unsigned long long)entry->dma_handle, &address_high, &address_low); - entry->descriptor.dnlau = cpu_to_be32(address_high); - entry->descriptor.dnlal = cpu_to_be32(address_low); + prev->descriptor.dnlau = cpu_to_be32(address_high); + prev->descriptor.dnlal = cpu_to_be32(address_low); } -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 10/16] vme: ca91cx42: fix LM_CTL address mask
Universe II datasheet defines following address space values
for LM_CTL[16:18]
000=A16
001=A24
010=A32
011,100,101=Reserved
110=User1
111=User2
Mask 5<<16 is not the right one for matching [16:18], instead we should
use 7<<16.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
Reported-by: Dan Carpenter
---
drivers/vme/bridges/vme_ca91cx42.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vme/bridges/vme_ca91cx42.h
b/drivers/vme/bridges/vme_ca91cx42.h
index d46b12d..d54119e 100644
--- a/drivers/vme/bridges/vme_ca91cx42.h
+++ b/drivers/vme/bridges/vme_ca91cx42.h
@@ -547,7 +547,7 @@ static const int CA91CX42_LINT_LM[] = { CA91CX42_LINT_LM0,
CA91CX42_LINT_LM1,
#define CA91CX42_LM_CTL_DATA (1<<22)
#define CA91CX42_LM_CTL_SUPR (1<<21)
#define CA91CX42_LM_CTL_NPRIV (1<<20)
-#define CA91CX42_LM_CTL_AS_M (5<<16)
+#define CA91CX42_LM_CTL_AS_M (7<<16)
#define CA91CX42_LM_CTL_AS_A16 0
#define CA91CX42_LM_CTL_AS_A24 (1<<16)
#define CA91CX42_LM_CTL_AS_A32 (1<<17)
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 06/16] vme: check for A64 overflow in vme_check_window()
Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- drivers/vme/vme.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c index 6bab2c4..1b78d27 100644 --- a/drivers/vme/vme.c +++ b/drivers/vme/vme.c @@ -199,10 +199,8 @@ static int vme_check_window(u32 aspace, unsigned long long vme_base, retval = -EFAULT; break; case VME_A64: - /* -* Any value held in an unsigned long long can be used as the -* base -*/ + if ((size != 0) && (vme_base > U64_MAX + 1 - size)) + retval = -EFAULT; break; case VME_CRCSR: if (((vme_base + size) > VME_CRCSR_MAX) || -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 08/16] staging: vme_user: provide DMA functionality
This introduces a new dma device that provides a single ioctl call that
provides DMA read and write functionality to the user space.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 201 -
drivers/staging/vme/devices/vme_user.h | 11 ++
2 files changed, 209 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index da828f4..e8a1ca6 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -79,15 +79,18 @@ static unsigned int bus_num;
* We shall support 4 masters and 4 slaves with this driver.
*/
#define VME_MAJOR 221 /* VME Major Device Number */
-#define VME_DEVS 9 /* Number of dev entries */
+#define VME_DEVS 10 /* Number of dev entries */
#define MASTER_MINOR 0
#define MASTER_MAX 3
#define SLAVE_MINOR4
#define SLAVE_MAX 7
#define CONTROL_MINOR 8
+#define DMA_MINOR 9
-#define PCI_BUF_SIZE 0x2 /* Size of one slave image buffer */
+#define PCI_BUF_SIZE 0x2 /* Size of one slave image buffer */
+
+#define VME_MAX_DMA_LEN0x400 /* Maximal DMA transfer length
*/
/*
* Structure to handle image related parameters.
@@ -125,7 +128,7 @@ static const int type[VME_DEVS] = { MASTER_MINOR,
MASTER_MINOR,
MASTER_MINOR, MASTER_MINOR,
SLAVE_MINOR,SLAVE_MINOR,
SLAVE_MINOR,SLAVE_MINOR,
- CONTROL_MINOR
+ CONTROL_MINOR, DMA_MINOR
};
@@ -443,6 +446,168 @@ static loff_t vme_user_llseek(struct file *file, loff_t
off, int whence)
return -EINVAL;
}
+static int vme_user_sg_to_dma_list(const struct vme_dma_op *dma_op,
+ struct sg_table *sgt,
+ int sg_count, struct vme_dma_list *dma_list)
+{
+ ssize_t pos = 0;
+ struct scatterlist *sg;
+ int i, ret;
+
+ for_each_sg(sgt->sgl, sg, sg_count, i) {
+ struct vme_dma_attr *pci_attr, *vme_attr, *src, *dest;
+ dma_addr_t hw_address = sg_dma_address(sg);
+ unsigned int hw_len = sg_dma_len(sg);
+
+ vme_attr = vme_dma_vme_attribute(dma_op->vme_addr + pos,
+dma_op->aspace,
+dma_op->cycle,
+dma_op->dwidth);
+ if (!vme_attr)
+ return -ENOMEM;
+
+ pci_attr = vme_dma_pci_attribute(hw_address);
+ if (!pci_attr) {
+ vme_dma_free_attribute(vme_attr);
+ return -ENOMEM;
+ }
+
+ switch (dma_op->dir) {
+ case VME_DMA_MEM_TO_VME:
+ src = pci_attr;
+ dest = vme_attr;
+ break;
+ case VME_DMA_VME_TO_MEM:
+ src = vme_attr;
+ dest = pci_attr;
+ break;
+ }
+
+ ret = vme_dma_list_add(dma_list, src, dest, hw_len);
+
+ /*
+* XXX VME API doesn't mention whether we should keep
+* attributes around
+*/
+ vme_dma_free_attribute(vme_attr);
+ vme_dma_free_attribute(pci_attr);
+
+ if (ret)
+ return ret;
+
+ pos += hw_len;
+ }
+
+ return 0;
+}
+
+static enum dma_data_direction vme_dir_to_dma_dir(unsigned vme_dir)
+{
+ switch (vme_dir) {
+ case VME_DMA_VME_TO_MEM:
+ return DMA_FROM_DEVICE;
+ case VME_DMA_MEM_TO_VME:
+ return DMA_TO_DEVICE;
+ }
+
+ return DMA_NONE;
+}
+
+static ssize_t vme_user_dma_ioctl(unsigned int minor,
+ const struct vme_dma_op *dma_op)
+{
+ unsigned int offset = offset_in_page(dma_op->buf_vaddr);
+ unsigned long nr_pages;
+ enum dma_data_direction dir;
+ struct vme_dma_list *dma_list;
+ struct sg_table *sgt = NULL;
+ struct page **pages = NULL;
+ long got_pages;
+ ssize_t count;
+ int retval, sg_count;
+
+ /* Prevent WARN from dma_map_sg */
+ if (dma_op->count == 0)
+ return 0;
+
+ /*
+* This is a voluntary limit to prevent huge allocation for pages
+* array. VME_MAX_DMA_LEN is not a fundamental VME constraint.
+*/
+ count = min_t(size_t, dma_op->count, VME_MAX_DMA_LEN);
+ nr_pages = (offset + count + PAGE_SIZE - 1) >> PAGE_SHIFT;
+
+ dir = vme_d
[PATCHv3 11/16] staging: vme_user: remove unused counters
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 31 ---
1 file changed, 31 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index e8a1ca6..cc0d3df 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -107,18 +107,6 @@ struct image_desc {
};
static struct image_desc image[VME_DEVS];
-struct driver_stats {
- unsigned long reads;
- unsigned long writes;
- unsigned long ioctls;
- unsigned long irqs;
- unsigned long berrs;
- unsigned long dmaerrors;
- unsigned long timeouts;
- unsigned long external;
-};
-static struct driver_stats statistics;
-
static struct cdev *vme_user_cdev; /* Character device */
static struct class *vme_user_sysfs_class; /* Sysfs class */
static struct vme_dev *vme_user_bridge;/* Pointer to user
device */
@@ -170,20 +158,6 @@ static const struct vm_operations_struct vme_user_vm_ops =
{
};
-/*
- * Reset all the statistic counters
- */
-static void reset_counters(void)
-{
- statistics.reads = 0;
- statistics.writes = 0;
- statistics.ioctls = 0;
- statistics.irqs = 0;
- statistics.berrs = 0;
- statistics.dmaerrors = 0;
- statistics.timeouts = 0;
-}
-
static int vme_user_open(struct inode *inode, struct file *file)
{
int err;
@@ -631,8 +605,6 @@ static int vme_user_ioctl(struct inode *inode, struct file
*file,
dma_addr_t pci_addr;
void __user *argp = (void __user *)arg;
- statistics.ioctls++;
-
switch (type[minor]) {
case CONTROL_MINOR:
switch (cmd) {
@@ -944,9 +916,6 @@ static int vme_user_probe(struct vme_dev *vdev)
image[i].users = 0;
}
- /* Initialise statistics counters */
- reset_counters();
-
/* Assign major and minor numbers for the driver */
err = register_chrdev_region(MKDEV(VME_MAJOR, 0), VME_DEVS,
driver_name);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 03/16] vme: tsi148: fix first DMA item mapping
This moves DMA mapping of the first list element to vme_list_add, the
same place where other elements mappings occur. This prevents extra
mapping or over-unmapping in the cases when vme_list_exec is called more
or less than one time respectively.
Also adds dma_mapping_error check.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/vme/bridges/vme_tsi148.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index 1be4136..6562348 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -1833,17 +1833,21 @@ static int tsi148_dma_list_add(struct vme_dma_list
*list,
/* Add to list */
list_add_tail(&entry->list, &list->entries);
+ entry->dma_handle = dma_map_single(tsi148_bridge->parent,
+ &entry->descriptor,
+ sizeof(struct tsi148_dma_descriptor), DMA_TO_DEVICE);
+ if (dma_mapping_error(tsi148_bridge->parent, entry->dma_handle)) {
+ dev_err(tsi148_bridge->parent, "DMA mapping error\n");
+ retval = -EINVAL;
+ goto err_dma;
+ }
+
/* Fill out previous descriptors "Next Address" */
if (entry->list.prev != &list->entries) {
- prev = list_entry(entry->list.prev, struct tsi148_dma_entry,
- list);
- /* We need the bus address for the pointer */
- entry->dma_handle = dma_map_single(tsi148_bridge->parent,
- &entry->descriptor,
- sizeof(struct tsi148_dma_descriptor), DMA_TO_DEVICE);
-
reg_split((unsigned long long)entry->dma_handle, &address_high,
&address_low);
+ prev = list_entry(entry->list.prev, struct tsi148_dma_entry,
+ list);
prev->descriptor.dnlau = cpu_to_be32(address_high);
prev->descriptor.dnlal = cpu_to_be32(address_low);
@@ -1851,6 +1855,7 @@ static int tsi148_dma_list_add(struct vme_dma_list *list,
return 0;
+err_dma:
err_dest:
err_source:
err_align:
@@ -1921,10 +1926,6 @@ static int tsi148_dma_list_exec(struct vme_dma_list
*list)
entry = list_first_entry(&list->entries, struct tsi148_dma_entry,
list);
- entry->dma_handle = dma_map_single(tsi148_bridge->parent,
- &entry->descriptor,
- sizeof(struct tsi148_dma_descriptor), DMA_TO_DEVICE);
-
mutex_unlock(&ctrlr->mtx);
reg_split(entry->dma_handle, &bus_addr_high, &bus_addr_low);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 09/16] vme: ca91cx42: return error code on DMA error
Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- drivers/vme/bridges/vme_ca91cx42.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/vme/bridges/vme_ca91cx42.c b/drivers/vme/bridges/vme_ca91cx42.c index e9bd657..f692efc 100644 --- a/drivers/vme/bridges/vme_ca91cx42.c +++ b/drivers/vme/bridges/vme_ca91cx42.c @@ -1269,6 +1269,7 @@ static int ca91cx42_dma_list_exec(struct vme_dma_list *list) dev_err(dev, "ca91c042: DMA Error. DGCS=%08X\n", val); val = ioread32(bridge->base + DCTL); + retval = -EIO; } exit: -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 12/16] staging: vme_user: remove forward declarations
Reorder code so that forward declarations are not needed.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 139 ++---
1 file changed, 60 insertions(+), 79 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index cc0d3df..4755737 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -119,44 +119,11 @@ static const int type[VME_DEVS] = { MASTER_MINOR,
MASTER_MINOR,
CONTROL_MINOR, DMA_MINOR
};
-
-static int vme_user_open(struct inode *, struct file *);
-static int vme_user_release(struct inode *, struct file *);
-static ssize_t vme_user_read(struct file *, char __user *, size_t, loff_t *);
-static ssize_t vme_user_write(struct file *, const char __user *, size_t,
- loff_t *);
-static loff_t vme_user_llseek(struct file *, loff_t, int);
-static long vme_user_unlocked_ioctl(struct file *, unsigned int, unsigned
long);
-static int vme_user_mmap(struct file *file, struct vm_area_struct *vma);
-
-static void vme_user_vm_open(struct vm_area_struct *vma);
-static void vme_user_vm_close(struct vm_area_struct *vma);
-
-static int vme_user_match(struct vme_dev *);
-static int vme_user_probe(struct vme_dev *);
-static int vme_user_remove(struct vme_dev *);
-
-static const struct file_operations vme_user_fops = {
- .open = vme_user_open,
- .release = vme_user_release,
- .read = vme_user_read,
- .write = vme_user_write,
- .llseek = vme_user_llseek,
- .unlocked_ioctl = vme_user_unlocked_ioctl,
- .compat_ioctl = vme_user_unlocked_ioctl,
- .mmap = vme_user_mmap,
-};
-
struct vme_user_vma_priv {
unsigned int minor;
atomic_t refcnt;
};
-static const struct vm_operations_struct vme_user_vm_ops = {
- .open = vme_user_vm_open,
- .close = vme_user_vm_close,
-};
-
static int vme_user_open(struct inode *inode, struct file *file)
{
@@ -761,6 +728,11 @@ static void vme_user_vm_close(struct vm_area_struct *vma)
kfree(vma_priv);
}
+static const struct vm_operations_struct vme_user_vm_ops = {
+ .open = vme_user_vm_open,
+ .close = vme_user_vm_close,
+};
+
static int vme_user_master_mmap(unsigned int minor, struct vm_area_struct *vma)
{
int err;
@@ -802,6 +774,16 @@ static int vme_user_mmap(struct file *file, struct
vm_area_struct *vma)
return -ENODEV;
}
+static const struct file_operations vme_user_fops = {
+ .open = vme_user_open,
+ .release = vme_user_release,
+ .read = vme_user_read,
+ .write = vme_user_write,
+ .llseek = vme_user_llseek,
+ .unlocked_ioctl = vme_user_unlocked_ioctl,
+ .compat_ioctl = vme_user_unlocked_ioctl,
+ .mmap = vme_user_mmap,
+};
/*
* Unallocate a previously allocated buffer
@@ -828,52 +810,6 @@ static void buf_unalloc(int num)
}
}
-static struct vme_driver vme_user_driver = {
- .name = driver_name,
- .match = vme_user_match,
- .probe = vme_user_probe,
- .remove = vme_user_remove,
-};
-
-
-static int __init vme_user_init(void)
-{
- int retval = 0;
-
- pr_info("VME User Space Access Driver\n");
-
- if (bus_num == 0) {
- pr_err("No cards, skipping registration\n");
- retval = -ENODEV;
- goto err_nocard;
- }
-
- /* Let's start by supporting one bus, we can support more than one
-* in future revisions if that ever becomes necessary.
-*/
- if (bus_num > VME_USER_BUS_MAX) {
- pr_err("Driver only able to handle %d buses\n",
- VME_USER_BUS_MAX);
- bus_num = VME_USER_BUS_MAX;
- }
-
- /*
-* Here we just register the maximum number of devices we can and
-* leave vme_user_match() to allow only 1 to go through to probe().
-* This way, if we later want to allow multiple user access devices,
-* we just change the code in vme_user_match().
-*/
- retval = vme_register_driver(&vme_user_driver, VME_MAX_SLOTS);
- if (retval != 0)
- goto err_reg;
-
- return retval;
-
-err_reg:
-err_nocard:
- return retval;
-}
-
static int vme_user_match(struct vme_dev *vdev)
{
int i;
@@ -,6 +1047,51 @@ static int vme_user_remove(struct vme_dev *dev)
return 0;
}
+static struct vme_driver vme_user_driver = {
+ .name = driver_name,
+ .match = vme_user_match,
+ .probe = vme_user_probe,
+ .remove = vme_user_remove,
+};
+
+static int __init vme_user_init(void)
+{
+ int retval = 0;
+
+ pr_info("VME User Space Access Driver\n");
+
+ if (bus_num == 0) {
+ pr_err("No cards, skipping registration\n");
+
[PATCHv3 05/16] staging: vme_user: refactor llseek to switch(){}
This makes vme_user_llseek ignore all minors that don't have llseek
implementation.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 19ba749..da828f4 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -430,15 +430,17 @@ static loff_t vme_user_llseek(struct file *file, loff_t
off, int whence)
size_t image_size;
loff_t res;
- if (minor == CONTROL_MINOR)
- return -EINVAL;
-
- mutex_lock(&image[minor].mutex);
- image_size = vme_get_size(image[minor].resource);
- res = fixed_size_llseek(file, off, whence, image_size);
- mutex_unlock(&image[minor].mutex);
+ switch (type[minor]) {
+ case MASTER_MINOR:
+ case SLAVE_MINOR:
+ mutex_lock(&image[minor].mutex);
+ image_size = vme_get_size(image[minor].resource);
+ res = fixed_size_llseek(file, off, whence, image_size);
+ mutex_unlock(&image[minor].mutex);
+ return res;
+ }
- return res;
+ return -EINVAL;
}
/*
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 14/16] staging: vme_user: remove buf_unalloc helper
buf_unalloc is essentially a vme_free_consistent:
1) image[i].kern_buf is never NULL in buf_alloc call
2) kern_buf, pci_buf and size_buf get zeroed in vme_user_probe anyway
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 31 ---
1 file changed, 4 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 381b052..5cc782e 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -742,31 +742,6 @@ static const struct file_operations vme_user_fops = {
.mmap = vme_user_mmap,
};
-/*
- * Unallocate a previously allocated buffer
- */
-static void buf_unalloc(int num)
-{
- if (image[num].kern_buf) {
-#ifdef VME_DEBUG
- pr_debug("UniverseII:Releasing buffer at %p\n",
-image[num].pci_buf);
-#endif
-
- vme_free_consistent(image[num].resource, image[num].size_buf,
- image[num].kern_buf, image[num].pci_buf);
-
- image[num].kern_buf = NULL;
- image[num].pci_buf = 0;
- image[num].size_buf = 0;
-
-#ifdef VME_DEBUG
- } else {
- pr_debug("UniverseII: Buffer not allocated\n");
-#endif
- }
-}
-
static int vme_user_match(struct vme_dev *vdev)
{
int i;
@@ -958,7 +933,8 @@ err_master:
err_slave:
while (i > SLAVE_MINOR) {
i--;
- buf_unalloc(i);
+ vme_free_consistent(image[i].resource, image[i].size_buf,
+ image[i].kern_buf, image[i].pci_buf);
vme_slave_free(image[i].resource);
}
err_class:
@@ -990,7 +966,8 @@ static int vme_user_remove(struct vme_dev *dev)
for (i = SLAVE_MINOR; i < (SLAVE_MAX + 1); i++) {
vme_slave_set(image[i].resource, 0, 0, 0, 0, VME_A32, 0);
- buf_unalloc(i);
+ vme_free_consistent(image[i].resource, image[i].size_buf,
+ image[i].kern_buf, image[i].pci_buf);
vme_slave_free(image[i].resource);
}
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 16/16] vme: provide uapi header
This separates VME related constants that are a part of both kernel and
user space API into a common uapi header.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
include/linux/vme.h | 54 ++---
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/vme.h | 56 +++
3 files changed, 59 insertions(+), 52 deletions(-)
create mode 100644 include/uapi/linux/vme.h
diff --git a/include/linux/vme.h b/include/linux/vme.h
index c013135..ecfd389 100644
--- a/include/linux/vme.h
+++ b/include/linux/vme.h
@@ -1,6 +1,8 @@
#ifndef _VME_H_
#define _VME_H_
+#include
+
/* Resource Type */
enum vme_resource_type {
VME_MASTER,
@@ -9,47 +11,6 @@ enum vme_resource_type {
VME_LM
};
-/* VME Address Spaces */
-#define VME_A160x1
-#define VME_A240x2
-#defineVME_A32 0x4
-#define VME_A640x8
-#define VME_CRCSR 0x10
-#define VME_USER1 0x20
-#define VME_USER2 0x40
-#define VME_USER3 0x80
-#define VME_USER4 0x100
-
-#define VME_A16_MAX0x1ULL
-#define VME_A24_MAX0x100ULL
-#define VME_A32_MAX0x1ULL
-#define VME_A64_MAX0x1ULL
-#define VME_CRCSR_MAX 0x100ULL
-
-
-/* VME Cycle Types */
-#define VME_SCT0x1
-#define VME_BLT0x2
-#define VME_MBLT 0x4
-#define VME_2eVME 0x8
-#define VME_2eSST 0x10
-#define VME_2eSSTB 0x20
-
-#define VME_2eSST160 0x100
-#define VME_2eSST267 0x200
-#define VME_2eSST320 0x400
-
-#defineVME_SUPER 0x1000
-#defineVME_USER0x2000
-#defineVME_PROG0x4000
-#defineVME_DATA0x8000
-
-/* VME Data Widths */
-#define VME_D8 0x1
-#define VME_D160x2
-#define VME_D320x4
-#define VME_D640x8
-
/* Arbitration Scheduling Modes */
#define VME_R_ROBIN_MODE 0x1
#define VME_PRIORITY_MODE 0x2
@@ -58,17 +19,6 @@ enum vme_resource_type {
#define VME_DMA_PCI(1<<1)
#define VME_DMA_VME(1<<2)
-#define VME_DMA_PATTERN_BYTE (1<<0)
-#define VME_DMA_PATTERN_WORD (1<<1)
-#define VME_DMA_PATTERN_INCREMENT (1<<2)
-
-#define VME_DMA_VME_TO_MEM (1<<0)
-#define VME_DMA_MEM_TO_VME (1<<1)
-#define VME_DMA_VME_TO_VME (1<<2)
-#define VME_DMA_MEM_TO_MEM (1<<3)
-#define VME_DMA_PATTERN_TO_VME (1<<4)
-#define VME_DMA_PATTERN_TO_MEM (1<<5)
-
struct vme_dma_attr {
u32 type;
void *private;
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 1a0006a..ad25e3f 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -439,6 +439,7 @@ header-y += virtio_rng.h
header-y += virtio_scsi.h
header-y += virtio_types.h
header-y += vm_sockets.h
+header-y += vme.h
header-y += vt.h
header-y += wait.h
header-y += wanrouter.h
diff --git a/include/uapi/linux/vme.h b/include/uapi/linux/vme.h
new file mode 100644
index 000..3dbf8b0
--- /dev/null
+++ b/include/uapi/linux/vme.h
@@ -0,0 +1,56 @@
+#ifndef _UAPI_LINUX_VME
+#define _UAPI_LINUX_VME
+
+/* VME Address Spaces */
+#define VME_A160x1
+#define VME_A240x2
+#define VME_A320x4
+#define VME_A640x8
+#define VME_CRCSR 0x10
+#define VME_USER1 0x20
+#define VME_USER2 0x40
+#define VME_USER3 0x80
+#define VME_USER4 0x100
+
+#define VME_A16_MAX0x1ULL
+#define VME_A24_MAX0x100ULL
+#define VME_A32_MAX0x1ULL
+#define VME_A64_MAX0x1ULL
+#define VME_CRCSR_MAX 0x100ULL
+
+/* VME Cycle Types */
+#define VME_SCT0x1
+#define VME_BLT0x2
+#define VME_MBLT 0x4
+#define VME_2eVME 0x8
+#define VME_2eSST 0x10
+#define VME_2eSSTB 0x20
+
+#define VME_2eSST160 0x100
+#define VME_2eSST267 0x200
+#define VME_2eSST320 0x400
+
+#define VME_SUPER 0x1000
+#define VME_USER 0x2000
+#define VME_PROG 0x4000
+#define VME_DATA 0x8000
+
+/* VME Data Widths */
+#define VME_D8 0x1
+#define VME_D160x2
+#define VME_D320x4
+#define VME_D640x8
+
+/* VME Transfer Directions */
+#define VME_DMA_VME_TO_MEM (1 << 0)
+#define VME_DMA_MEM_TO_VME (1 << 1)
+#define VME_DMA_VME_TO_VME (1 << 2)
+#define VME_DMA_MEM_TO_MEM (1 << 3)
+#define VME_DMA_PATTERN_TO_VME (1 << 4)
+#define VME_DMA_PATTERN_TO_MEM (1 << 5)
+
+#define VME_DMA_PATTERN_BYTE (1 << 0)
+#define VME_DMA_PATTERN_WORD (1 << 1)
+#define VME_DMA_PATTERN_INCREMENT (1 << 2)
+
+
[PATCHv3 13/16] staging: vme_user: remove open/release
Checking for image[minor].resource != NULL is not needed since all
resources are allocated before device is created.
image[minor].users accounting is deleted because it's not being used.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 44 --
1 file changed, 44 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 4755737..381b052 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -102,7 +102,6 @@ struct image_desc {
struct mutex mutex; /* Mutex for locking image */
struct device *device; /* Sysfs device */
struct vme_resource *resource; /* VME resource */
- int users; /* Number of current users */
int mmap_count; /* Number of current mmap's */
};
static struct image_desc image[VME_DEVS];
@@ -125,46 +124,6 @@ struct vme_user_vma_priv {
};
-static int vme_user_open(struct inode *inode, struct file *file)
-{
- int err;
- unsigned int minor = MINOR(inode->i_rdev);
-
- mutex_lock(&image[minor].mutex);
- /* Allow device to be opened if a resource is needed and allocated. */
- if (minor < CONTROL_MINOR && image[minor].resource == NULL) {
- pr_err("No resources allocated for device\n");
- err = -EINVAL;
- goto err_res;
- }
-
- /* Increment user count */
- image[minor].users++;
-
- mutex_unlock(&image[minor].mutex);
-
- return 0;
-
-err_res:
- mutex_unlock(&image[minor].mutex);
-
- return err;
-}
-
-static int vme_user_release(struct inode *inode, struct file *file)
-{
- unsigned int minor = MINOR(inode->i_rdev);
-
- mutex_lock(&image[minor].mutex);
-
- /* Decrement user count */
- image[minor].users--;
-
- mutex_unlock(&image[minor].mutex);
-
- return 0;
-}
-
/*
* We are going ot alloc a page during init per window for small transfers.
* Small transfers will go VME -> buffer -> user space. Larger (more than a
@@ -775,8 +734,6 @@ static int vme_user_mmap(struct file *file, struct
vm_area_struct *vma)
}
static const struct file_operations vme_user_fops = {
- .open = vme_user_open,
- .release = vme_user_release,
.read = vme_user_read,
.write = vme_user_write,
.llseek = vme_user_llseek,
@@ -849,7 +806,6 @@ static int vme_user_probe(struct vme_dev *vdev)
mutex_init(&image[i].mutex);
image[i].device = NULL;
image[i].resource = NULL;
- image[i].users = 0;
}
/* Assign major and minor numbers for the driver */
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 15/16] vme: tsi148: depend on HAS_DMA for Kconfig
Signed-off-by: Dmitry Kalinkin Cc: Igor Alekseev --- drivers/vme/bridges/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vme/bridges/Kconfig b/drivers/vme/bridges/Kconfig index 9331064..f6d8545 100644 --- a/drivers/vme/bridges/Kconfig +++ b/drivers/vme/bridges/Kconfig @@ -9,7 +9,7 @@ config VME_CA91CX42 config VME_TSI148 tristate "Tempe" - depends on VIRT_TO_BUS + depends on HAS_DMA help If you say Y here you get support for the Tundra TSI148 VME bridge chip. -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv3 00/16] vme DMA and user space driver improvements
On Sun, May 31, 2015 at 6:06 AM, Greg Kroah-Hartman wrote: > On Thu, May 28, 2015 at 03:06:57PM +0300, Dmitry Kalinkin wrote: >> The first item in this submission documents previously introduced >> vme_master_mmap() call. Following, there are three fixes for the tsi148 >> driver's DMA. There was one bug that rendered it imposible to use DMA >> lists with more than one item. The other was related to the place where >> dma_map_single was called on the first DMA descriptor in the DMA list. The >> last bug was about DMA transfer not stopping at interruption by signal, >> which is a possible DoS attack vector. I also made an attempt to fix the >> same issue in the ca91cx42 driver. I don't have access to this hardware to >> test, so this is based only on my understanding of the datasheet (checked >> ca91c042's errata as well). >> >> A new /sys/bus/vme/dma0 device with a new ioctl for making DMA transfers >> was introduced in vme_user driver. The logic of the function is similar to >> the one found in existing drivers. >> >> One question that I had while implementing this feature was whether we >> should keep vme_dma_attr objects until vme_dma_list_exec() call. API >> doesn't specify this, the existing vme bridge drivers copy all information >> from attributes during vme_dma_list_add(). So for simplicity this >> implementation frees vme_dma_attr's before vme_dma_list_exec() is done. >> >> Changes in v2 [patches 1-6, now 1-5 and 8]: >> * vme_addr check for vme_user DMA >> * limit on DMA operation length in vme_user >> * reorder dma_op ioctl struct to omit __packed attribute >> * change dma_op->write into dma_op->dir >> * use vme_check_window assure DMA operation correctness >> >> New changes include vme_user code cleanup, a couple of ca91cx42 fixes >> (again, tested for compilation only). >> >> I also propose a change to export some of VME subsytem related constants >> to the user space. These can be useful if vme_user is to go into the kernel. >> Also, email >> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2012-July/029084.html >> mentions that we probably can now get rid of this comment: >> > /* XXX We do not want to push aspace, cycle and width >> > * to userspace as they are >> > */ >> >> v3 adresses code style problems > > I need an ack from the VME maintainer before I can take these... > > thanks, > > greg k-h Greg, I don't know how to make this happen. I could resend patches CC'ing previous contributors to linux VME subsystem, and maybe get some feedback that way. Also, there are some patches that IMO don't need any special VME subsystem expertise, namely: Documentation: mention vme_master_mmap() in VME API vme: ca91cx42: return error code on DMA error staging: vme_user: remove unused counters staging: vme_user: remove forward declarations staging: vme_user: remove open/release staging: vme_user: remove buf_unalloc helper vme: tsi148: depend on HAS_DMA for Kconfig Thanks, Dmitry ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv3 00/16] vme DMA and user space driver improvements
On Sat, Jun 13, 2015 at 3:31 AM, Greg Kroah-Hartman wrote: > On Wed, Jun 10, 2015 at 04:09:19PM +0300, Dmitry Kalinkin wrote: >> Also, there are some patches that IMO don't need any special VME >> subsystem expertise, namely: >> Documentation: mention vme_master_mmap() in VME API >> vme: ca91cx42: return error code on DMA error >> staging: vme_user: remove unused counters >> staging: vme_user: remove forward declarations >> staging: vme_user: remove open/release >> staging: vme_user: remove buf_unalloc helper >> vme: tsi148: depend on HAS_DMA for Kconfig > > I've taken all of these except patches 12, 13, 14 and 16. > I thought 12 was the most harmless out of the whole set. Am I wrong? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv3 00/16] vme DMA and user space driver improvements
> On 13 Jun 2015, at 05:24, Greg Kroah-Hartman > wrote: > > On Sat, Jun 13, 2015 at 05:04:28AM +0300, Dmitry Kalinkin wrote: >> On Sat, Jun 13, 2015 at 3:31 AM, Greg Kroah-Hartman >> wrote: >> I thought 12 was the most harmless out of the whole set. Am I wrong? > > You added a new userspace api, that someone else is going to have to > maintain, that's not "harmless" at all. That is 16. 12, 13, 14 are unrelated to uapi: staging: vme_user: remove forward declarations staging: vme_user: remove open/release staging: vme_user: remove buf_unalloc helper ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv4 2/4] staging: vme_user: remove open/release
Checking for image[minor].resource != NULL is not needed since all
resources are allocated before device is created.
image[minor].users accounting is deleted because it's not being used.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 44 --
1 file changed, 44 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 8e46d60..a72f7a9 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -99,7 +99,6 @@ struct image_desc {
struct mutex mutex; /* Mutex for locking image */
struct device *device; /* Sysfs device */
struct vme_resource *resource; /* VME resource */
- int users; /* Number of current users */
int mmap_count; /* Number of current mmap's */
};
static struct image_desc image[VME_DEVS];
@@ -122,46 +121,6 @@ struct vme_user_vma_priv {
};
-static int vme_user_open(struct inode *inode, struct file *file)
-{
- int err;
- unsigned int minor = MINOR(inode->i_rdev);
-
- mutex_lock(&image[minor].mutex);
- /* Allow device to be opened if a resource is needed and allocated. */
- if (minor < CONTROL_MINOR && image[minor].resource == NULL) {
- pr_err("No resources allocated for device\n");
- err = -EINVAL;
- goto err_res;
- }
-
- /* Increment user count */
- image[minor].users++;
-
- mutex_unlock(&image[minor].mutex);
-
- return 0;
-
-err_res:
- mutex_unlock(&image[minor].mutex);
-
- return err;
-}
-
-static int vme_user_release(struct inode *inode, struct file *file)
-{
- unsigned int minor = MINOR(inode->i_rdev);
-
- mutex_lock(&image[minor].mutex);
-
- /* Decrement user count */
- image[minor].users--;
-
- mutex_unlock(&image[minor].mutex);
-
- return 0;
-}
-
/*
* We are going ot alloc a page during init per window for small transfers.
* Small transfers will go VME -> buffer -> user space. Larger (more than a
@@ -596,8 +555,6 @@ static int vme_user_mmap(struct file *file, struct
vm_area_struct *vma)
}
static const struct file_operations vme_user_fops = {
- .open = vme_user_open,
- .release = vme_user_release,
.read = vme_user_read,
.write = vme_user_write,
.llseek = vme_user_llseek,
@@ -670,7 +627,6 @@ static int vme_user_probe(struct vme_dev *vdev)
mutex_init(&image[i].mutex);
image[i].device = NULL;
image[i].resource = NULL;
- image[i].users = 0;
}
/* Assign major and minor numbers for the driver */
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv4 3/4] staging: vme_user: remove buf_unalloc helper
buf_unalloc is essentially a vme_free_consistent:
1) image[i].kern_buf is never NULL in buf_alloc call
2) kern_buf, pci_buf and size_buf get zeroed in vme_user_probe anyway
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 31 ---
1 file changed, 4 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index a72f7a9..9cca97a 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -563,31 +563,6 @@ static const struct file_operations vme_user_fops = {
.mmap = vme_user_mmap,
};
-/*
- * Unallocate a previously allocated buffer
- */
-static void buf_unalloc(int num)
-{
- if (image[num].kern_buf) {
-#ifdef VME_DEBUG
- pr_debug("UniverseII:Releasing buffer at %p\n",
-image[num].pci_buf);
-#endif
-
- vme_free_consistent(image[num].resource, image[num].size_buf,
- image[num].kern_buf, image[num].pci_buf);
-
- image[num].kern_buf = NULL;
- image[num].pci_buf = 0;
- image[num].size_buf = 0;
-
-#ifdef VME_DEBUG
- } else {
- pr_debug("UniverseII: Buffer not allocated\n");
-#endif
- }
-}
-
static int vme_user_match(struct vme_dev *vdev)
{
int i;
@@ -765,7 +740,8 @@ err_master:
err_slave:
while (i > SLAVE_MINOR) {
i--;
- buf_unalloc(i);
+ vme_free_consistent(image[i].resource, image[i].size_buf,
+ image[i].kern_buf, image[i].pci_buf);
vme_slave_free(image[i].resource);
}
err_class:
@@ -795,7 +771,8 @@ static int vme_user_remove(struct vme_dev *dev)
for (i = SLAVE_MINOR; i < (SLAVE_MAX + 1); i++) {
vme_slave_set(image[i].resource, 0, 0, 0, 0, VME_A32, 0);
- buf_unalloc(i);
+ vme_free_consistent(image[i].resource, image[i].size_buf,
+ image[i].kern_buf, image[i].pci_buf);
vme_slave_free(image[i].resource);
}
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv4 4/4] staging: vme_user: provide DMA functionality
This introduces a new dma device that provides a single ioctl call that
provides DMA read and write functionality to the user space.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 201 -
drivers/staging/vme/devices/vme_user.h | 11 ++
2 files changed, 209 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 9cca97a..5cc782e 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -79,15 +79,18 @@ static unsigned int bus_num;
* We shall support 4 masters and 4 slaves with this driver.
*/
#define VME_MAJOR 221 /* VME Major Device Number */
-#define VME_DEVS 9 /* Number of dev entries */
+#define VME_DEVS 10 /* Number of dev entries */
#define MASTER_MINOR 0
#define MASTER_MAX 3
#define SLAVE_MINOR4
#define SLAVE_MAX 7
#define CONTROL_MINOR 8
+#define DMA_MINOR 9
-#define PCI_BUF_SIZE 0x2 /* Size of one slave image buffer */
+#define PCI_BUF_SIZE 0x2 /* Size of one slave image buffer */
+
+#define VME_MAX_DMA_LEN0x400 /* Maximal DMA transfer length
*/
/*
* Structure to handle image related parameters.
@@ -112,7 +115,7 @@ static const int type[VME_DEVS] = { MASTER_MINOR,
MASTER_MINOR,
MASTER_MINOR, MASTER_MINOR,
SLAVE_MINOR,SLAVE_MINOR,
SLAVE_MINOR,SLAVE_MINOR,
- CONTROL_MINOR
+ CONTROL_MINOR, DMA_MINOR
};
struct vme_user_vma_priv {
@@ -343,6 +346,168 @@ static loff_t vme_user_llseek(struct file *file, loff_t
off, int whence)
return -EINVAL;
}
+static int vme_user_sg_to_dma_list(const struct vme_dma_op *dma_op,
+ struct sg_table *sgt,
+ int sg_count, struct vme_dma_list *dma_list)
+{
+ ssize_t pos = 0;
+ struct scatterlist *sg;
+ int i, ret;
+
+ for_each_sg(sgt->sgl, sg, sg_count, i) {
+ struct vme_dma_attr *pci_attr, *vme_attr, *src, *dest;
+ dma_addr_t hw_address = sg_dma_address(sg);
+ unsigned int hw_len = sg_dma_len(sg);
+
+ vme_attr = vme_dma_vme_attribute(dma_op->vme_addr + pos,
+dma_op->aspace,
+dma_op->cycle,
+dma_op->dwidth);
+ if (!vme_attr)
+ return -ENOMEM;
+
+ pci_attr = vme_dma_pci_attribute(hw_address);
+ if (!pci_attr) {
+ vme_dma_free_attribute(vme_attr);
+ return -ENOMEM;
+ }
+
+ switch (dma_op->dir) {
+ case VME_DMA_MEM_TO_VME:
+ src = pci_attr;
+ dest = vme_attr;
+ break;
+ case VME_DMA_VME_TO_MEM:
+ src = vme_attr;
+ dest = pci_attr;
+ break;
+ }
+
+ ret = vme_dma_list_add(dma_list, src, dest, hw_len);
+
+ /*
+* XXX VME API doesn't mention whether we should keep
+* attributes around
+*/
+ vme_dma_free_attribute(vme_attr);
+ vme_dma_free_attribute(pci_attr);
+
+ if (ret)
+ return ret;
+
+ pos += hw_len;
+ }
+
+ return 0;
+}
+
+static enum dma_data_direction vme_dir_to_dma_dir(unsigned vme_dir)
+{
+ switch (vme_dir) {
+ case VME_DMA_VME_TO_MEM:
+ return DMA_FROM_DEVICE;
+ case VME_DMA_MEM_TO_VME:
+ return DMA_TO_DEVICE;
+ }
+
+ return DMA_NONE;
+}
+
+static ssize_t vme_user_dma_ioctl(unsigned int minor,
+ const struct vme_dma_op *dma_op)
+{
+ unsigned int offset = offset_in_page(dma_op->buf_vaddr);
+ unsigned long nr_pages;
+ enum dma_data_direction dir;
+ struct vme_dma_list *dma_list;
+ struct sg_table *sgt = NULL;
+ struct page **pages = NULL;
+ long got_pages;
+ ssize_t count;
+ int retval, sg_count;
+
+ /* Prevent WARN from dma_map_sg */
+ if (dma_op->count == 0)
+ return 0;
+
+ /*
+* This is a voluntary limit to prevent huge allocation for pages
+* array. VME_MAX_DMA_LEN is not a fundamental VME constraint.
+*/
+ count = min_t(size_t, dma_op->count, VME_MAX_DMA_LEN);
+ nr_pages = (offset + count + PAGE_SIZE - 1) >> PAGE_SHI
[PATCHv4 1/4] staging: vme_user: remove forward declarations
Reorder code so that forward declarations are not needed.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/staging/vme/devices/vme_user.c | 139 ++---
1 file changed, 60 insertions(+), 79 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 449b8cd..8e46d60 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -116,44 +116,11 @@ static const int type[VME_DEVS] = { MASTER_MINOR,
MASTER_MINOR,
CONTROL_MINOR
};
-
-static int vme_user_open(struct inode *, struct file *);
-static int vme_user_release(struct inode *, struct file *);
-static ssize_t vme_user_read(struct file *, char __user *, size_t, loff_t *);
-static ssize_t vme_user_write(struct file *, const char __user *, size_t,
- loff_t *);
-static loff_t vme_user_llseek(struct file *, loff_t, int);
-static long vme_user_unlocked_ioctl(struct file *, unsigned int, unsigned
long);
-static int vme_user_mmap(struct file *file, struct vm_area_struct *vma);
-
-static void vme_user_vm_open(struct vm_area_struct *vma);
-static void vme_user_vm_close(struct vm_area_struct *vma);
-
-static int vme_user_match(struct vme_dev *);
-static int vme_user_probe(struct vme_dev *);
-static int vme_user_remove(struct vme_dev *);
-
-static const struct file_operations vme_user_fops = {
- .open = vme_user_open,
- .release = vme_user_release,
- .read = vme_user_read,
- .write = vme_user_write,
- .llseek = vme_user_llseek,
- .unlocked_ioctl = vme_user_unlocked_ioctl,
- .compat_ioctl = vme_user_unlocked_ioctl,
- .mmap = vme_user_mmap,
-};
-
struct vme_user_vma_priv {
unsigned int minor;
atomic_t refcnt;
};
-static const struct vm_operations_struct vme_user_vm_ops = {
- .open = vme_user_vm_open,
- .close = vme_user_vm_close,
-};
-
static int vme_user_open(struct inode *inode, struct file *file)
{
@@ -582,6 +549,11 @@ static void vme_user_vm_close(struct vm_area_struct *vma)
kfree(vma_priv);
}
+static const struct vm_operations_struct vme_user_vm_ops = {
+ .open = vme_user_vm_open,
+ .close = vme_user_vm_close,
+};
+
static int vme_user_master_mmap(unsigned int minor, struct vm_area_struct *vma)
{
int err;
@@ -623,6 +595,16 @@ static int vme_user_mmap(struct file *file, struct
vm_area_struct *vma)
return -ENODEV;
}
+static const struct file_operations vme_user_fops = {
+ .open = vme_user_open,
+ .release = vme_user_release,
+ .read = vme_user_read,
+ .write = vme_user_write,
+ .llseek = vme_user_llseek,
+ .unlocked_ioctl = vme_user_unlocked_ioctl,
+ .compat_ioctl = vme_user_unlocked_ioctl,
+ .mmap = vme_user_mmap,
+};
/*
* Unallocate a previously allocated buffer
@@ -649,52 +631,6 @@ static void buf_unalloc(int num)
}
}
-static struct vme_driver vme_user_driver = {
- .name = driver_name,
- .match = vme_user_match,
- .probe = vme_user_probe,
- .remove = vme_user_remove,
-};
-
-
-static int __init vme_user_init(void)
-{
- int retval = 0;
-
- pr_info("VME User Space Access Driver\n");
-
- if (bus_num == 0) {
- pr_err("No cards, skipping registration\n");
- retval = -ENODEV;
- goto err_nocard;
- }
-
- /* Let's start by supporting one bus, we can support more than one
-* in future revisions if that ever becomes necessary.
-*/
- if (bus_num > VME_USER_BUS_MAX) {
- pr_err("Driver only able to handle %d buses\n",
- VME_USER_BUS_MAX);
- bus_num = VME_USER_BUS_MAX;
- }
-
- /*
-* Here we just register the maximum number of devices we can and
-* leave vme_user_match() to allow only 1 to go through to probe().
-* This way, if we later want to allow multiple user access devices,
-* we just change the code in vme_user_match().
-*/
- retval = vme_register_driver(&vme_user_driver, VME_MAX_SLOTS);
- if (retval != 0)
- goto err_reg;
-
- return retval;
-
-err_reg:
-err_nocard:
- return retval;
-}
-
static int vme_user_match(struct vme_dev *vdev)
{
int i;
@@ -916,6 +852,51 @@ static int vme_user_remove(struct vme_dev *dev)
return 0;
}
+static struct vme_driver vme_user_driver = {
+ .name = driver_name,
+ .match = vme_user_match,
+ .probe = vme_user_probe,
+ .remove = vme_user_remove,
+};
+
+static int __init vme_user_init(void)
+{
+ int retval = 0;
+
+ pr_info("VME User Space Access Driver\n");
+
+ if (bus_num == 0) {
+ pr_err("No cards, skipping registration\n");
+
[PATCHv4 0/4] vme DMA and user space driver improvements
This reorders patches so that Greg can take first three, while fourth awaits maintainer's approval. Dmitry Kalinkin (4): staging: vme_user: remove forward declarations staging: vme_user: remove open/release staging: vme_user: remove buf_unalloc helper staging: vme_user: provide DMA functionality drivers/staging/vme/devices/vme_user.c | 409 + drivers/staging/vme/devices/vme_user.h | 11 + 2 files changed, 270 insertions(+), 150 deletions(-) -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/9] staging: vme_user: allow large read()/write()
This changes large master transfers to do shorter read/write rather than
return -EINVAL. User space will now be able to optimistically request a
large transfer and get at least some data.
This also removes comments suggesting on how to implement large
transfers. Current vme_master_* read and write implementations use CPU
copies that don't produce burst PCI accesses and subsequently no block
transfer on VME bus. In the end overall performance is quiet low and it
can't be fixed by doing direct copy to user space. Much easier solution
would be to just reuse kernel buffer.
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 67 --
1 file changed, 24 insertions(+), 43 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index db5f890..52cd638 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -132,63 +132,44 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
ssize_t retval;
ssize_t copied = 0;
- if (count <= image[minor].size_buf) {
- /* We copy to kernel buffer */
- copied = vme_master_read(image[minor].resource,
- image[minor].kern_buf, count, *ppos);
- if (copied < 0)
- return (int)copied;
-
- retval = __copy_to_user(buf, image[minor].kern_buf,
- (unsigned long)copied);
- if (retval != 0) {
- copied = (copied - retval);
- pr_info("User copy failed\n");
- return -EINVAL;
- }
+ if (count > image[minor].size_buf)
+ count = image[minor].size_buf;
- } else {
- /* XXX Need to write this */
- pr_info("Currently don't support large transfers\n");
- /* Map in pages from userspace */
+ /* We copy to kernel buffer */
+ copied = vme_master_read(image[minor].resource, image[minor].kern_buf,
+count, *ppos);
+ if (copied < 0)
+ return (int)copied;
- /* Call vme_master_read to do the transfer */
+ retval = __copy_to_user(buf, image[minor].kern_buf,
+ (unsigned long)copied);
+ if (retval != 0) {
+ copied = (copied - retval);
+ pr_info("User copy failed\n");
return -EINVAL;
}
return copied;
}
-/*
- * We are going to alloc a page during init per window for small transfers.
- * Small transfers will go user space -> buffer -> VME. Larger (more than a
- * page) transfers will lock the user space buffer into memory and then
- * transfer the data directly from the user space buffers out to VME.
- */
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
ssize_t retval;
ssize_t copied = 0;
- if (count <= image[minor].size_buf) {
- retval = __copy_from_user(image[minor].kern_buf, buf,
- (unsigned long)count);
- if (retval != 0)
- copied = (copied - retval);
- else
- copied = count;
-
- copied = vme_master_write(image[minor].resource,
- image[minor].kern_buf, copied, *ppos);
- } else {
- /* XXX Need to write this */
- pr_info("Currently don't support large transfers\n");
- /* Map in pages from userspace */
-
- /* Call vme_master_write to do the transfer */
- return -EINVAL;
- }
+ if (count > image[minor].size_buf)
+ count = image[minor].size_buf;
+
+ retval = __copy_from_user(image[minor].kern_buf, buf,
+ (unsigned long)count);
+ if (retval != 0)
+ copied = (copied - retval);
+ else
+ copied = count;
+
+ copied = vme_master_write(image[minor].resource, image[minor].kern_buf,
+ copied, *ppos);
return copied;
}
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/9] staging: vme_user: fix kmalloc style
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index b6d81e5..db5f890 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -526,7 +526,7 @@ static int vme_user_master_mmap(unsigned int minor, struct
vm_area_struct *vma)
return err;
}
- vma_priv = kmalloc(sizeof(struct vme_user_vma_priv), GFP_KERNEL);
+ vma_priv = kmalloc(sizeof(*vma_priv), GFP_KERNEL);
if (!vma_priv) {
mutex_unlock(&image[minor].mutex);
return -ENOMEM;
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/9] staging: vme_user: fix code alignment
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 5ff44fb..285e00e 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -128,7 +128,7 @@ struct vme_user_vma_priv {
* transfer the data directly into the user space buffers.
*/
static ssize_t resource_to_user(int minor, char __user *buf, size_t count,
- loff_t *ppos)
+ loff_t *ppos)
{
ssize_t retval;
ssize_t copied = 0;
@@ -167,7 +167,7 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
* transfer the data directly from the user space buffers out to VME.
*/
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
ssize_t retval;
ssize_t copied = 0;
@@ -195,7 +195,7 @@ static ssize_t resource_from_user(unsigned int minor, const
char __user *buf,
}
static ssize_t buffer_to_user(unsigned int minor, char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
void *image_ptr;
ssize_t retval;
@@ -214,7 +214,7 @@ static ssize_t buffer_to_user(unsigned int minor, char
__user *buf,
}
static ssize_t buffer_from_user(unsigned int minor, const char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
void *image_ptr;
size_t retval;
@@ -233,7 +233,7 @@ static ssize_t buffer_from_user(unsigned int minor, const
char __user *buf,
}
static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count,
- loff_t *ppos)
+loff_t *ppos)
{
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
@@ -279,7 +279,7 @@ static ssize_t vme_user_read(struct file *file, char __user
*buf, size_t count,
}
static ssize_t vme_user_write(struct file *file, const char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
@@ -354,7 +354,7 @@ static loff_t vme_user_llseek(struct file *file, loff_t
off, int whence)
* already been defined.
*/
static int vme_user_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, unsigned long arg)
+ unsigned int cmd, unsigned long arg)
{
struct vme_master master;
struct vme_slave slave;
@@ -390,12 +390,13 @@ static int vme_user_ioctl(struct inode *inode, struct
file *file,
* to userspace as they are
*/
retval = vme_master_get(image[minor].resource,
- &master.enable, &master.vme_addr,
- &master.size, &master.aspace,
- &master.cycle, &master.dwidth);
+ &master.enable,
+ &master.vme_addr,
+ &master.size, &master.aspace,
+ &master.cycle, &master.dwidth);
copied = copy_to_user(argp, &master,
- sizeof(struct vme_master));
+ sizeof(struct vme_master));
if (copied != 0) {
pr_warn("Partial copy to userspace\n");
return -EFAULT;
@@ -435,12 +436,12 @@ static int vme_user_ioctl(struct inode *inode, struct
file *file,
* to userspace as they are
*/
retval = vme_slave_get(image[minor].resource,
- &slave.enable, &slave.vme_addr,
- &slave.size, &pci_addr, &slave.aspace,
- &slave.cycle);
+ &slave.enable, &slave.vme_addr,
+ &slave.size, &pci_addr,
+ &slave.aspace, &slave.cycle);
copied = copy_to_user(argp, &slave,
- sizeof(struct vme_slave));
+ sizeof(struct vme_slave));
if (copied != 0) {
pr_warn("Partial copy to user
[PATCH 0/9] vme_user checkpatch fixes and read()/write() rework
First four patches are fixes for various checpatch warnings. Next there is a change to drop large read()/write() stub followed by a change to rework user copy error codes. Last three changes are refactorings. Dmitry Kalinkin (9): staging: vme_user: fix code alignment staging: vme_user: fix blank lines staging: vme_user: fix NULL comparison style staging: vme_user: fix kmalloc style staging: vme_user: allow large read()/write() staging: vme_user: return -EFAULT on __copy_*_user errors staging: vme_user: remove unused variable staging: vme_user: remove distracting comment staging: vme_user: remove okcount variable drivers/staging/vme/devices/vme_user.c | 158 +++-- 1 file changed, 51 insertions(+), 107 deletions(-) -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 9/9] staging: vme_user: remove okcount variable
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 6f5bbc4..14f9554 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -188,7 +188,6 @@ static ssize_t vme_user_read(struct file *file, char __user
*buf, size_t count,
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
size_t image_size;
- size_t okcount;
if (minor == CONTROL_MINOR)
return 0;
@@ -206,16 +205,14 @@ static ssize_t vme_user_read(struct file *file, char
__user *buf, size_t count,
/* Ensure not reading past end of the image */
if (*ppos + count > image_size)
- okcount = image_size - *ppos;
- else
- okcount = count;
+ count = image_size - *ppos;
switch (type[minor]) {
case MASTER_MINOR:
- retval = resource_to_user(minor, buf, okcount, ppos);
+ retval = resource_to_user(minor, buf, count, ppos);
break;
case SLAVE_MINOR:
- retval = buffer_to_user(minor, buf, okcount, ppos);
+ retval = buffer_to_user(minor, buf, count, ppos);
break;
default:
retval = -EINVAL;
@@ -234,7 +231,6 @@ static ssize_t vme_user_write(struct file *file, const char
__user *buf,
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
size_t image_size;
- size_t okcount;
if (minor == CONTROL_MINOR)
return 0;
@@ -251,16 +247,14 @@ static ssize_t vme_user_write(struct file *file, const
char __user *buf,
/* Ensure not reading past end of the image */
if (*ppos + count > image_size)
- okcount = image_size - *ppos;
- else
- okcount = count;
+ count = image_size - *ppos;
switch (type[minor]) {
case MASTER_MINOR:
- retval = resource_from_user(minor, buf, okcount, ppos);
+ retval = resource_from_user(minor, buf, count, ppos);
break;
case SLAVE_MINOR:
- retval = buffer_from_user(minor, buf, okcount, ppos);
+ retval = buffer_from_user(minor, buf, count, ppos);
break;
default:
retval = -EINVAL;
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/9] staging: vme_user: fix blank lines
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 285e00e..1f00ad7 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -101,13 +101,13 @@ struct image_desc {
struct vme_resource *resource; /* VME resource */
int mmap_count; /* Number of current mmap's */
};
+
static struct image_desc image[VME_DEVS];
static struct cdev *vme_user_cdev; /* Character device */
static struct class *vme_user_sysfs_class; /* Sysfs class */
static struct vme_dev *vme_user_bridge;/* Pointer to user
device */
-
static const int type[VME_DEVS] = {MASTER_MINOR, MASTER_MINOR,
MASTER_MINOR, MASTER_MINOR,
SLAVE_MINOR,SLAVE_MINOR,
@@ -120,7 +120,6 @@ struct vme_user_vma_priv {
atomic_t refcnt;
};
-
/*
* We are going ot alloc a page during init per window for small transfers.
* Small transfers will go VME -> buffer -> user space. Larger (more than a
@@ -836,7 +835,6 @@ static void __exit vme_user_exit(void)
vme_unregister_driver(&vme_user_driver);
}
-
MODULE_PARM_DESC(bus, "Enumeration of VMEbus to which the driver is
connected");
module_param_array(bus, int, &bus_num, 0);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/9] staging: vme_user: fix NULL comparison style
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 1f00ad7..b6d81e5 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -527,7 +527,7 @@ static int vme_user_master_mmap(unsigned int minor, struct
vm_area_struct *vma)
}
vma_priv = kmalloc(sizeof(struct vme_user_vma_priv), GFP_KERNEL);
- if (vma_priv == NULL) {
+ if (!vma_priv) {
mutex_unlock(&image[minor].mutex);
return -ENOMEM;
}
@@ -588,7 +588,7 @@ static int vme_user_probe(struct vme_dev *vdev)
char *name;
/* Save pointer to the bridge device */
- if (vme_user_bridge != NULL) {
+ if (vme_user_bridge) {
dev_err(&vdev->dev, "Driver can only be loaded for 1 device\n");
err = -EINVAL;
goto err_dev;
@@ -636,7 +636,7 @@ static int vme_user_probe(struct vme_dev *vdev)
*/
image[i].resource = vme_slave_request(vme_user_bridge,
VME_A24, VME_SCT);
- if (image[i].resource == NULL) {
+ if (!image[i].resource) {
dev_warn(&vdev->dev,
"Unable to allocate slave resource\n");
err = -ENOMEM;
@@ -645,7 +645,7 @@ static int vme_user_probe(struct vme_dev *vdev)
image[i].size_buf = PCI_BUF_SIZE;
image[i].kern_buf = vme_alloc_consistent(image[i].resource,
image[i].size_buf, &image[i].pci_buf);
- if (image[i].kern_buf == NULL) {
+ if (!image[i].kern_buf) {
dev_warn(&vdev->dev,
"Unable to allocate memory for buffer\n");
image[i].pci_buf = 0;
@@ -663,7 +663,7 @@ static int vme_user_probe(struct vme_dev *vdev)
/* XXX Need to properly request attributes */
image[i].resource = vme_master_request(vme_user_bridge,
VME_A32, VME_SCT, VME_D32);
- if (image[i].resource == NULL) {
+ if (!image[i].resource) {
dev_warn(&vdev->dev,
"Unable to allocate master resource\n");
err = -ENOMEM;
@@ -671,7 +671,7 @@ static int vme_user_probe(struct vme_dev *vdev)
}
image[i].size_buf = PCI_BUF_SIZE;
image[i].kern_buf = kmalloc(image[i].size_buf, GFP_KERNEL);
- if (image[i].kern_buf == NULL) {
+ if (!image[i].kern_buf) {
err = -ENOMEM;
vme_master_free(image[i].resource);
goto err_master;
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/9] staging: vme_user: return -EFAULT on __copy_*_user errors
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 47 --
1 file changed, 11 insertions(+), 36 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 52cd638..85eb6ee 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -129,7 +129,6 @@ struct vme_user_vma_priv {
static ssize_t resource_to_user(int minor, char __user *buf, size_t count,
loff_t *ppos)
{
- ssize_t retval;
ssize_t copied = 0;
if (count > image[minor].size_buf)
@@ -141,13 +140,8 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
if (copied < 0)
return (int)copied;
- retval = __copy_to_user(buf, image[minor].kern_buf,
- (unsigned long)copied);
- if (retval != 0) {
- copied = (copied - retval);
- pr_info("User copy failed\n");
- return -EINVAL;
- }
+ if (__copy_to_user(buf, image[minor].kern_buf, (unsigned long)copied))
+ return -EFAULT;
return copied;
}
@@ -155,21 +149,16 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
- ssize_t retval;
ssize_t copied = 0;
if (count > image[minor].size_buf)
count = image[minor].size_buf;
- retval = __copy_from_user(image[minor].kern_buf, buf,
- (unsigned long)count);
- if (retval != 0)
- copied = (copied - retval);
- else
- copied = count;
+ if (__copy_from_user(image[minor].kern_buf, buf, (unsigned long)count))
+ return -EFAULT;
copied = vme_master_write(image[minor].resource, image[minor].kern_buf,
- copied, *ppos);
+ count, *ppos);
return copied;
}
@@ -178,38 +167,24 @@ static ssize_t buffer_to_user(unsigned int minor, char
__user *buf,
size_t count, loff_t *ppos)
{
void *image_ptr;
- ssize_t retval;
image_ptr = image[minor].kern_buf + *ppos;
+ if (__copy_to_user(buf, image_ptr, (unsigned long)count))
+ return -EINVAL;
- retval = __copy_to_user(buf, image_ptr, (unsigned long)count);
- if (retval != 0) {
- retval = (count - retval);
- pr_warn("Partial copy to userspace\n");
- } else
- retval = count;
-
- /* Return number of bytes successfully read */
- return retval;
+ return count;
}
static ssize_t buffer_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
void *image_ptr;
- size_t retval;
image_ptr = image[minor].kern_buf + *ppos;
+ if (__copy_from_user(image_ptr, buf, (unsigned long)count))
+ return -EINVAL;
- retval = __copy_from_user(image_ptr, buf, (unsigned long)count);
- if (retval != 0) {
- retval = (count - retval);
- pr_warn("Partial copy to userspace\n");
- } else
- retval = count;
-
- /* Return number of bytes successfully read */
- return retval;
+ return count;
}
static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count,
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 8/9] staging: vme_user: remove distracting comment
Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index 28a70f4..6f5bbc4 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -134,7 +134,6 @@ static ssize_t resource_to_user(int minor, char __user *buf, size_t count, if (count > image[minor].size_buf) count = image[minor].size_buf; - /* We copy to kernel buffer */ copied = vme_master_read(image[minor].resource, image[minor].kern_buf, count, *ppos); if (copied < 0) -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/9] staging: vme_user: remove unused variable
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 85eb6ee..28a70f4 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -149,18 +149,14 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
- ssize_t copied = 0;
-
if (count > image[minor].size_buf)
count = image[minor].size_buf;
if (__copy_from_user(image[minor].kern_buf, buf, (unsigned long)count))
return -EFAULT;
- copied = vme_master_write(image[minor].resource, image[minor].kern_buf,
- count, *ppos);
-
- return copied;
+ return vme_master_write(image[minor].resource, image[minor].kern_buf,
+ count, *ppos);
}
static ssize_t buffer_to_user(unsigned int minor, char __user *buf,
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/9] staging: vme_user: fix code alignment
> On 23 Jun 2015, at 16:21, Frans Klaver wrote: > > You left one in the function declarations (vme_user_write). If you mean forward declarations, they are already gone in Greg’s tree: https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/commit/drivers/staging/vme/devices/vme_user.c?h=staging-testing&id=e4aea6aa03267b496c21abefe170bb0d77192882 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/9] staging: vme_user: return -EFAULT on __copy_*_user errors
> On 23 Jun 2015, at 16:51, Dan Carpenter wrote:
>
> On Tue, Jun 23, 2015 at 03:42:30PM +0300, Dmitry Kalinkin wrote:
>> @@ -178,38 +167,24 @@ static ssize_t buffer_to_user(unsigned int minor, char
>> __user *buf,
>>size_t count, loff_t *ppos)
>> {
>> void *image_ptr;
>> -ssize_t retval;
>>
>> image_ptr = image[minor].kern_buf + *ppos;
>> +if (__copy_to_user(buf, image_ptr, (unsigned long)count))
>> +return -EINVAL;
>
> s/EINVAL/EFAULT/
Right. Will fix in v2.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 1/9] staging: vme_user: fix code alignment
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 9cca97a..ccf9602 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -128,7 +128,7 @@ struct vme_user_vma_priv {
* transfer the data directly into the user space buffers.
*/
static ssize_t resource_to_user(int minor, char __user *buf, size_t count,
- loff_t *ppos)
+ loff_t *ppos)
{
ssize_t retval;
ssize_t copied = 0;
@@ -167,7 +167,7 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
* transfer the data directly from the user space buffers out to VME.
*/
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
ssize_t retval;
ssize_t copied = 0;
@@ -195,7 +195,7 @@ static ssize_t resource_from_user(unsigned int minor, const
char __user *buf,
}
static ssize_t buffer_to_user(unsigned int minor, char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
void *image_ptr;
ssize_t retval;
@@ -214,7 +214,7 @@ static ssize_t buffer_to_user(unsigned int minor, char
__user *buf,
}
static ssize_t buffer_from_user(unsigned int minor, const char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
void *image_ptr;
size_t retval;
@@ -233,7 +233,7 @@ static ssize_t buffer_from_user(unsigned int minor, const
char __user *buf,
}
static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count,
- loff_t *ppos)
+loff_t *ppos)
{
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
@@ -279,7 +279,7 @@ static ssize_t vme_user_read(struct file *file, char __user
*buf, size_t count,
}
static ssize_t vme_user_write(struct file *file, const char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
@@ -354,7 +354,7 @@ static loff_t vme_user_llseek(struct file *file, loff_t
off, int whence)
* already been defined.
*/
static int vme_user_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, unsigned long arg)
+ unsigned int cmd, unsigned long arg)
{
struct vme_master master;
struct vme_slave slave;
@@ -390,12 +390,13 @@ static int vme_user_ioctl(struct inode *inode, struct
file *file,
* to userspace as they are
*/
retval = vme_master_get(image[minor].resource,
- &master.enable, &master.vme_addr,
- &master.size, &master.aspace,
- &master.cycle, &master.dwidth);
+ &master.enable,
+ &master.vme_addr,
+ &master.size, &master.aspace,
+ &master.cycle, &master.dwidth);
copied = copy_to_user(argp, &master,
- sizeof(struct vme_master));
+ sizeof(struct vme_master));
if (copied != 0) {
pr_warn("Partial copy to userspace\n");
return -EFAULT;
@@ -435,12 +436,12 @@ static int vme_user_ioctl(struct inode *inode, struct
file *file,
* to userspace as they are
*/
retval = vme_slave_get(image[minor].resource,
- &slave.enable, &slave.vme_addr,
- &slave.size, &pci_addr, &slave.aspace,
- &slave.cycle);
+ &slave.enable, &slave.vme_addr,
+ &slave.size, &pci_addr,
+ &slave.aspace, &slave.cycle);
copied = copy_to_user(argp, &slave,
- sizeof(struct vme_slave));
+ sizeof(struct vme_slave));
if (copied != 0) {
pr_warn("Partial copy to user
[PATCHv2 2/9] staging: vme_user: fix blank lines
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index ccf9602..494655a 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -101,13 +101,13 @@ struct image_desc {
struct vme_resource *resource; /* VME resource */
int mmap_count; /* Number of current mmap's */
};
+
static struct image_desc image[VME_DEVS];
static struct cdev *vme_user_cdev; /* Character device */
static struct class *vme_user_sysfs_class; /* Sysfs class */
static struct vme_dev *vme_user_bridge;/* Pointer to user
device */
-
static const int type[VME_DEVS] = {MASTER_MINOR, MASTER_MINOR,
MASTER_MINOR, MASTER_MINOR,
SLAVE_MINOR,SLAVE_MINOR,
@@ -120,7 +120,6 @@ struct vme_user_vma_priv {
atomic_t refcnt;
};
-
/*
* We are going ot alloc a page during init per window for small transfers.
* Small transfers will go VME -> buffer -> user space. Larger (more than a
@@ -836,7 +835,6 @@ static void __exit vme_user_exit(void)
vme_unregister_driver(&vme_user_driver);
}
-
MODULE_PARM_DESC(bus, "Enumeration of VMEbus to which the driver is
connected");
module_param_array(bus, int, &bus_num, 0);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 0/9] vme_user checkpatch fixes and read()/write() rework
First four patches are fixes for various checpatch warnings. Next there is a
change to drop large read()/write() stub followed by a change to rework user
copy error codes. Last three changes are refactorings.
v2 fixes ("vme_user: return -EFAULT on __copy_*_user errors") that had EINVAL
instead of EFAULT in a couple of places.
Dmitry Kalinkin (9):
staging: vme_user: fix code alignment
staging: vme_user: fix blank lines
staging: vme_user: fix NULL comparison style
staging: vme_user: fix kmalloc style
staging: vme_user: allow large read()/write()
staging: vme_user: return -EFAULT on __copy_*_user errors
staging: vme_user: remove unused variable
staging: vme_user: remove distracting comment
staging: vme_user: remove okcount variable
drivers/staging/vme/devices/vme_user.c | 158 +++--
1 file changed, 51 insertions(+), 107 deletions(-)
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 3/9] staging: vme_user: fix NULL comparison style
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 494655a..2ff15f0 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -527,7 +527,7 @@ static int vme_user_master_mmap(unsigned int minor, struct
vm_area_struct *vma)
}
vma_priv = kmalloc(sizeof(struct vme_user_vma_priv), GFP_KERNEL);
- if (vma_priv == NULL) {
+ if (!vma_priv) {
mutex_unlock(&image[minor].mutex);
return -ENOMEM;
}
@@ -588,7 +588,7 @@ static int vme_user_probe(struct vme_dev *vdev)
char *name;
/* Save pointer to the bridge device */
- if (vme_user_bridge != NULL) {
+ if (vme_user_bridge) {
dev_err(&vdev->dev, "Driver can only be loaded for 1 device\n");
err = -EINVAL;
goto err_dev;
@@ -636,7 +636,7 @@ static int vme_user_probe(struct vme_dev *vdev)
*/
image[i].resource = vme_slave_request(vme_user_bridge,
VME_A24, VME_SCT);
- if (image[i].resource == NULL) {
+ if (!image[i].resource) {
dev_warn(&vdev->dev,
"Unable to allocate slave resource\n");
err = -ENOMEM;
@@ -645,7 +645,7 @@ static int vme_user_probe(struct vme_dev *vdev)
image[i].size_buf = PCI_BUF_SIZE;
image[i].kern_buf = vme_alloc_consistent(image[i].resource,
image[i].size_buf, &image[i].pci_buf);
- if (image[i].kern_buf == NULL) {
+ if (!image[i].kern_buf) {
dev_warn(&vdev->dev,
"Unable to allocate memory for buffer\n");
image[i].pci_buf = 0;
@@ -663,7 +663,7 @@ static int vme_user_probe(struct vme_dev *vdev)
/* XXX Need to properly request attributes */
image[i].resource = vme_master_request(vme_user_bridge,
VME_A32, VME_SCT, VME_D32);
- if (image[i].resource == NULL) {
+ if (!image[i].resource) {
dev_warn(&vdev->dev,
"Unable to allocate master resource\n");
err = -ENOMEM;
@@ -671,7 +671,7 @@ static int vme_user_probe(struct vme_dev *vdev)
}
image[i].size_buf = PCI_BUF_SIZE;
image[i].kern_buf = kmalloc(image[i].size_buf, GFP_KERNEL);
- if (image[i].kern_buf == NULL) {
+ if (!image[i].kern_buf) {
err = -ENOMEM;
vme_master_free(image[i].resource);
goto err_master;
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 8/9] staging: vme_user: remove distracting comment
Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index cf47649..de9eda5 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -134,7 +134,6 @@ static ssize_t resource_to_user(int minor, char __user *buf, size_t count, if (count > image[minor].size_buf) count = image[minor].size_buf; - /* We copy to kernel buffer */ copied = vme_master_read(image[minor].resource, image[minor].kern_buf, count, *ppos); if (copied < 0) -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 5/9] staging: vme_user: allow large read()/write()
This changes large master transfers to do shorter read/write rather than
return -EINVAL. User space will now be able to optimistically request a
large transfer and get at least some data.
This also removes comments suggesting on how to implement large
transfers. Current vme_master_* read and write implementations use CPU
copies that don't produce burst PCI accesses and subsequently no block
transfer on VME bus. In the end overall performance is quiet low and it
can't be fixed by doing direct copy to user space. Much easier solution
would be to just reuse kernel buffer.
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 67 --
1 file changed, 24 insertions(+), 43 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 3467cde..101f7b9 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -132,63 +132,44 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
ssize_t retval;
ssize_t copied = 0;
- if (count <= image[minor].size_buf) {
- /* We copy to kernel buffer */
- copied = vme_master_read(image[minor].resource,
- image[minor].kern_buf, count, *ppos);
- if (copied < 0)
- return (int)copied;
-
- retval = __copy_to_user(buf, image[minor].kern_buf,
- (unsigned long)copied);
- if (retval != 0) {
- copied = (copied - retval);
- pr_info("User copy failed\n");
- return -EINVAL;
- }
+ if (count > image[minor].size_buf)
+ count = image[minor].size_buf;
- } else {
- /* XXX Need to write this */
- pr_info("Currently don't support large transfers\n");
- /* Map in pages from userspace */
+ /* We copy to kernel buffer */
+ copied = vme_master_read(image[minor].resource, image[minor].kern_buf,
+count, *ppos);
+ if (copied < 0)
+ return (int)copied;
- /* Call vme_master_read to do the transfer */
+ retval = __copy_to_user(buf, image[minor].kern_buf,
+ (unsigned long)copied);
+ if (retval != 0) {
+ copied = (copied - retval);
+ pr_info("User copy failed\n");
return -EINVAL;
}
return copied;
}
-/*
- * We are going to alloc a page during init per window for small transfers.
- * Small transfers will go user space -> buffer -> VME. Larger (more than a
- * page) transfers will lock the user space buffer into memory and then
- * transfer the data directly from the user space buffers out to VME.
- */
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
ssize_t retval;
ssize_t copied = 0;
- if (count <= image[minor].size_buf) {
- retval = __copy_from_user(image[minor].kern_buf, buf,
- (unsigned long)count);
- if (retval != 0)
- copied = (copied - retval);
- else
- copied = count;
-
- copied = vme_master_write(image[minor].resource,
- image[minor].kern_buf, copied, *ppos);
- } else {
- /* XXX Need to write this */
- pr_info("Currently don't support large transfers\n");
- /* Map in pages from userspace */
-
- /* Call vme_master_write to do the transfer */
- return -EINVAL;
- }
+ if (count > image[minor].size_buf)
+ count = image[minor].size_buf;
+
+ retval = __copy_from_user(image[minor].kern_buf, buf,
+ (unsigned long)count);
+ if (retval != 0)
+ copied = (copied - retval);
+ else
+ copied = count;
+
+ copied = vme_master_write(image[minor].resource, image[minor].kern_buf,
+ copied, *ppos);
return copied;
}
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 4/9] staging: vme_user: fix kmalloc style
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 2ff15f0..3467cde 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -526,7 +526,7 @@ static int vme_user_master_mmap(unsigned int minor, struct
vm_area_struct *vma)
return err;
}
- vma_priv = kmalloc(sizeof(struct vme_user_vma_priv), GFP_KERNEL);
+ vma_priv = kmalloc(sizeof(*vma_priv), GFP_KERNEL);
if (!vma_priv) {
mutex_unlock(&image[minor].mutex);
return -ENOMEM;
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 7/9] staging: vme_user: remove unused variable
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 070e63f..cf47649 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -149,18 +149,14 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
- ssize_t copied = 0;
-
if (count > image[minor].size_buf)
count = image[minor].size_buf;
if (__copy_from_user(image[minor].kern_buf, buf, (unsigned long)count))
return -EFAULT;
- copied = vme_master_write(image[minor].resource, image[minor].kern_buf,
- count, *ppos);
-
- return copied;
+ return vme_master_write(image[minor].resource, image[minor].kern_buf,
+ count, *ppos);
}
static ssize_t buffer_to_user(unsigned int minor, char __user *buf,
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 6/9] staging: vme_user: return -EFAULT on __copy_*_user errors
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 47 --
1 file changed, 11 insertions(+), 36 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 101f7b9..070e63f 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -129,7 +129,6 @@ struct vme_user_vma_priv {
static ssize_t resource_to_user(int minor, char __user *buf, size_t count,
loff_t *ppos)
{
- ssize_t retval;
ssize_t copied = 0;
if (count > image[minor].size_buf)
@@ -141,13 +140,8 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
if (copied < 0)
return (int)copied;
- retval = __copy_to_user(buf, image[minor].kern_buf,
- (unsigned long)copied);
- if (retval != 0) {
- copied = (copied - retval);
- pr_info("User copy failed\n");
- return -EINVAL;
- }
+ if (__copy_to_user(buf, image[minor].kern_buf, (unsigned long)copied))
+ return -EFAULT;
return copied;
}
@@ -155,21 +149,16 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
- ssize_t retval;
ssize_t copied = 0;
if (count > image[minor].size_buf)
count = image[minor].size_buf;
- retval = __copy_from_user(image[minor].kern_buf, buf,
- (unsigned long)count);
- if (retval != 0)
- copied = (copied - retval);
- else
- copied = count;
+ if (__copy_from_user(image[minor].kern_buf, buf, (unsigned long)count))
+ return -EFAULT;
copied = vme_master_write(image[minor].resource, image[minor].kern_buf,
- copied, *ppos);
+ count, *ppos);
return copied;
}
@@ -178,38 +167,24 @@ static ssize_t buffer_to_user(unsigned int minor, char
__user *buf,
size_t count, loff_t *ppos)
{
void *image_ptr;
- ssize_t retval;
image_ptr = image[minor].kern_buf + *ppos;
+ if (__copy_to_user(buf, image_ptr, (unsigned long)count))
+ return -EFAULT;
- retval = __copy_to_user(buf, image_ptr, (unsigned long)count);
- if (retval != 0) {
- retval = (count - retval);
- pr_warn("Partial copy to userspace\n");
- } else
- retval = count;
-
- /* Return number of bytes successfully read */
- return retval;
+ return count;
}
static ssize_t buffer_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
void *image_ptr;
- size_t retval;
image_ptr = image[minor].kern_buf + *ppos;
+ if (__copy_from_user(image_ptr, buf, (unsigned long)count))
+ return -EFAULT;
- retval = __copy_from_user(image_ptr, buf, (unsigned long)count);
- if (retval != 0) {
- retval = (count - retval);
- pr_warn("Partial copy to userspace\n");
- } else
- retval = count;
-
- /* Return number of bytes successfully read */
- return retval;
+ return count;
}
static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count,
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv2 9/9] staging: vme_user: remove okcount variable
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index de9eda5..efed9c7 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -188,7 +188,6 @@ static ssize_t vme_user_read(struct file *file, char __user
*buf, size_t count,
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
size_t image_size;
- size_t okcount;
if (minor == CONTROL_MINOR)
return 0;
@@ -206,16 +205,14 @@ static ssize_t vme_user_read(struct file *file, char
__user *buf, size_t count,
/* Ensure not reading past end of the image */
if (*ppos + count > image_size)
- okcount = image_size - *ppos;
- else
- okcount = count;
+ count = image_size - *ppos;
switch (type[minor]) {
case MASTER_MINOR:
- retval = resource_to_user(minor, buf, okcount, ppos);
+ retval = resource_to_user(minor, buf, count, ppos);
break;
case SLAVE_MINOR:
- retval = buffer_to_user(minor, buf, okcount, ppos);
+ retval = buffer_to_user(minor, buf, count, ppos);
break;
default:
retval = -EINVAL;
@@ -234,7 +231,6 @@ static ssize_t vme_user_write(struct file *file, const char
__user *buf,
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
size_t image_size;
- size_t okcount;
if (minor == CONTROL_MINOR)
return 0;
@@ -251,16 +247,14 @@ static ssize_t vme_user_write(struct file *file, const
char __user *buf,
/* Ensure not reading past end of the image */
if (*ppos + count > image_size)
- okcount = image_size - *ppos;
- else
- okcount = count;
+ count = image_size - *ppos;
switch (type[minor]) {
case MASTER_MINOR:
- retval = resource_from_user(minor, buf, okcount, ppos);
+ retval = resource_from_user(minor, buf, count, ppos);
break;
case SLAVE_MINOR:
- retval = buffer_from_user(minor, buf, okcount, ppos);
+ retval = buffer_from_user(minor, buf, count, ppos);
break;
default:
retval = -EINVAL;
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv2 6/9] staging: vme_user: return -EFAULT on __copy_*_user errors
> On 25 Jun 2015, at 14:27, Sudip Mukherjee wrote:
>
> On Tue, Jun 23, 2015 at 07:03:36PM +0300, Dmitry Kalinkin wrote:
>> Signed-off-by: Dmitry Kalinkin
>> ---
>> drivers/staging/vme/devices/vme_user.c | 47
>> --
>> 1 file changed, 11 insertions(+), 36 deletions(-)
>>
>
>> @@ -178,38 +167,24 @@ static ssize_t buffer_to_user(unsigned int minor, char
>> __user *buf,
>>size_t count, loff_t *ppos)
>> {
>> void *image_ptr;
>> -ssize_t retval;
>>
>> image_ptr = image[minor].kern_buf + *ppos;
>> +if (__copy_to_user(buf, image_ptr, (unsigned long)count))
>> +return -EFAULT;
>>
>> -retval = __copy_to_user(buf, image_ptr, (unsigned long)count);
>> -if (retval != 0) {
>> -retval = (count - retval);
>> -pr_warn("Partial copy to userspace\n");
>> -} else
>> -retval = count;
>> -
>> -/* Return number of bytes successfully read */
>> -return retval;
>> +return count;
> will it not affect the userspace code?
> previously number of bytes successfully read was returned, now incase of
> partial read -EFAULT is being returned.
Exactly.
Practically there is an access_ok() call in vfs_read() and vfs_write() that
will catch this first. I don’t know exactly what is the condition for
__copy_to_user to fail, but it is probably some rare arch-specific thing (and
we only care for x86/powerpc here). But when it happens it better be returning
proper error codes. This is why I think this is not a “we broke userspace”
situation.
Cheers,
Dmitry
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv2 6/9] staging: vme_user: return -EFAULT on __copy_*_user errors
> On 25 Jun 2015, at 15:05, Dmitry Kalinkin wrote:
>
>
>> On 25 Jun 2015, at 14:27, Sudip Mukherjee wrote:
>>
>> On Tue, Jun 23, 2015 at 07:03:36PM +0300, Dmitry Kalinkin wrote:
>>> Signed-off-by: Dmitry Kalinkin
>>> ---
>>> drivers/staging/vme/devices/vme_user.c | 47
>>> --
>>> 1 file changed, 11 insertions(+), 36 deletions(-)
>>>
>>
>>> @@ -178,38 +167,24 @@ static ssize_t buffer_to_user(unsigned int minor,
>>> char __user *buf,
>>> size_t count, loff_t *ppos)
>>> {
>>> void *image_ptr;
>>> - ssize_t retval;
>>>
>>> image_ptr = image[minor].kern_buf + *ppos;
>>> + if (__copy_to_user(buf, image_ptr, (unsigned long)count))
>>> + return -EFAULT;
>>>
>>> - retval = __copy_to_user(buf, image_ptr, (unsigned long)count);
>>> - if (retval != 0) {
>>> - retval = (count - retval);
>>> - pr_warn("Partial copy to userspace\n");
>>> - } else
>>> - retval = count;
>>> -
>>> - /* Return number of bytes successfully read */
>>> - return retval;
>>> + return count;
>> will it not affect the userspace code?
>> previously number of bytes successfully read was returned, now incase of
>> partial read -EFAULT is being returned.
> Exactly.
>
> Practically there is an access_ok() call in vfs_read() and vfs_write() that
> will catch this first. I don’t know exactly what is the condition for
> __copy_to_user to fail, but it is probably some rare arch-specific thing (and
> we only care for x86/powerpc here). But when it happens it better be returning
> proper error codes. This is why I think this is not a “we broke userspace”
> situation.
It seems like what I wrote above is not correct. access_ok does only coarse
checks and __copy_to_user does fail. Anyway, only a rare userspace application
would depend on “succesfull” read that was interrupted by a segfault. Also, if
__copy_to_user fails completely, the original code would return zero, which in
POSIX should mean something like “everything is good, try again later” and
this may cause infinite loops (e.g. python).
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 7/9] staging: vme_user: remove unused variable
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index ef876a4..947a38e 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -143,18 +143,14 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
- ssize_t copied = 0;
-
if (count > image[minor].size_buf)
count = image[minor].size_buf;
if (__copy_from_user(image[minor].kern_buf, buf, (unsigned long)count))
return -EFAULT;
- copied = vme_master_write(image[minor].resource, image[minor].kern_buf,
- count, *ppos);
-
- return copied;
+ return vme_master_write(image[minor].resource, image[minor].kern_buf,
+ count, *ppos);
}
static ssize_t buffer_to_user(unsigned int minor, char __user *buf,
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 3/9] staging: vme_user: fix NULL comparison style
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 494655a..2ff15f0 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -527,7 +527,7 @@ static int vme_user_master_mmap(unsigned int minor, struct
vm_area_struct *vma)
}
vma_priv = kmalloc(sizeof(struct vme_user_vma_priv), GFP_KERNEL);
- if (vma_priv == NULL) {
+ if (!vma_priv) {
mutex_unlock(&image[minor].mutex);
return -ENOMEM;
}
@@ -588,7 +588,7 @@ static int vme_user_probe(struct vme_dev *vdev)
char *name;
/* Save pointer to the bridge device */
- if (vme_user_bridge != NULL) {
+ if (vme_user_bridge) {
dev_err(&vdev->dev, "Driver can only be loaded for 1 device\n");
err = -EINVAL;
goto err_dev;
@@ -636,7 +636,7 @@ static int vme_user_probe(struct vme_dev *vdev)
*/
image[i].resource = vme_slave_request(vme_user_bridge,
VME_A24, VME_SCT);
- if (image[i].resource == NULL) {
+ if (!image[i].resource) {
dev_warn(&vdev->dev,
"Unable to allocate slave resource\n");
err = -ENOMEM;
@@ -645,7 +645,7 @@ static int vme_user_probe(struct vme_dev *vdev)
image[i].size_buf = PCI_BUF_SIZE;
image[i].kern_buf = vme_alloc_consistent(image[i].resource,
image[i].size_buf, &image[i].pci_buf);
- if (image[i].kern_buf == NULL) {
+ if (!image[i].kern_buf) {
dev_warn(&vdev->dev,
"Unable to allocate memory for buffer\n");
image[i].pci_buf = 0;
@@ -663,7 +663,7 @@ static int vme_user_probe(struct vme_dev *vdev)
/* XXX Need to properly request attributes */
image[i].resource = vme_master_request(vme_user_bridge,
VME_A32, VME_SCT, VME_D32);
- if (image[i].resource == NULL) {
+ if (!image[i].resource) {
dev_warn(&vdev->dev,
"Unable to allocate master resource\n");
err = -ENOMEM;
@@ -671,7 +671,7 @@ static int vme_user_probe(struct vme_dev *vdev)
}
image[i].size_buf = PCI_BUF_SIZE;
image[i].kern_buf = kmalloc(image[i].size_buf, GFP_KERNEL);
- if (image[i].kern_buf == NULL) {
+ if (!image[i].kern_buf) {
err = -ENOMEM;
vme_master_free(image[i].resource);
goto err_master;
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 0/9] vme_user checkpatch fixes and read()/write() rework
First four patches are fixes for various checpatch warnings. Next there is a
change to drop large read()/write() stub followed by a change to rework user
copy error codes. Last three changes are refactorings.
v2 fixes ("vme_user: return -EFAULT on __copy_*_user errors") that had EINVAL
instead of EFAULT in a couple of places.
v3 fixes ("allow large read()/write()") to also remove the comment right above
resource_to_user()
v3 also renames ("vme_user: return -EFAULT on __copy_*_user errors") to
("switch to returning -EFAULT on __copy_*_user errors")
Dmitry Kalinkin (9):
staging: vme_user: fix code alignment
staging: vme_user: fix blank lines
staging: vme_user: fix NULL comparison style
staging: vme_user: fix kmalloc style
staging: vme_user: allow large read()/write()
staging: vme_user: switch to returning -EFAULT on __copy_*_user errors
staging: vme_user: remove unused variable
staging: vme_user: remove distracting comment
staging: vme_user: remove okcount variable
drivers/staging/vme/devices/vme_user.c | 164 ++---
1 file changed, 51 insertions(+), 113 deletions(-)
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 9/9] staging: vme_user: remove okcount variable
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 18 ++
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 7ca943c..b3e3c2d 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -182,7 +182,6 @@ static ssize_t vme_user_read(struct file *file, char __user
*buf, size_t count,
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
size_t image_size;
- size_t okcount;
if (minor == CONTROL_MINOR)
return 0;
@@ -200,16 +199,14 @@ static ssize_t vme_user_read(struct file *file, char
__user *buf, size_t count,
/* Ensure not reading past end of the image */
if (*ppos + count > image_size)
- okcount = image_size - *ppos;
- else
- okcount = count;
+ count = image_size - *ppos;
switch (type[minor]) {
case MASTER_MINOR:
- retval = resource_to_user(minor, buf, okcount, ppos);
+ retval = resource_to_user(minor, buf, count, ppos);
break;
case SLAVE_MINOR:
- retval = buffer_to_user(minor, buf, okcount, ppos);
+ retval = buffer_to_user(minor, buf, count, ppos);
break;
default:
retval = -EINVAL;
@@ -228,7 +225,6 @@ static ssize_t vme_user_write(struct file *file, const char
__user *buf,
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
size_t image_size;
- size_t okcount;
if (minor == CONTROL_MINOR)
return 0;
@@ -245,16 +241,14 @@ static ssize_t vme_user_write(struct file *file, const
char __user *buf,
/* Ensure not reading past end of the image */
if (*ppos + count > image_size)
- okcount = image_size - *ppos;
- else
- okcount = count;
+ count = image_size - *ppos;
switch (type[minor]) {
case MASTER_MINOR:
- retval = resource_from_user(minor, buf, okcount, ppos);
+ retval = resource_from_user(minor, buf, count, ppos);
break;
case SLAVE_MINOR:
- retval = buffer_from_user(minor, buf, okcount, ppos);
+ retval = buffer_from_user(minor, buf, count, ppos);
break;
default:
retval = -EINVAL;
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 1/9] staging: vme_user: fix code alignment
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 9cca97a..ccf9602 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -128,7 +128,7 @@ struct vme_user_vma_priv {
* transfer the data directly into the user space buffers.
*/
static ssize_t resource_to_user(int minor, char __user *buf, size_t count,
- loff_t *ppos)
+ loff_t *ppos)
{
ssize_t retval;
ssize_t copied = 0;
@@ -167,7 +167,7 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
* transfer the data directly from the user space buffers out to VME.
*/
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
ssize_t retval;
ssize_t copied = 0;
@@ -195,7 +195,7 @@ static ssize_t resource_from_user(unsigned int minor, const
char __user *buf,
}
static ssize_t buffer_to_user(unsigned int minor, char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
void *image_ptr;
ssize_t retval;
@@ -214,7 +214,7 @@ static ssize_t buffer_to_user(unsigned int minor, char
__user *buf,
}
static ssize_t buffer_from_user(unsigned int minor, const char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
void *image_ptr;
size_t retval;
@@ -233,7 +233,7 @@ static ssize_t buffer_from_user(unsigned int minor, const
char __user *buf,
}
static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count,
- loff_t *ppos)
+loff_t *ppos)
{
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
@@ -279,7 +279,7 @@ static ssize_t vme_user_read(struct file *file, char __user
*buf, size_t count,
}
static ssize_t vme_user_write(struct file *file, const char __user *buf,
- size_t count, loff_t *ppos)
+ size_t count, loff_t *ppos)
{
unsigned int minor = MINOR(file_inode(file)->i_rdev);
ssize_t retval;
@@ -354,7 +354,7 @@ static loff_t vme_user_llseek(struct file *file, loff_t
off, int whence)
* already been defined.
*/
static int vme_user_ioctl(struct inode *inode, struct file *file,
- unsigned int cmd, unsigned long arg)
+ unsigned int cmd, unsigned long arg)
{
struct vme_master master;
struct vme_slave slave;
@@ -390,12 +390,13 @@ static int vme_user_ioctl(struct inode *inode, struct
file *file,
* to userspace as they are
*/
retval = vme_master_get(image[minor].resource,
- &master.enable, &master.vme_addr,
- &master.size, &master.aspace,
- &master.cycle, &master.dwidth);
+ &master.enable,
+ &master.vme_addr,
+ &master.size, &master.aspace,
+ &master.cycle, &master.dwidth);
copied = copy_to_user(argp, &master,
- sizeof(struct vme_master));
+ sizeof(struct vme_master));
if (copied != 0) {
pr_warn("Partial copy to userspace\n");
return -EFAULT;
@@ -435,12 +436,12 @@ static int vme_user_ioctl(struct inode *inode, struct
file *file,
* to userspace as they are
*/
retval = vme_slave_get(image[minor].resource,
- &slave.enable, &slave.vme_addr,
- &slave.size, &pci_addr, &slave.aspace,
- &slave.cycle);
+ &slave.enable, &slave.vme_addr,
+ &slave.size, &pci_addr,
+ &slave.aspace, &slave.cycle);
copied = copy_to_user(argp, &slave,
- sizeof(struct vme_slave));
+ sizeof(struct vme_slave));
if (copied != 0) {
pr_warn("Partial copy to user
[PATCHv3 5/9] staging: vme_user: allow large read()/write()
This changes large master transfers to do shorter read/write rather than
return -EINVAL. User space will now be able to optimistically request a
large transfer and get at least some data.
This also removes comments suggesting on how to implement large
transfers. Current vme_master_* read and write implementations use CPU
copies that don't produce burst PCI accesses and subsequently no block
transfer on VME bus. In the end overall performance is quiet low and it
can't be fixed by doing direct copy to user space. Much easier solution
would be to just reuse kernel buffer.
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 73 +++---
1 file changed, 24 insertions(+), 49 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 3467cde..a2345db 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -120,75 +120,50 @@ struct vme_user_vma_priv {
atomic_t refcnt;
};
-/*
- * We are going ot alloc a page during init per window for small transfers.
- * Small transfers will go VME -> buffer -> user space. Larger (more than a
- * page) transfers will lock the user space buffer into memory and then
- * transfer the data directly into the user space buffers.
- */
static ssize_t resource_to_user(int minor, char __user *buf, size_t count,
loff_t *ppos)
{
ssize_t retval;
ssize_t copied = 0;
- if (count <= image[minor].size_buf) {
- /* We copy to kernel buffer */
- copied = vme_master_read(image[minor].resource,
- image[minor].kern_buf, count, *ppos);
- if (copied < 0)
- return (int)copied;
-
- retval = __copy_to_user(buf, image[minor].kern_buf,
- (unsigned long)copied);
- if (retval != 0) {
- copied = (copied - retval);
- pr_info("User copy failed\n");
- return -EINVAL;
- }
+ if (count > image[minor].size_buf)
+ count = image[minor].size_buf;
- } else {
- /* XXX Need to write this */
- pr_info("Currently don't support large transfers\n");
- /* Map in pages from userspace */
+ /* We copy to kernel buffer */
+ copied = vme_master_read(image[minor].resource, image[minor].kern_buf,
+count, *ppos);
+ if (copied < 0)
+ return (int)copied;
- /* Call vme_master_read to do the transfer */
+ retval = __copy_to_user(buf, image[minor].kern_buf,
+ (unsigned long)copied);
+ if (retval != 0) {
+ copied = (copied - retval);
+ pr_info("User copy failed\n");
return -EINVAL;
}
return copied;
}
-/*
- * We are going to alloc a page during init per window for small transfers.
- * Small transfers will go user space -> buffer -> VME. Larger (more than a
- * page) transfers will lock the user space buffer into memory and then
- * transfer the data directly from the user space buffers out to VME.
- */
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
ssize_t retval;
ssize_t copied = 0;
- if (count <= image[minor].size_buf) {
- retval = __copy_from_user(image[minor].kern_buf, buf,
- (unsigned long)count);
- if (retval != 0)
- copied = (copied - retval);
- else
- copied = count;
-
- copied = vme_master_write(image[minor].resource,
- image[minor].kern_buf, copied, *ppos);
- } else {
- /* XXX Need to write this */
- pr_info("Currently don't support large transfers\n");
- /* Map in pages from userspace */
-
- /* Call vme_master_write to do the transfer */
- return -EINVAL;
- }
+ if (count > image[minor].size_buf)
+ count = image[minor].size_buf;
+
+ retval = __copy_from_user(image[minor].kern_buf, buf,
+ (unsigned long)count);
+ if (retval != 0)
+ copied = (copied - retval);
+ else
+ copied = count;
+
+ copied = vme_master_write(image[minor].resource, image[minor].kern_buf,
+ copied, *ppos);
return copied;
}
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 8/9] staging: vme_user: remove distracting comment
Signed-off-by: Dmitry Kalinkin --- drivers/staging/vme/devices/vme_user.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index 947a38e..7ca943c 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -128,7 +128,6 @@ static ssize_t resource_to_user(int minor, char __user *buf, size_t count, if (count > image[minor].size_buf) count = image[minor].size_buf; - /* We copy to kernel buffer */ copied = vme_master_read(image[minor].resource, image[minor].kern_buf, count, *ppos); if (copied < 0) -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 4/9] staging: vme_user: fix kmalloc style
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index 2ff15f0..3467cde 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -526,7 +526,7 @@ static int vme_user_master_mmap(unsigned int minor, struct
vm_area_struct *vma)
return err;
}
- vma_priv = kmalloc(sizeof(struct vme_user_vma_priv), GFP_KERNEL);
+ vma_priv = kmalloc(sizeof(*vma_priv), GFP_KERNEL);
if (!vma_priv) {
mutex_unlock(&image[minor].mutex);
return -ENOMEM;
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 2/9] staging: vme_user: fix blank lines
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index ccf9602..494655a 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -101,13 +101,13 @@ struct image_desc {
struct vme_resource *resource; /* VME resource */
int mmap_count; /* Number of current mmap's */
};
+
static struct image_desc image[VME_DEVS];
static struct cdev *vme_user_cdev; /* Character device */
static struct class *vme_user_sysfs_class; /* Sysfs class */
static struct vme_dev *vme_user_bridge;/* Pointer to user
device */
-
static const int type[VME_DEVS] = {MASTER_MINOR, MASTER_MINOR,
MASTER_MINOR, MASTER_MINOR,
SLAVE_MINOR,SLAVE_MINOR,
@@ -120,7 +120,6 @@ struct vme_user_vma_priv {
atomic_t refcnt;
};
-
/*
* We are going ot alloc a page during init per window for small transfers.
* Small transfers will go VME -> buffer -> user space. Larger (more than a
@@ -836,7 +835,6 @@ static void __exit vme_user_exit(void)
vme_unregister_driver(&vme_user_driver);
}
-
MODULE_PARM_DESC(bus, "Enumeration of VMEbus to which the driver is
connected");
module_param_array(bus, int, &bus_num, 0);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv3 6/9] staging: vme_user: switch to returning -EFAULT on __copy_*_user errors
Signed-off-by: Dmitry Kalinkin
---
drivers/staging/vme/devices/vme_user.c | 47 --
1 file changed, 11 insertions(+), 36 deletions(-)
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index a2345db..ef876a4 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -123,7 +123,6 @@ struct vme_user_vma_priv {
static ssize_t resource_to_user(int minor, char __user *buf, size_t count,
loff_t *ppos)
{
- ssize_t retval;
ssize_t copied = 0;
if (count > image[minor].size_buf)
@@ -135,13 +134,8 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
if (copied < 0)
return (int)copied;
- retval = __copy_to_user(buf, image[minor].kern_buf,
- (unsigned long)copied);
- if (retval != 0) {
- copied = (copied - retval);
- pr_info("User copy failed\n");
- return -EINVAL;
- }
+ if (__copy_to_user(buf, image[minor].kern_buf, (unsigned long)copied))
+ return -EFAULT;
return copied;
}
@@ -149,21 +143,16 @@ static ssize_t resource_to_user(int minor, char __user
*buf, size_t count,
static ssize_t resource_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
- ssize_t retval;
ssize_t copied = 0;
if (count > image[minor].size_buf)
count = image[minor].size_buf;
- retval = __copy_from_user(image[minor].kern_buf, buf,
- (unsigned long)count);
- if (retval != 0)
- copied = (copied - retval);
- else
- copied = count;
+ if (__copy_from_user(image[minor].kern_buf, buf, (unsigned long)count))
+ return -EFAULT;
copied = vme_master_write(image[minor].resource, image[minor].kern_buf,
- copied, *ppos);
+ count, *ppos);
return copied;
}
@@ -172,38 +161,24 @@ static ssize_t buffer_to_user(unsigned int minor, char
__user *buf,
size_t count, loff_t *ppos)
{
void *image_ptr;
- ssize_t retval;
image_ptr = image[minor].kern_buf + *ppos;
+ if (__copy_to_user(buf, image_ptr, (unsigned long)count))
+ return -EFAULT;
- retval = __copy_to_user(buf, image_ptr, (unsigned long)count);
- if (retval != 0) {
- retval = (count - retval);
- pr_warn("Partial copy to userspace\n");
- } else
- retval = count;
-
- /* Return number of bytes successfully read */
- return retval;
+ return count;
}
static ssize_t buffer_from_user(unsigned int minor, const char __user *buf,
size_t count, loff_t *ppos)
{
void *image_ptr;
- size_t retval;
image_ptr = image[minor].kern_buf + *ppos;
+ if (__copy_from_user(image_ptr, buf, (unsigned long)count))
+ return -EFAULT;
- retval = __copy_from_user(image_ptr, buf, (unsigned long)count);
- if (retval != 0) {
- retval = (count - retval);
- pr_warn("Partial copy to userspace\n");
- } else
- retval = count;
-
- /* Return number of bytes successfully read */
- return retval;
+ return count;
}
static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count,
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3] VME bus error handling overhaul
This moves tsi148 error handling into VME subsystem so it can be shared with the other bridge driver. Then there is a change to close a fixme on separating errors by address space. And finally a fix for memory leak problem that was introduced with support of mmap's. The next logical step in this direction would be to add error handling support to ca91cx42 and make it unconditional for tsi148. It also makes much sense to add synchronization to error-related list operations (spinlocks, rcu). Dmitry Kalinkin (3): vme: move tsi148 error handling into VME subsystem vme: include address space in error filtering vme: change bus error handling scheme drivers/vme/bridges/vme_ca91cx42.c | 3 +- drivers/vme/bridges/vme_tsi148.c | 170 ++--- drivers/vme/vme.c | 83 ++ drivers/vme/vme_bridge.h | 21 +++-- 4 files changed, 147 insertions(+), 130 deletions(-) -- 1.8.3.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] vme: move tsi148 error handling into VME subsystem
Error handling code found in tsi148 is not device specific. In fact it
already relies on shared vme_bus_error struct and vme_bridge.vme_errors
field. The other bridge driver could reuse this code if it is shared.
This introduces a slight behavior change: vme error message won't be
triggered in a rare case when err_chk=1 and kmalloc fails.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/vme/bridges/vme_tsi148.c | 93 +++-
drivers/vme/vme.c| 86 +
drivers/vme/vme_bridge.h | 6 +++
3 files changed, 99 insertions(+), 86 deletions(-)
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index fb1e7ad..c1f9385 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -169,7 +169,6 @@ static u32 tsi148_VERR_irqhandler(struct vme_bridge
*tsi148_bridge)
unsigned int error_addr_high, error_addr_low;
unsigned long long error_addr;
u32 error_attrib;
- struct vme_bus_error *error = NULL;
struct tsi148_driver *bridge;
bridge = tsi148_bridge->driver_priv;
@@ -186,23 +185,12 @@ static u32 tsi148_VERR_irqhandler(struct vme_bridge
*tsi148_bridge)
"Occurred\n");
}
- if (err_chk) {
- error = kmalloc(sizeof(struct vme_bus_error), GFP_ATOMIC);
- if (error) {
- error->address = error_addr;
- error->attributes = error_attrib;
- list_add_tail(&error->list, &tsi148_bridge->vme_errors);
- } else {
- dev_err(tsi148_bridge->parent,
- "Unable to alloc memory for VMEbus Error
reporting\n");
- }
- }
-
- if (!error) {
+ if (err_chk)
+ vme_bus_error_handler(tsi148_bridge, error_addr, error_attrib);
+ else
dev_err(tsi148_bridge->parent,
"VME Bus Error at address: 0x%llx, attributes: %08x\n",
error_addr, error_attrib);
- }
/* Clear Status */
iowrite32be(TSI148_LCSR_VEAT_VESCL, bridge->base + TSI148_LCSR_VEAT);
@@ -483,73 +471,6 @@ static int tsi148_irq_generate(struct vme_bridge
*tsi148_bridge, int level,
}
/*
- * Find the first error in this address range
- */
-static struct vme_bus_error *tsi148_find_error(struct vme_bridge
*tsi148_bridge,
- u32 aspace, unsigned long long address, size_t count)
-{
- struct list_head *err_pos;
- struct vme_bus_error *vme_err, *valid = NULL;
- unsigned long long bound;
-
- bound = address + count;
-
- /*
-* XXX We are currently not looking at the address space when parsing
-* for errors. This is because parsing the Address Modifier Codes
-* is going to be quite resource intensive to do properly. We
-* should be OK just looking at the addresses and this is certainly
-* much better than what we had before.
-*/
- err_pos = NULL;
- /* Iterate through errors */
- list_for_each(err_pos, &tsi148_bridge->vme_errors) {
- vme_err = list_entry(err_pos, struct vme_bus_error, list);
- if ((vme_err->address >= address) &&
- (vme_err->address < bound)) {
-
- valid = vme_err;
- break;
- }
- }
-
- return valid;
-}
-
-/*
- * Clear errors in the provided address range.
- */
-static void tsi148_clear_errors(struct vme_bridge *tsi148_bridge,
- u32 aspace, unsigned long long address, size_t count)
-{
- struct list_head *err_pos, *temp;
- struct vme_bus_error *vme_err;
- unsigned long long bound;
-
- bound = address + count;
-
- /*
-* XXX We are currently not looking at the address space when parsing
-* for errors. This is because parsing the Address Modifier Codes
-* is going to be quite resource intensive to do properly. We
-* should be OK just looking at the addresses and this is certainly
-* much better than what we had before.
-*/
- err_pos = NULL;
- /* Iterate through errors */
- list_for_each_safe(err_pos, temp, &tsi148_bridge->vme_errors) {
- vme_err = list_entry(err_pos, struct vme_bus_error, list);
-
- if ((vme_err->address >= address) &&
- (vme_err->address < bound)) {
-
- list_del(err_pos);
- kfree(vme_err);
- }
- }
-}
-
-/*
* Initialize a slave window with the requested attributes.
*/
static int tsi148_slave_set(struct vme_slave_resource *image, int enabled,
@@ -1323,14
[PATCH 3/3] vme: change bus error handling scheme
The current VME bus error handler adds errors to the bridge error list.
vme_master_{read,write} then traverses that list to look for relevant
errors.
Such scheme didn't work well for accesses going through vme_master_mmap
because they would also allocate a vme_bus_error, but have no way to do
vme_clear_errors call to free that memory.
This changes the error handling process to be other way around: now
vme_master_{read,write} defines a window in VME address space that will
catch possible errors. VME bus error interrupt only traverses these
windows and marks those that had errors in them.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/vme/bridges/vme_ca91cx42.c | 3 +-
drivers/vme/bridges/vme_tsi148.c | 83 +-
drivers/vme/vme.c | 92 ++
drivers/vme/vme_bridge.h | 23 ++
4 files changed, 91 insertions(+), 110 deletions(-)
diff --git a/drivers/vme/bridges/vme_ca91cx42.c
b/drivers/vme/bridges/vme_ca91cx42.c
index f692efc..834883d 100644
--- a/drivers/vme/bridges/vme_ca91cx42.c
+++ b/drivers/vme/bridges/vme_ca91cx42.c
@@ -204,8 +204,7 @@ static int ca91cx42_irq_init(struct vme_bridge
*ca91cx42_bridge)
/* Need pdev */
pdev = container_of(ca91cx42_bridge->parent, struct pci_dev, dev);
- /* Initialise list for VME bus errors */
- INIT_LIST_HEAD(&ca91cx42_bridge->vme_errors);
+ INIT_LIST_HEAD(&ca91cx42_bridge->vme_error_handlers);
mutex_init(&ca91cx42_bridge->irq_mtx);
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index 76ccfae..b0132e0 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -314,8 +314,7 @@ static int tsi148_irq_init(struct vme_bridge *tsi148_bridge)
bridge = tsi148_bridge->driver_priv;
- /* Initialise list for VME bus errors */
- INIT_LIST_HEAD(&tsi148_bridge->vme_errors);
+ INIT_LIST_HEAD(&tsi148_bridge->vme_error_handlers);
mutex_init(&tsi148_bridge->irq_mtx);
@@ -1187,7 +1186,7 @@ static ssize_t tsi148_master_read(struct
vme_master_resource *image, void *buf,
int retval, enabled;
unsigned long long vme_base, size;
u32 aspace, cycle, dwidth;
- struct vme_bus_error *vme_err = NULL;
+ struct vme_error_handler *handler;
struct vme_bridge *tsi148_bridge;
void __iomem *addr = image->kern_base + offset;
unsigned int done = 0;
@@ -1197,6 +1196,17 @@ static ssize_t tsi148_master_read(struct
vme_master_resource *image, void *buf,
spin_lock(&image->lock);
+ if (err_chk) {
+ __tsi148_master_get(image, &enabled, &vme_base, &size, &aspace,
+ &cycle, &dwidth);
+ handler = vme_register_error_handler(tsi148_bridge, aspace,
+vme_base + offset, count);
+ if (!handler) {
+ spin_unlock(&image->lock);
+ return -ENOMEM;
+ }
+ }
+
/* The following code handles VME address alignment. We cannot use
* memcpy_xxx here because it may cut data transfers in to 8-bit
* cycles when D16 or D32 cycles are required on the VME bus.
@@ -1240,24 +1250,16 @@ static ssize_t tsi148_master_read(struct
vme_master_resource *image, void *buf,
out:
retval = count;
- if (!err_chk)
- goto skip_chk;
-
- __tsi148_master_get(image, &enabled, &vme_base, &size, &aspace, &cycle,
- &dwidth);
-
- vme_err = vme_find_error(tsi148_bridge, aspace, vme_base + offset,
- count);
- if (vme_err != NULL) {
- dev_err(image->parent->parent, "First VME read error detected "
- "an at address 0x%llx\n", vme_err->address);
- retval = vme_err->address - (vme_base + offset);
- /* Clear down save errors in this address range */
- vme_clear_errors(tsi148_bridge, aspace, vme_base + offset,
- count);
+ if (err_chk) {
+ if (handler->num_errors) {
+ dev_err(image->parent->parent,
+ "First VME read error detected an at address
0x%llx\n",
+ handler->first_error);
+ retval = handler->first_error - (vme_base + offset);
+ }
+ vme_unregister_error_handler(handler);
}
-skip_chk:
spin_unlock(&image->lock);
return retval;
@@ -1274,7 +1276,7 @@ static ssize_t tsi148_master_write(struct
vme_master_resource *image, void *buf,
unsigned int done = 0;
unsigned
[PATCH 2/3] vme: include address space in error filtering
Also changes vme_bus_error_handler to take generic address modifier code
instead of raw contents of a device-specific attribute register.
Signed-off-by: Dmitry Kalinkin
Cc: Igor Alekseev
---
drivers/vme/bridges/vme_tsi148.c | 4 ++-
drivers/vme/vme.c| 61 +++-
drivers/vme/vme_bridge.h | 4 +--
3 files changed, 46 insertions(+), 23 deletions(-)
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index c1f9385..76ccfae 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -169,6 +169,7 @@ static u32 tsi148_VERR_irqhandler(struct vme_bridge
*tsi148_bridge)
unsigned int error_addr_high, error_addr_low;
unsigned long long error_addr;
u32 error_attrib;
+ int error_am;
struct tsi148_driver *bridge;
bridge = tsi148_bridge->driver_priv;
@@ -176,6 +177,7 @@ static u32 tsi148_VERR_irqhandler(struct vme_bridge
*tsi148_bridge)
error_addr_high = ioread32be(bridge->base + TSI148_LCSR_VEAU);
error_addr_low = ioread32be(bridge->base + TSI148_LCSR_VEAL);
error_attrib = ioread32be(bridge->base + TSI148_LCSR_VEAT);
+ error_am = (error_attrib & TSI148_LCSR_VEAT_AM_M) >> 8;
reg_join(error_addr_high, error_addr_low, &error_addr);
@@ -186,7 +188,7 @@ static u32 tsi148_VERR_irqhandler(struct vme_bridge
*tsi148_bridge)
}
if (err_chk)
- vme_bus_error_handler(tsi148_bridge, error_addr, error_attrib);
+ vme_bus_error_handler(tsi148_bridge, error_addr, error_am);
else
dev_err(tsi148_bridge->parent,
"VME Bus Error at address: 0x%llx, attributes: %08x\n",
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 6803744..2b79cd2 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -223,6 +223,39 @@ int vme_check_window(u32 aspace, unsigned long long
vme_base,
}
EXPORT_SYMBOL(vme_check_window);
+static u32 vme_get_aspace(int am)
+{
+ switch (am) {
+ case 0x29:
+ case 0x2D:
+ return VME_A16;
+ case 0x38:
+ case 0x39:
+ case 0x3A:
+ case 0x3B:
+ case 0x3C:
+ case 0x3D:
+ case 0x3E:
+ case 0x3F:
+ return VME_A24;
+ case 0x8:
+ case 0x9:
+ case 0xA:
+ case 0xB:
+ case 0xC:
+ case 0xD:
+ case 0xE:
+ case 0xF:
+ return VME_A32;
+ case 0x0:
+ case 0x1:
+ case 0x3:
+ return VME_A64;
+ }
+
+ return 0;
+}
+
/*
* Request a slave image with specific attributes, return some unique
* identifier.
@@ -991,14 +1024,14 @@ int vme_dma_free(struct vme_resource *resource)
EXPORT_SYMBOL(vme_dma_free);
void vme_bus_error_handler(struct vme_bridge *bridge,
- unsigned long long address, u32 attributes)
+ unsigned long long address, int am)
{
struct vme_bus_error *error;
error = kmalloc(sizeof(struct vme_bus_error), GFP_ATOMIC);
if (error) {
+ error->aspace = vme_get_aspace(am);
error->address = address;
- error->attributes = attributes;
list_add_tail(&error->list, &bridge->vme_errors);
} else {
dev_err(bridge->parent,
@@ -1019,19 +1052,13 @@ struct vme_bus_error *vme_find_error(struct vme_bridge
*bridge, u32 aspace,
bound = address + count;
- /*
-* XXX We are currently not looking at the address space when parsing
-* for errors. This is because parsing the Address Modifier Codes
-* is going to be quite resource intensive to do properly. We
-* should be OK just looking at the addresses and this is certainly
-* much better than what we had before.
-*/
err_pos = NULL;
/* Iterate through errors */
list_for_each(err_pos, &bridge->vme_errors) {
vme_err = list_entry(err_pos, struct vme_bus_error, list);
- if ((vme_err->address >= address) &&
- (vme_err->address < bound)) {
+ if ((vme_err->aspace == aspace) &&
+ (vme_err->address >= address) &&
+ (vme_err->address < bound)) {
valid = vme_err;
break;
@@ -1054,20 +1081,14 @@ void vme_clear_errors(struct vme_bridge *bridge, u32
aspace,
bound = address + count;
- /*
-* XXX We are currently not looking at the address space when parsing
-* for errors. This is because parsing the Address Modifier Codes
-* is going to be quite resource intensive to do properly. We
-* should be OK just looking at
Re: [PATCHv3 6/9] staging: vme_user: switch to returning -EFAULT on __copy_*_user errors
On Mon, Jul 6, 2015 at 3:51 PM, Martyn Welch wrote:
> On 26/06/15 21:39, Dmitry Kalinkin wrote:
>>
>> Signed-off-by: Dmitry Kalinkin
>> ---
>> drivers/staging/vme/devices/vme_user.c | 47
>> --
>> 1 file changed, 11 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/staging/vme/devices/vme_user.c
>> b/drivers/staging/vme/devices/vme_user.c
>> index a2345db..ef876a4 100644
>> --- a/drivers/staging/vme/devices/vme_user.c
>> +++ b/drivers/staging/vme/devices/vme_user.c
>> @@ -123,7 +123,6 @@ struct vme_user_vma_priv {
>> static ssize_t resource_to_user(int minor, char __user *buf, size_t
>> count,
>> loff_t *ppos)
>> {
>> - ssize_t retval;
>> ssize_t copied = 0;
>>
>> if (count > image[minor].size_buf)
>> @@ -135,13 +134,8 @@ static ssize_t resource_to_user(int minor, char
>> __user *buf, size_t count,
>> if (copied < 0)
>> return (int)copied;
>>
>> - retval = __copy_to_user(buf, image[minor].kern_buf,
>> - (unsigned long)copied);
>> - if (retval != 0) {
>> - copied = (copied - retval);
>> - pr_info("User copy failed\n");
>> - return -EINVAL;
>> - }
>> + if (__copy_to_user(buf, image[minor].kern_buf, (unsigned
>> long)copied))
>> + return -EFAULT;
>>
>> return copied;
>
>
> Does __copy_to_user() not return the number of bytes still to be copied? The
> above seems to add the assumption that __copy_to_user()
> can't return part way through a copy.
Yes it does. And this information is useless in userspace.
Returning -EFAULT would be more informative in this case.
I couldn't find an example of kernel code doing this any other way:
http://lxr.free-electrons.com/ident?i=__copy_to_user
>
>> }
>> @@ -149,21 +143,16 @@ static ssize_t resource_to_user(int minor, char
>> __user *buf, size_t count,
>> static ssize_t resource_from_user(unsigned int minor, const char __user
>> *buf,
>> size_t count, loff_t *ppos)
>> {
>> - ssize_t retval;
>> ssize_t copied = 0;
>>
>> if (count > image[minor].size_buf)
>> count = image[minor].size_buf;
>>
>> - retval = __copy_from_user(image[minor].kern_buf, buf,
>> - (unsigned long)count);
>> - if (retval != 0)
>> - copied = (copied - retval);
>> - else
>> - copied = count;
>> + if (__copy_from_user(image[minor].kern_buf, buf, (unsigned
>> long)count))
>> + return -EFAULT;
>>
>
> Same here.
>
>> copied = vme_master_write(image[minor].resource,
>> image[minor].kern_buf,
>> - copied, *ppos);
>> + count, *ppos);
>>
>> return copied;
>> }
>> @@ -172,38 +161,24 @@ static ssize_t buffer_to_user(unsigned int minor,
>> char __user *buf,
>> size_t count, loff_t *ppos)
>> {
>> void *image_ptr;
>> - ssize_t retval;
>>
>> image_ptr = image[minor].kern_buf + *ppos;
>> + if (__copy_to_user(buf, image_ptr, (unsigned long)count))
>> + return -EFAULT;
>>
>
> Ditto.
>
>> - retval = __copy_to_user(buf, image_ptr, (unsigned long)count);
>> - if (retval != 0) {
>> - retval = (count - retval);
>> - pr_warn("Partial copy to userspace\n");
>> - } else
>> - retval = count;
>> -
>> - /* Return number of bytes successfully read */
>> - return retval;
>> + return count;
>> }
>>
>> static ssize_t buffer_from_user(unsigned int minor, const char __user
>> *buf,
>> size_t count, loff_t *ppos)
>> {
>> void *image_ptr;
>> - size_t retval;
>>
>> image_ptr = image[minor].kern_buf + *ppos;
>> + if (__copy_from_user(image_ptr, buf, (unsigned long)count))
>> + return -EFAULT;
>>
>
> And here.
>
>> - retval = __copy_from_user(image_ptr, buf, (unsigned long)count);
>> - if (retval != 0) {
>> - retval = (count - retval);
>> - pr_warn("Partial copy to userspace\n");
>> - } else
>> - retval = count;
>> -
>> - /* Return number of bytes successfully read */
>> - return retval;
>> + return count;
>> }
>>
>> static ssize_t vme_user_read(struct file *file, char __user *buf, size_t
>> count,
>>
>
> --
> Martyn Welch (Lead Software Engineer) | Registered in England and Wales
> GE Intelligent Platforms | (3828642) at 100 Barbirolli Square
> T +44(0)1327322748 | Manchester, M2 3AB
> E martyn.we...@ge.com | VAT:GB 927559189
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
On Mon, Jul 6, 2015 at 4:22 PM, Martyn Welch wrote: > > Sorry about the *really* late reply, loads of emails some how missed my > periodic search of the mailing list. > > I'm happy with the addition of DMA, just not sure whether it's worth adding > an extra device file just to handle DMA. Could the user space application > not just use the control device? That would require an additional ioctl field for DMA channel id in case we want to support both DMA channels on tsi148. It would make sense to save that device minor if Documentation/devices.txt was good. But it has only 4 slave and 4 master windows whereas we would want to make some parameters for vme_user to configure this allocation numbers up to 8 slaves and 8 masters. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] VME bus error handling overhaul
Hi Martyn, > On 06 Jul 2015, at 15:31, Martyn Welch wrote: > > These are looking good to me. Thank you for finding time to look at this and the others. > > You mention error handling in relation to vme_master_mmap, am I right in > thinking patch 3 avoids errors being recorded when triggered by an mmap > access (so as not to appear as a spurious error on a later access)? Yes, this is covered as well. > > I think it would be worth at least logging errors to the kernel log should > they be generated and not be handled by a error handler, so someone using > mmap gets at least some form of notification that their accesses are > resulting on bus errors. What do you think? Yes. I agree with you. Bus error handler could print a message when an error doesn’t trigger any of the registered handlers. What would be really useful, is if we could dispatch such errors to the userspace in a more direct way. Not sure how, though. Cheers, Dmitry > > > On 02/07/15 15:11, Dmitry Kalinkin wrote: >> This moves tsi148 error handling into VME subsystem so it can be shared with >> the other bridge driver. Then there is a change to close a fixme on >> separating >> errors by address space. And finally a fix for memory leak problem that was >> introduced with support of mmap's. >> >> The next logical step in this direction would be to add error handling >> support >> to ca91cx42 and make it unconditional for tsi148. It also makes much sense to >> add synchronization to error-related list operations (spinlocks, rcu). >> >> Dmitry Kalinkin (3): >> vme: move tsi148 error handling into VME subsystem >> vme: include address space in error filtering >> vme: change bus error handling scheme >> >> drivers/vme/bridges/vme_ca91cx42.c | 3 +- >> drivers/vme/bridges/vme_tsi148.c | 170 >> ++--- >> drivers/vme/vme.c | 83 ++ >> drivers/vme/vme_bridge.h | 21 +++-- >> 4 files changed, 147 insertions(+), 130 deletions(-) >> > > -- > Martyn Welch (Lead Software Engineer) | Registered in England and Wales > GE Intelligent Platforms | (3828642) at 100 Barbirolli Square > T +44(0)1327322748 | Manchester, M2 3AB > E martyn.we...@ge.com | VAT:GB 927559189 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] vme: print unhandled VME access errors
This will enable error messages for accesses done through mmap.
Signed-off-by: Dmitry Kalinkin
---
This depends on '[PATCH 0/3] VME bus error handling overhaul' patchset.
---
drivers/vme/vme.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 7a10d92..72924b0 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -1028,6 +1028,7 @@ void vme_bus_error_handler(struct vme_bridge *bridge,
{
struct list_head *handler_pos = NULL;
struct vme_error_handler *handler;
+ int handler_triggered = 0;
u32 aspace = vme_get_aspace(am);
list_for_each(handler_pos, &bridge->vme_error_handlers) {
@@ -1040,8 +1041,14 @@ void vme_bus_error_handler(struct vme_bridge *bridge,
handler->first_error = address;
if (handler->num_errors != UINT_MAX)
handler->num_errors++;
+ handler_triggered = 1;
}
}
+
+ if (!handler_triggered)
+ dev_err(bridge->parent,
+ "Unhandled VME access error at address 0x%llx\n",
+ address);
}
EXPORT_SYMBOL(vme_bus_error_handler);
--
1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
On Mon, Jul 6, 2015 at 5:48 PM, Martyn Welch wrote: > > > On 06/07/15 14:50, Dmitry Kalinkin wrote: >> >> On Mon, Jul 6, 2015 at 4:22 PM, Martyn Welch wrote: >>> >>> >>> Sorry about the *really* late reply, loads of emails some how missed my >>> periodic search of the mailing list. >>> >>> I'm happy with the addition of DMA, just not sure whether it's worth >>> adding >>> an extra device file just to handle DMA. Could the user space application >>> not just use the control device? >> >> That would require an additional ioctl field for DMA channel id in case we >> want >> to support both DMA channels on tsi148. >> > > Or just dynamically allocate and free a resource for the DMA operation? That seems to be a too high level. Also notice how vme_user_dma_ioctl is doing without locks now. Acquiring a resource for operation would introduce at least one. > >> It would make sense to save that device minor if Documentation/devices.txt >> was good. >> But it has only 4 slave and 4 master windows whereas we would want to >> make some parameters for vme_user to configure this allocation numbers up >> to 8 slaves and 8 masters. >> > > The vme_user module was originally envisaged as a mechanism to provide > support for applications that had been written to use the original driver at > vmelinux.org. That part I never understood. vmelinux.org's cvs has a very dated driver with a very limited capabilities. This one looks like a grandpa of the one we have (both tsi148 and universe): ftp://ftp.prosoft.ru/pub/Hardware/Fastwel/CPx/CPC600/Software/Drivers/Linux/tsi148.tar.gz There is also VME4L driver by MEN (tsi148 only): https://www.men.de/software/13z014-90/ Some other driver: http://www.awa.tohoku.ac.jp/~sanshiro/kinoko-e/vmedrv/ Some other driver (universe only): https://github.com/mgmarino/VMELinux/blob/master/driver/universe.c Driver by CERN (dynamic window allocation): https://github.com/cota/ht-drivers/tree/master/vmebridge/driver The point is: there are many drivers of different quality. All all of them include some sort of userspace interface and that, as you mention below, seems to work well for many cases. All I'm trying to do is to make vme_user to be at least as useful as drivers above without looking back at vmelinux. > Some functionality was dropped as it was not good practice > (such as receiving VME interrupts in user space, it's not really doable if > the slave card is Release On Register Access rather than Release on > Acknowledge), Didn't know about RORA. I wonder how different this is compared to the PCI bus case. > so the interface became more of a debug mechanism for me. > Others have clearly found it provides enough for them to allow drivers to be > written in user space. > > I was thinking that the opposite might be better, no windows were mapped at > module load, windows could be allocated and mapped using the control device. > This would ensure that unused resources were still available for kernel > based drivers and would mean the driver wouldn't be pre-allocating a bunch > of fairly substantially sized slave window buffers (the buffers could also > be allocated to match the size of the slave window requested). What do you > think? I'm not a VME expert, but it seems that VME windows are a quiet limited resource no matter how you allocate your resources. Theoretically we could put up to 32 different boards in a single crate, so there won't be enough windows for each driver to allocate. That said, there is no way around this when putting together a really heterogeneous VME system. To overcome such problem, one could develop a different kernel API that would not provide windows to the drivers, but handle reads and writes by reconfiguring windows on the fly, which in turn would introduce more latency. Those who need such API are welcome to develop it :) As for dynamic vme_user device allocation, I don't see the point in this. The only existing kernel VME driver allocates windows in advance, user is just to make sure to leave one free window if she wants to use that. Module parameter for window count will be dynamic enough to handle that. Cheers, Dmitry ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
Hi Alessio, [Sorry for double post] > On 07 Jul 2015, at 10:08, Alessio Igor Bogani > wrote: > > Hi Dmitry, > > On 6 July 2015 at 19:24, Dmitry Kalinkin wrote: > [...] > I'm not a VME expert, but it seems that VME windows are a quiet limited > resource > no matter how you allocate your resources. Theoretically we could put up to 32 > different boards in a single crate, so there won't be enough windows for each > driver to allocate. That said, there is no way around this when putting > together > a really heterogeneous VME system. To overcome such problem, one could > develop a different kernel API that would not provide windows to the > drivers, but > handle reads and writes by reconfiguring windows on the fly, which in turn > would > introduce more latency. > > In my humble opinion using user-space drivers (as workaround for limited > windows/images) introduce more latency than let VME driver dynamically > configure windows/images. After all VME systems usually aren't so much > dynamic by its nature (Who likes continuously put in and out a board which > requires an insertion force between 20 and 50 kg?) and are instead heavily > used in critical contexts often in non-stop way. Userspace drivers are not exactly doing this differently. It’s just that you can use that interface to quickly build more flexible site-specific software that knows about whole VME system. So there, having a low level access to windows works well (there is a 20+ year history of such drivers). But if we want reusable drivers, especially in the kernel, that will require some more effort in making a driver stack. The API I had in mind would have only vme_master_read and vme_master_write that would take absolute addresses (not relative to any window). These variants of access functions would then try to reuse any window that is already able to serve the request or wait for a free window and reconfigure it for the need of the request. After usage the window is to be returned back to the window pool. Other way to implement these would be to use DMA for everything, since it doesn’t have the limitations that the windows have. > > In fact this is a big obstacle for adoption of this VME stack (at least for > us). We use VME a lot and we care about latency as well so we use only > kernel-space drivers for ours VME boards but unfortunately the VME stack let > us to link a single board with a single window/image (so max 8 boards on > tsi148) only. That said that stack has proven to be very rock solid. Current VME stack links windows not to the boards, but to device drivers. Driver could potentially minimise window usage within it’s scope (any sort of window reusing, like mapping whole A16 once to be used with all boards), but this won’t work across multiple drivers. Even if all of your drivers are window-wise economic, they will still need some amount of windows per each driver. Not that we have that many kernel drivers... > > Until now we have used a modified version of the old vmelinux.org stack for > sharing windows/images between all (ours) VME kernel drivers and we would be > very happy to see something similar in vanilla (at least coalescence two > adjacent addresses with same modifiers). > > Those who need such API are welcome to develop it :) > > I would be glad to try it if the maintainer is willing to receive this type > of changes. > > Ciao, > Alessio > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
> On 07 Jul 2015, at 15:51, Alessio Igor Bogani > wrote: > >> Current VME stack links windows not to the boards, but to device drivers. >> Driver >> could potentially minimise window usage within it’s scope (any sort of >> window >> reusing, like mapping whole A16 once to be used with all boards), but this >> won’t >> work across multiple drivers. Even if all of your drivers are window-wise >> economic, >> they will still need some amount of windows per each driver. Not that we >> have that >> many kernel drivers... > > Yes you can share a window/image between all boards of the same type > (in effect we are porting our drivers in this way) *but* it isn't the > expected way to work (see Documentation/vme_api.txt struct > vme_driver's probe() and match() functions and the GE PIO2 VME > driver). And vme_pio2 can’t handle more than 8 boards. This shows that the current design needs some adjustments. Also would be great if probe() and match() allowed for void *private data field. Cheers, Dmitry ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
