All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv3 00/16]  vme DMA and user space driver improvements
@ 2015-05-28 12:06 Dmitry Kalinkin
  2015-05-28 12:06 ` [PATCHv3 01/16] Documentation: mention vme_master_mmap() in VME API Dmitry Kalinkin
                   ` (16 more replies)
  0 siblings, 17 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:06 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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


^ permalink raw reply	[flat|nested] 49+ messages in thread

* [PATCHv3 01/16] Documentation: mention vme_master_mmap() in VME API
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
@ 2015-05-28 12:06 ` Dmitry Kalinkin
  2015-05-28 12:06 ` [PATCHv3 02/16] vme: tsi148: fix DMA lists longer that one item Dmitry Kalinkin
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:06 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 02/16] vme: tsi148: fix DMA lists longer that one item
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
  2015-05-28 12:06 ` [PATCHv3 01/16] Documentation: mention vme_master_mmap() in VME API Dmitry Kalinkin
@ 2015-05-28 12:06 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 03/16] vme: tsi148: fix first DMA item mapping Dmitry Kalinkin
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:06 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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 <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 03/16] vme: tsi148: fix first DMA item mapping
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
  2015-05-28 12:06 ` [PATCHv3 01/16] Documentation: mention vme_master_mmap() in VME API Dmitry Kalinkin
  2015-05-28 12:06 ` [PATCHv3 02/16] vme: tsi148: fix DMA lists longer that one item Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 04/16] vme: stop DMA transfer on interruption Dmitry Kalinkin
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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 <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 04/16] vme: stop DMA transfer on interruption
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (2 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 03/16] vme: tsi148: fix first DMA item mapping Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 05/16] staging: vme_user: refactor llseek to switch(){} Dmitry Kalinkin
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 05/16] staging: vme_user: refactor llseek to switch(){}
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (3 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 04/16] vme: stop DMA transfer on interruption Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 06/16] vme: check for A64 overflow in vme_check_window() Dmitry Kalinkin
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

This makes vme_user_llseek ignore all minors that don't have llseek
implementation.

Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 06/16] vme: check for A64 overflow in vme_check_window()
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (4 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 05/16] staging: vme_user: refactor llseek to switch(){} Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 07/16] vme: export vme_check_window() Dmitry Kalinkin
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 07/16] vme: export vme_check_window()
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (5 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 06/16] vme: check for A64 overflow in vme_check_window() Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 08/16] staging: vme_user: provide DMA functionality Dmitry Kalinkin
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (6 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 07/16] vme: export vme_check_window() Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-06-13  0:28   ` Greg Kroah-Hartman
  2015-05-28 12:07 ` [PATCHv3 09/16] vme: ca91cx42: return error code on DMA error Dmitry Kalinkin
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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 <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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_MINOR	4
 #define SLAVE_MAX	7
 #define CONTROL_MINOR	8
+#define DMA_MINOR	9
 
-#define PCI_BUF_SIZE  0x20000	/* Size of one slave image buffer */
+#define PCI_BUF_SIZE	0x20000		/* Size of one slave image buffer */
+
+#define VME_MAX_DMA_LEN	0x4000000	/* 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_dma_dir(dma_op->dir);
+	if (dir == DMA_NONE)
+		return -EINVAL;
+
+	pages = kmalloc_array(nr_pages, sizeof(pages[0]), GFP_KERNEL);
+	if (!pages) {
+		retval = -ENOMEM;
+		goto free;
+	}
+
+	sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+	if (!sgt) {
+		retval = -ENOMEM;
+		goto free;
+	}
+
+	dma_list = vme_new_dma_list(image[minor].resource);
+	if (!dma_list) {
+		retval = -ENOMEM;
+		goto free;
+	}
+
+	got_pages = get_user_pages_fast(dma_op->buf_vaddr, nr_pages,
+					dir == DMA_FROM_DEVICE, pages);
+	if (got_pages != nr_pages) {
+		pr_debug("Not all pages were pinned\n");
+		retval = (got_pages < 0) ? got_pages : -EFAULT;
+		goto release_pages;
+	}
+
+	retval = sg_alloc_table_from_pages(sgt, pages, nr_pages,
+					   offset, count, GFP_KERNEL);
+	if (retval)
+		goto release_pages;
+
+	sg_count = dma_map_sg(vme_user_bridge->dev.parent,
+			      sgt->sgl, sgt->nents, dir);
+	if (!sg_count) {
+		pr_debug("DMA mapping error\n");
+		retval = -EFAULT;
+		goto free_sgt;
+	}
+
+	retval = vme_user_sg_to_dma_list(dma_op, sgt, sg_count, dma_list);
+	if (retval)
+		goto dma_unmap;
+
+	retval = vme_dma_list_exec(dma_list);
+
+dma_unmap:
+	dma_unmap_sg(vme_user_bridge->dev.parent, sgt->sgl, sgt->nents, dir);
+
+free_sgt:
+	sg_free_table(sgt);
+
+release_pages:
+	if (got_pages > 0)
+		release_pages(pages, got_pages, 0);
+
+	vme_dma_list_free(dma_list);
+
+free:
+	kfree(sgt);
+	kfree(pages);
+
+	if (retval)
+		return retval;
+
+	return count;
+}
+
 /*
  * The ioctls provided by the old VME access method (the one at vmelinux.org)
  * are most certainly wrong as the effectively push the registers layout
@@ -459,6 +624,7 @@ static int vme_user_ioctl(struct inode *inode, struct file *file,
 	struct vme_master master;
 	struct vme_slave slave;
 	struct vme_irq_id irq_req;
+	struct vme_dma_op dma_op;
 	unsigned long copied;
 	unsigned int minor = MINOR(inode->i_rdev);
 	int retval;
@@ -569,6 +735,19 @@ static int vme_user_ioctl(struct inode *inode, struct file *file,
 			break;
 		}
 		break;
+	case DMA_MINOR:
+		switch (cmd) {
+		case VME_DO_DMA:
+			copied = copy_from_user(&dma_op, argp,
+						sizeof(dma_op));
+			if (copied != 0) {
+				pr_warn("Partial copy from userspace\n");
+				return -EFAULT;
+			}
+
+			return vme_user_dma_ioctl(minor, &dma_op);
+		}
+		break;
 	}
 
 	return -EINVAL;
@@ -842,6 +1021,15 @@ static int vme_user_probe(struct vme_dev *vdev)
 		}
 	}
 
+	image[DMA_MINOR].resource = vme_dma_request(vme_user_bridge,
+		VME_DMA_VME_TO_MEM | VME_DMA_MEM_TO_VME);
+	if (!image[DMA_MINOR].resource) {
+		dev_warn(&vdev->dev,
+			 "Unable to allocate dma resource\n");
+		err = -ENOMEM;
+		goto err_master;
+	}
+
 	/* Create sysfs entries - on udev systems this creates the dev files */
 	vme_user_sysfs_class = class_create(THIS_MODULE, driver_name);
 	if (IS_ERR(vme_user_sysfs_class)) {
@@ -864,6 +1052,9 @@ static int vme_user_probe(struct vme_dev *vdev)
 		case SLAVE_MINOR:
 			name = "bus/vme/s%d";
 			break;
+		case DMA_MINOR:
+			name = "bus/vme/dma0";
+			break;
 		default:
 			err = -EINVAL;
 			goto err_sysfs;
@@ -888,6 +1079,8 @@ err_sysfs:
 	}
 	class_destroy(vme_user_sysfs_class);
 
+	vme_dma_free(image[DMA_MINOR].resource);
+
 	/* Ensure counter set correcty to unalloc all master windows */
 	i = MASTER_MAX + 1;
 err_master:
@@ -927,6 +1120,8 @@ static int vme_user_remove(struct vme_dev *dev)
 	}
 	class_destroy(vme_user_sysfs_class);
 
+	vme_dma_free(image[DMA_MINOR].resource);
+
 	for (i = MASTER_MINOR; i < (MASTER_MAX + 1); i++) {
 		kfree(image[i].kern_buf);
 		vme_master_free(image[i].resource);
diff --git a/drivers/staging/vme/devices/vme_user.h b/drivers/staging/vme/devices/vme_user.h
index b8cc7bc..252b1c9 100644
--- a/drivers/staging/vme/devices/vme_user.h
+++ b/drivers/staging/vme/devices/vme_user.h
@@ -48,11 +48,22 @@ struct vme_irq_id {
 	__u8 statid;
 };
 
+struct vme_dma_op {
+	__u64 vme_addr;		/* Starting Address on the VMEbus */
+	__u64 buf_vaddr;	/* Pointer to userspace memory */
+	__u32 count;		/* Count of bytes to copy */
+	__u32 aspace;		/* Address Space */
+	__u32 cycle;		/* Cycle properties */
+	__u32 dwidth;		/* Data transfer width */
+	__u32 dir;		/* Transfer Direction */
+};
+
 #define VME_GET_SLAVE _IOR(VME_IOC_MAGIC, 1, struct vme_slave)
 #define VME_SET_SLAVE _IOW(VME_IOC_MAGIC, 2, struct vme_slave)
 #define VME_GET_MASTER _IOR(VME_IOC_MAGIC, 3, struct vme_master)
 #define VME_SET_MASTER _IOW(VME_IOC_MAGIC, 4, struct vme_master)
 #define VME_IRQ_GEN _IOW(VME_IOC_MAGIC, 5, struct vme_irq_id)
+#define VME_DO_DMA _IOW(VME_IOC_MAGIC, 7, struct vme_dma_op)
 
 #endif /* _VME_USER_H_ */
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 09/16] vme: ca91cx42: return error code on DMA error
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (7 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 08/16] staging: vme_user: provide DMA functionality Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 10/16] vme: ca91cx42: fix LM_CTL address mask Dmitry Kalinkin
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 10/16] vme: ca91cx42: fix LM_CTL address mask
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (8 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 09/16] vme: ca91cx42: return error code on DMA error Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 11/16] staging: vme_user: remove unused counters Dmitry Kalinkin
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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 <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 11/16] staging: vme_user: remove unused counters
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (9 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 10/16] vme: ca91cx42: fix LM_CTL address mask Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 12/16] staging: vme_user: remove forward declarations Dmitry Kalinkin
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 12/16] staging: vme_user: remove forward declarations
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (10 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 11/16] staging: vme_user: remove unused counters Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 13/16] staging: vme_user: remove open/release Dmitry Kalinkin
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

Reorder code so that forward declarations are not needed.

Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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;
@@ -1111,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");
+		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 void __exit vme_user_exit(void)
 {
 	vme_unregister_driver(&vme_user_driver);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 13/16] staging: vme_user: remove open/release
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (11 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 12/16] staging: vme_user: remove forward declarations Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 14/16] staging: vme_user: remove buf_unalloc helper Dmitry Kalinkin
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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 <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 14/16] staging: vme_user: remove buf_unalloc helper
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (12 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 13/16] staging: vme_user: remove open/release Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 15/16] vme: tsi148: depend on HAS_DMA for Kconfig Dmitry Kalinkin
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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 <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 15/16] vme: tsi148: depend on HAS_DMA for Kconfig
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (13 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 14/16] staging: vme_user: remove buf_unalloc helper Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-05-28 12:07 ` [PATCHv3 16/16] vme: provide uapi header Dmitry Kalinkin
  2015-05-31  3:06 ` [PATCHv3 00/16] vme DMA and user space driver improvements Greg Kroah-Hartman
  16 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv3 16/16] vme: provide uapi header
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (14 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 15/16] vme: tsi148: depend on HAS_DMA for Kconfig Dmitry Kalinkin
@ 2015-05-28 12:07 ` Dmitry Kalinkin
  2015-06-13  0:30   ` Greg Kroah-Hartman
  2015-05-31  3:06 ` [PATCHv3 00/16] vme DMA and user space driver improvements Greg Kroah-Hartman
  16 siblings, 1 reply; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-05-28 12:07 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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 <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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 <uapi/linux/vme.h>
+
 /* Resource Type */
 enum vme_resource_type {
 	VME_MASTER,
@@ -9,47 +11,6 @@ enum vme_resource_type {
 	VME_LM
 };
 
-/* VME Address Spaces */
-#define VME_A16		0x1
-#define VME_A24		0x2
-#define	VME_A32		0x4
-#define VME_A64		0x8
-#define VME_CRCSR	0x10
-#define VME_USER1	0x20
-#define VME_USER2	0x40
-#define VME_USER3	0x80
-#define VME_USER4	0x100
-
-#define VME_A16_MAX	0x10000ULL
-#define VME_A24_MAX	0x1000000ULL
-#define VME_A32_MAX	0x100000000ULL
-#define VME_A64_MAX	0x10000000000000000ULL
-#define VME_CRCSR_MAX	0x1000000ULL
-
-
-/* VME Cycle Types */
-#define VME_SCT		0x1
-#define VME_BLT		0x2
-#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_D16		0x2
-#define VME_D32		0x4
-#define VME_D64		0x8
-
 /* 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 0000000..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_A16		0x1
+#define VME_A24		0x2
+#define VME_A32		0x4
+#define VME_A64		0x8
+#define VME_CRCSR	0x10
+#define VME_USER1	0x20
+#define VME_USER2	0x40
+#define VME_USER3	0x80
+#define VME_USER4	0x100
+
+#define VME_A16_MAX	0x10000ULL
+#define VME_A24_MAX	0x1000000ULL
+#define VME_A32_MAX	0x100000000ULL
+#define VME_A64_MAX	0x10000000000000000ULL
+#define VME_CRCSR_MAX	0x1000000ULL
+
+/* VME Cycle Types */
+#define VME_SCT		0x1
+#define VME_BLT		0x2
+#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_D16		0x2
+#define VME_D32		0x4
+#define VME_D64		0x8
+
+/* 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 00/16]  vme DMA and user space driver improvements
  2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
                   ` (15 preceding siblings ...)
  2015-05-28 12:07 ` [PATCHv3 16/16] vme: provide uapi header Dmitry Kalinkin
@ 2015-05-31  3:06 ` Greg Kroah-Hartman
  2015-06-10 13:09   ` Dmitry Kalinkin
  16 siblings, 1 reply; 49+ messages in thread
From: Greg Kroah-Hartman @ 2015-05-31  3:06 UTC (permalink / raw)
  To: Dmitry Kalinkin
  Cc: linux-kernel, devel, Martyn Welch, Manohar Vanga, Igor Alekseev

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.        
> 
> 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

I need an ack from the VME maintainer before I can take these...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 00/16] vme DMA and user space driver improvements
  2015-05-31  3:06 ` [PATCHv3 00/16] vme DMA and user space driver improvements Greg Kroah-Hartman
@ 2015-06-10 13:09   ` Dmitry Kalinkin
  2015-06-13  0:31     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-06-10 13:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, Martyn Welch, Manohar Vanga, Igor Alekseev

On Sun, May 31, 2015 at 6:06 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> 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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-05-28 12:07 ` [PATCHv3 08/16] staging: vme_user: provide DMA functionality Dmitry Kalinkin
@ 2015-06-13  0:28   ` Greg Kroah-Hartman
  2015-07-06 13:22     ` Martyn Welch
  0 siblings, 1 reply; 49+ messages in thread
From: Greg Kroah-Hartman @ 2015-06-13  0:28 UTC (permalink / raw)
  To: Dmitry Kalinkin
  Cc: linux-kernel, devel, Martyn Welch, Manohar Vanga, Igor Alekseev

On Thu, May 28, 2015 at 03:07:05PM +0300, Dmitry Kalinkin wrote:
> 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 <dmitry.kalinkin@gmail.com>
> Cc: Igor Alekseev <igor.alekseev@itep.ru>
> ---
>  drivers/staging/vme/devices/vme_user.c | 201 ++++++++++++++++++++++++++++++++-
>  drivers/staging/vme/devices/vme_user.h |  11 ++
>  2 files changed, 209 insertions(+), 3 deletions(-)

I want to get Martyn's feedback on this, as it's adding a new feature to
the subsystem that he's going to have to maintain.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 16/16] vme: provide uapi header
  2015-05-28 12:07 ` [PATCHv3 16/16] vme: provide uapi header Dmitry Kalinkin
@ 2015-06-13  0:30   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 49+ messages in thread
From: Greg Kroah-Hartman @ 2015-06-13  0:30 UTC (permalink / raw)
  To: Dmitry Kalinkin
  Cc: linux-kernel, devel, Martyn Welch, Manohar Vanga, Igor Alekseev

On Thu, May 28, 2015 at 03:07:13PM +0300, Dmitry Kalinkin wrote:
> This separates VME related constants that are a part of both kernel and
> user space API into a common uapi header.

Why?  We don't want non-userspace things in the uapi header file, that's
not needed and just exports things to userspace we do not need/want
there.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 00/16] vme DMA and user space driver improvements
  2015-06-10 13:09   ` Dmitry Kalinkin
@ 2015-06-13  0:31     ` Greg Kroah-Hartman
  2015-06-13  2:04       ` Dmitry Kalinkin
  0 siblings, 1 reply; 49+ messages in thread
From: Greg Kroah-Hartman @ 2015-06-13  0:31 UTC (permalink / raw)
  To: Dmitry Kalinkin
  Cc: devel, Martyn Welch, Igor Alekseev, linux-kernel, Manohar Vanga

On Wed, Jun 10, 2015 at 04:09:19PM +0300, Dmitry Kalinkin wrote:
> On Sun, May 31, 2015 at 6:06 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> 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

I've taken all of these except patches 12, 13, 14 and 16.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 00/16] vme DMA and user space driver improvements
  2015-06-13  0:31     ` Greg Kroah-Hartman
@ 2015-06-13  2:04       ` Dmitry Kalinkin
  2015-06-13  2:24         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-06-13  2:04 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devel, Martyn Welch, Igor Alekseev, linux-kernel, Manohar Vanga

On Sat, Jun 13, 2015 at 3:31 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> 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?

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 00/16] vme DMA and user space driver improvements
  2015-06-13  2:04       ` Dmitry Kalinkin
@ 2015-06-13  2:24         ` Greg Kroah-Hartman
  2015-06-13  2:30           ` Dmitry Kalinkin
  0 siblings, 1 reply; 49+ messages in thread
From: Greg Kroah-Hartman @ 2015-06-13  2:24 UTC (permalink / raw)
  To: Dmitry Kalinkin
  Cc: devel, Martyn Welch, Manohar Vanga, Igor Alekseev, linux-kernel

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
> <gregkh@linuxfoundation.org> 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?

You added a new userspace api, that someone else is going to have to
maintain, that's not "harmless" at all.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 00/16] vme DMA and user space driver improvements
  2015-06-13  2:24         ` Greg Kroah-Hartman
@ 2015-06-13  2:30           ` Dmitry Kalinkin
  2015-06-13  4:40             ` Greg Kroah-Hartman
  0 siblings, 1 reply; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-06-13  2:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: devel, Martyn Welch, Manohar Vanga, Igor Alekseev, linux-kernel


> On 13 Jun 2015, at 05:24, Greg Kroah-Hartman <gregkh@linuxfoundation.org> 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
>> <gregkh@linuxfoundation.org> 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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 00/16] vme DMA and user space driver improvements
  2015-06-13  2:30           ` Dmitry Kalinkin
@ 2015-06-13  4:40             ` Greg Kroah-Hartman
  2015-06-13 13:34               ` [PATCHv4 0/4] " Dmitry Kalinkin
  0 siblings, 1 reply; 49+ messages in thread
From: Greg Kroah-Hartman @ 2015-06-13  4:40 UTC (permalink / raw)
  To: Dmitry Kalinkin
  Cc: devel, Martyn Welch, Manohar Vanga, Igor Alekseev, linux-kernel

On Sat, Jun 13, 2015 at 05:30:09AM +0300, Dmitry Kalinkin wrote:
> 
> > On 13 Jun 2015, at 05:24, Greg Kroah-Hartman <gregkh@linuxfoundation.org> 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
> >> <gregkh@linuxfoundation.org> 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.

Oops, no, that was 8, I didn't include that either, maybe that's why 12
and others didn't apply.

Anyway, rebase on my tree and resend and we can go from there.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 49+ messages in thread

* [PATCHv4 0/4] vme DMA and user space driver improvements
  2015-06-13  4:40             ` Greg Kroah-Hartman
@ 2015-06-13 13:34               ` Dmitry Kalinkin
  2015-06-13 13:34                 ` [PATCHv4 1/4] staging: vme_user: remove forward declarations Dmitry Kalinkin
                                   ` (4 more replies)
  0 siblings, 5 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-06-13 13:34 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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


^ permalink raw reply	[flat|nested] 49+ messages in thread

* [PATCHv4 1/4] staging: vme_user: remove forward declarations
  2015-06-13 13:34               ` [PATCHv4 0/4] " Dmitry Kalinkin
@ 2015-06-13 13:34                 ` Dmitry Kalinkin
  2015-06-13 13:34                 ` [PATCHv4 2/4] staging: vme_user: remove open/release Dmitry Kalinkin
                                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-06-13 13:34 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

Reorder code so that forward declarations are not needed.

Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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");
+		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 void __exit vme_user_exit(void)
 {
 	vme_unregister_driver(&vme_user_driver);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv4 2/4] staging: vme_user: remove open/release
  2015-06-13 13:34               ` [PATCHv4 0/4] " Dmitry Kalinkin
  2015-06-13 13:34                 ` [PATCHv4 1/4] staging: vme_user: remove forward declarations Dmitry Kalinkin
@ 2015-06-13 13:34                 ` Dmitry Kalinkin
  2015-06-13 13:34                 ` [PATCHv4 3/4] staging: vme_user: remove buf_unalloc helper Dmitry Kalinkin
                                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-06-13 13:34 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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 <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv4 3/4] staging: vme_user: remove buf_unalloc helper
  2015-06-13 13:34               ` [PATCHv4 0/4] " Dmitry Kalinkin
  2015-06-13 13:34                 ` [PATCHv4 1/4] staging: vme_user: remove forward declarations Dmitry Kalinkin
  2015-06-13 13:34                 ` [PATCHv4 2/4] staging: vme_user: remove open/release Dmitry Kalinkin
@ 2015-06-13 13:34                 ` Dmitry Kalinkin
  2015-06-13 13:34                 ` [PATCHv4 4/4] staging: vme_user: provide DMA functionality Dmitry Kalinkin
  2015-06-13 21:47                 ` [PATCHv4 0/4] vme DMA and user space driver improvements Greg Kroah-Hartman
  4 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-06-13 13:34 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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 <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* [PATCHv4 4/4] staging: vme_user: provide DMA functionality
  2015-06-13 13:34               ` [PATCHv4 0/4] " Dmitry Kalinkin
                                   ` (2 preceding siblings ...)
  2015-06-13 13:34                 ` [PATCHv4 3/4] staging: vme_user: remove buf_unalloc helper Dmitry Kalinkin
@ 2015-06-13 13:34                 ` Dmitry Kalinkin
  2015-06-13 21:47                 ` [PATCHv4 0/4] vme DMA and user space driver improvements Greg Kroah-Hartman
  4 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-06-13 13:34 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Igor Alekseev,
	Dmitry Kalinkin

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 <dmitry.kalinkin@gmail.com>
Cc: Igor Alekseev <igor.alekseev@itep.ru>
---
 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_MINOR	4
 #define SLAVE_MAX	7
 #define CONTROL_MINOR	8
+#define DMA_MINOR	9
 
-#define PCI_BUF_SIZE  0x20000	/* Size of one slave image buffer */
+#define PCI_BUF_SIZE	0x20000		/* Size of one slave image buffer */
+
+#define VME_MAX_DMA_LEN	0x4000000	/* 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_SHIFT;
+
+	dir = vme_dir_to_dma_dir(dma_op->dir);
+	if (dir == DMA_NONE)
+		return -EINVAL;
+
+	pages = kmalloc_array(nr_pages, sizeof(pages[0]), GFP_KERNEL);
+	if (!pages) {
+		retval = -ENOMEM;
+		goto free;
+	}
+
+	sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
+	if (!sgt) {
+		retval = -ENOMEM;
+		goto free;
+	}
+
+	dma_list = vme_new_dma_list(image[minor].resource);
+	if (!dma_list) {
+		retval = -ENOMEM;
+		goto free;
+	}
+
+	got_pages = get_user_pages_fast(dma_op->buf_vaddr, nr_pages,
+					dir == DMA_FROM_DEVICE, pages);
+	if (got_pages != nr_pages) {
+		pr_debug("Not all pages were pinned\n");
+		retval = (got_pages < 0) ? got_pages : -EFAULT;
+		goto release_pages;
+	}
+
+	retval = sg_alloc_table_from_pages(sgt, pages, nr_pages,
+					   offset, count, GFP_KERNEL);
+	if (retval)
+		goto release_pages;
+
+	sg_count = dma_map_sg(vme_user_bridge->dev.parent,
+			      sgt->sgl, sgt->nents, dir);
+	if (!sg_count) {
+		pr_debug("DMA mapping error\n");
+		retval = -EFAULT;
+		goto free_sgt;
+	}
+
+	retval = vme_user_sg_to_dma_list(dma_op, sgt, sg_count, dma_list);
+	if (retval)
+		goto dma_unmap;
+
+	retval = vme_dma_list_exec(dma_list);
+
+dma_unmap:
+	dma_unmap_sg(vme_user_bridge->dev.parent, sgt->sgl, sgt->nents, dir);
+
+free_sgt:
+	sg_free_table(sgt);
+
+release_pages:
+	if (got_pages > 0)
+		release_pages(pages, got_pages, 0);
+
+	vme_dma_list_free(dma_list);
+
+free:
+	kfree(sgt);
+	kfree(pages);
+
+	if (retval)
+		return retval;
+
+	return count;
+}
+
 /*
  * The ioctls provided by the old VME access method (the one at vmelinux.org)
  * are most certainly wrong as the effectively push the registers layout
@@ -359,6 +524,7 @@ static int vme_user_ioctl(struct inode *inode, struct file *file,
 	struct vme_master master;
 	struct vme_slave slave;
 	struct vme_irq_id irq_req;
+	struct vme_dma_op dma_op;
 	unsigned long copied;
 	unsigned int minor = MINOR(inode->i_rdev);
 	int retval;
@@ -467,6 +633,19 @@ static int vme_user_ioctl(struct inode *inode, struct file *file,
 			break;
 		}
 		break;
+	case DMA_MINOR:
+		switch (cmd) {
+		case VME_DO_DMA:
+			copied = copy_from_user(&dma_op, argp,
+						sizeof(dma_op));
+			if (copied != 0) {
+				pr_warn("Partial copy from userspace\n");
+				return -EFAULT;
+			}
+
+			return vme_user_dma_ioctl(minor, &dma_op);
+		}
+		break;
 	}
 
 	return -EINVAL;
@@ -678,6 +857,15 @@ static int vme_user_probe(struct vme_dev *vdev)
 		}
 	}
 
+	image[DMA_MINOR].resource = vme_dma_request(vme_user_bridge,
+		VME_DMA_VME_TO_MEM | VME_DMA_MEM_TO_VME);
+	if (!image[DMA_MINOR].resource) {
+		dev_warn(&vdev->dev,
+			 "Unable to allocate dma resource\n");
+		err = -ENOMEM;
+		goto err_master;
+	}
+
 	/* Create sysfs entries - on udev systems this creates the dev files */
 	vme_user_sysfs_class = class_create(THIS_MODULE, driver_name);
 	if (IS_ERR(vme_user_sysfs_class)) {
@@ -700,6 +888,9 @@ static int vme_user_probe(struct vme_dev *vdev)
 		case SLAVE_MINOR:
 			name = "bus/vme/s%d";
 			break;
+		case DMA_MINOR:
+			name = "bus/vme/dma0";
+			break;
 		default:
 			err = -EINVAL;
 			goto err_sysfs;
@@ -724,6 +915,8 @@ err_sysfs:
 	}
 	class_destroy(vme_user_sysfs_class);
 
+	vme_dma_free(image[DMA_MINOR].resource);
+
 	/* Ensure counter set correcty to unalloc all master windows */
 	i = MASTER_MAX + 1;
 err_master:
@@ -764,6 +957,8 @@ static int vme_user_remove(struct vme_dev *dev)
 	}
 	class_destroy(vme_user_sysfs_class);
 
+	vme_dma_free(image[DMA_MINOR].resource);
+
 	for (i = MASTER_MINOR; i < (MASTER_MAX + 1); i++) {
 		kfree(image[i].kern_buf);
 		vme_master_free(image[i].resource);
diff --git a/drivers/staging/vme/devices/vme_user.h b/drivers/staging/vme/devices/vme_user.h
index b8cc7bc..252b1c9 100644
--- a/drivers/staging/vme/devices/vme_user.h
+++ b/drivers/staging/vme/devices/vme_user.h
@@ -48,11 +48,22 @@ struct vme_irq_id {
 	__u8 statid;
 };
 
+struct vme_dma_op {
+	__u64 vme_addr;		/* Starting Address on the VMEbus */
+	__u64 buf_vaddr;	/* Pointer to userspace memory */
+	__u32 count;		/* Count of bytes to copy */
+	__u32 aspace;		/* Address Space */
+	__u32 cycle;		/* Cycle properties */
+	__u32 dwidth;		/* Data transfer width */
+	__u32 dir;		/* Transfer Direction */
+};
+
 #define VME_GET_SLAVE _IOR(VME_IOC_MAGIC, 1, struct vme_slave)
 #define VME_SET_SLAVE _IOW(VME_IOC_MAGIC, 2, struct vme_slave)
 #define VME_GET_MASTER _IOR(VME_IOC_MAGIC, 3, struct vme_master)
 #define VME_SET_MASTER _IOW(VME_IOC_MAGIC, 4, struct vme_master)
 #define VME_IRQ_GEN _IOW(VME_IOC_MAGIC, 5, struct vme_irq_id)
+#define VME_DO_DMA _IOW(VME_IOC_MAGIC, 7, struct vme_dma_op)
 
 #endif /* _VME_USER_H_ */
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* Re: [PATCHv4 0/4] vme DMA and user space driver improvements
  2015-06-13 13:34               ` [PATCHv4 0/4] " Dmitry Kalinkin
                                   ` (3 preceding siblings ...)
  2015-06-13 13:34                 ` [PATCHv4 4/4] staging: vme_user: provide DMA functionality Dmitry Kalinkin
@ 2015-06-13 21:47                 ` Greg Kroah-Hartman
  4 siblings, 0 replies; 49+ messages in thread
From: Greg Kroah-Hartman @ 2015-06-13 21:47 UTC (permalink / raw)
  To: Dmitry Kalinkin
  Cc: linux-kernel, devel, Martyn Welch, Manohar Vanga, Igor Alekseev

On Sat, Jun 13, 2015 at 04:34:00PM +0300, Dmitry Kalinkin wrote:
> 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

Applied the first 3, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-06-13  0:28   ` Greg Kroah-Hartman
@ 2015-07-06 13:22     ` Martyn Welch
  2015-07-06 13:50       ` Dmitry Kalinkin
  0 siblings, 1 reply; 49+ messages in thread
From: Martyn Welch @ 2015-07-06 13:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Dmitry Kalinkin
  Cc: linux-kernel, devel, Manohar Vanga, Igor Alekseev

On 13/06/15 01:28, Greg Kroah-Hartman wrote:
> On Thu, May 28, 2015 at 03:07:05PM +0300, Dmitry Kalinkin wrote:
>> 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 <dmitry.kalinkin@gmail.com>
>> Cc: Igor Alekseev <igor.alekseev@itep.ru>
>> ---
>>   drivers/staging/vme/devices/vme_user.c | 201 ++++++++++++++++++++++++++++++++-
>>   drivers/staging/vme/devices/vme_user.h |  11 ++
>>   2 files changed, 209 insertions(+), 3 deletions(-)
>
> I want to get Martyn's feedback on this, as it's adding a new feature to
> the subsystem that he's going to have to maintain.
>

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?

-- 
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.welch@ge.com                  | VAT:GB 927559189

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-07-06 13:22     ` Martyn Welch
@ 2015-07-06 13:50       ` Dmitry Kalinkin
  2015-07-06 14:48         ` Martyn Welch
  0 siblings, 1 reply; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-07-06 13:50 UTC (permalink / raw)
  To: Martyn Welch
  Cc: Greg Kroah-Hartman, linux-kernel, devel, Manohar Vanga, Igor Alekseev

On Mon, Jul 6, 2015 at 4:22 PM, Martyn Welch <martyn.welch@ge.com> 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.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-07-06 13:50       ` Dmitry Kalinkin
@ 2015-07-06 14:48         ` Martyn Welch
  2015-07-06 17:24           ` Dmitry Kalinkin
  0 siblings, 1 reply; 49+ messages in thread
From: Martyn Welch @ 2015-07-06 14:48 UTC (permalink / raw)
  To: Dmitry Kalinkin
  Cc: Greg Kroah-Hartman, linux-kernel, devel, Manohar Vanga, Igor Alekseev



On 06/07/15 14:50, Dmitry Kalinkin wrote:
> On Mon, Jul 6, 2015 at 4:22 PM, Martyn Welch <martyn.welch@ge.com> 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?

> 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. 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), 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?


-- 
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.welch@ge.com                  | VAT:GB 927559189

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-07-06 14:48         ` Martyn Welch
@ 2015-07-06 17:24           ` Dmitry Kalinkin
  2015-07-07  7:13             ` Alessio Igor Bogani
                               ` (2 more replies)
  0 siblings, 3 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-07-06 17:24 UTC (permalink / raw)
  To: Martyn Welch
  Cc: Greg Kroah-Hartman, linux-kernel, devel, Manohar Vanga, Igor Alekseev

On Mon, Jul 6, 2015 at 5:48 PM, Martyn Welch <martyn.welch@ge.com> wrote:
>
>
> On 06/07/15 14:50, Dmitry Kalinkin wrote:
>>
>> On Mon, Jul 6, 2015 at 4:22 PM, Martyn Welch <martyn.welch@ge.com> 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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-07-06 17:24           ` Dmitry Kalinkin
@ 2015-07-07  7:13             ` Alessio Igor Bogani
       [not found]             ` <CAPk1OjEX7YX5J=yMPOGyGg7ZT6P-iKtaGRDDv2oARPFUcdnKnQ@mail.gmail.com>
  2015-07-08 13:22             ` Martyn Welch
  2 siblings, 0 replies; 49+ messages in thread
From: Alessio Igor Bogani @ 2015-07-07  7:13 UTC (permalink / raw)
  To: devel; +Cc: LKML

Hi Dmitry,

On 6 July 2015 at 19:24, Dmitry Kalinkin <dmitry.kalinkin@gmail.com> 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.

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.

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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
       [not found]             ` <CAPk1OjEX7YX5J=yMPOGyGg7ZT6P-iKtaGRDDv2oARPFUcdnKnQ@mail.gmail.com>
@ 2015-07-07 10:52               ` Dmitry Kalinkin
  2015-07-08 13:57                 ` Martyn Welch
       [not found]               ` <78FC1849-FFE4-49E5-8421-25D27324F790@gmail.com>
  2015-07-08 13:28               ` [PATCHv3 08/16] staging: vme_user: provide DMA functionality Martyn Welch
  2 siblings, 1 reply; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-07-07 10:52 UTC (permalink / raw)
  To: Alessio Igor Bogani
  Cc: Martyn Welch, devel, Greg Kroah-Hartman, Igor Alekseev, LKML,
	Manohar Vanga

Hi Alessio,

[Sorry for double post]

> On 07 Jul 2015, at 10:08, Alessio Igor Bogani <alessioigorbogani@gmail.com> wrote:
> 
> Hi Dmitry,
> 
> On 6 July 2015 at 19:24, Dmitry Kalinkin <dmitry.kalinkin@gmail.com> 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
> 


^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
       [not found]               ` <78FC1849-FFE4-49E5-8421-25D27324F790@gmail.com>
@ 2015-07-07 12:51                 ` Alessio Igor Bogani
  2015-07-07 13:04                   ` Dmitry Kalinkin
  2015-07-08 13:41                   ` Martyn Welch
  0 siblings, 2 replies; 49+ messages in thread
From: Alessio Igor Bogani @ 2015-07-07 12:51 UTC (permalink / raw)
  To: Dmitry Kalinkin
  Cc: Martyn Welch, devel, Greg Kroah-Hartman, Igor Alekseev, LKML,
	Manohar Vanga

Hi Dmitry,

On 7 July 2015 at 12:47, Dmitry Kalinkin <dmitry.kalinkin@gmail.com> wrote:
[...]
> 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.

Interesting approach.

> Other way to implement these would be to use DMA for everything, since it
> doesn’t
> have the limitations that the windows have.


> On 07 Jul 2015, at 10:08, Alessio Igor Bogani <alessioigorbogani@gmail.com>
> wrote:
[...]
>> 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...

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).

Ciao,
Alessio

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-07-07 12:51                 ` Alessio Igor Bogani
@ 2015-07-07 13:04                   ` Dmitry Kalinkin
  2015-07-08 13:41                   ` Martyn Welch
  1 sibling, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-07-07 13:04 UTC (permalink / raw)
  To: Alessio Igor Bogani
  Cc: Martyn Welch, devel, Greg Kroah-Hartman, Igor Alekseev, LKML,
	Manohar Vanga


> On 07 Jul 2015, at 15:51, Alessio Igor Bogani <alessioigorbogani@gmail.com> wrote:
> 
<snip>
>> 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

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-07-06 17:24           ` Dmitry Kalinkin
  2015-07-07  7:13             ` Alessio Igor Bogani
       [not found]             ` <CAPk1OjEX7YX5J=yMPOGyGg7ZT6P-iKtaGRDDv2oARPFUcdnKnQ@mail.gmail.com>
@ 2015-07-08 13:22             ` Martyn Welch
  2015-07-08 15:02               ` Generic VME UIO driver Dmitry Kalinkin
  2 siblings, 1 reply; 49+ messages in thread
From: Martyn Welch @ 2015-07-08 13:22 UTC (permalink / raw)
  To: Dmitry Kalinkin
  Cc: Greg Kroah-Hartman, linux-kernel, devel, Manohar Vanga, Igor Alekseev



On 06/07/15 18:24, Dmitry Kalinkin wrote:
>> 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.

Little I suspect. What it does mean is that there's no generic mechanism 
for clearing down an interrupt, so a device specific interrupt routine 
is required, which needs to be in kernel space.

>> >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:)
>

The aim of the existing APIs is to provide a mechanism for allocating 
resources. You're right, the resources are limited when scaling to a 32 
slot crate. There's a number of ways to share the resources, though they 
tend to all have trade offs. My experience has been that the majority of 
VME systems don't tend to stretch up to 32 cards.

> 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.

If vme_user grabs all the VME windows, there are no windows available 
for any kernel level VME drivers to use. If a kernel level driver loads 
before vme_user and is allocated a window, if vme_user demands 8 windows 
(and assuming it doesn't deal with some already having been allocated 
gracefully, which it doesn't at the moment) then it doesn't load. 
Dynamic allocation would leave "unused" resources available rather than 
prospectively hogging them.

-- 
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.welch@ge.com                  | VAT:GB 927559189

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
       [not found]             ` <CAPk1OjEX7YX5J=yMPOGyGg7ZT6P-iKtaGRDDv2oARPFUcdnKnQ@mail.gmail.com>
  2015-07-07 10:52               ` Dmitry Kalinkin
       [not found]               ` <78FC1849-FFE4-49E5-8421-25D27324F790@gmail.com>
@ 2015-07-08 13:28               ` Martyn Welch
  2 siblings, 0 replies; 49+ messages in thread
From: Martyn Welch @ 2015-07-08 13:28 UTC (permalink / raw)
  To: Alessio Igor Bogani, Dmitry Kalinkin
  Cc: devel, Greg Kroah-Hartman, Igor Alekseev, LKML, Manohar Vanga

On 07/07/15 08:08, Alessio Igor Bogani wrote:
> I would be glad to try it if the maintainer is willing to receive this
> type of changes.

Such requirements have come up in the past. I'd welcome such support 
being contributed to the kernel. My view has been that such an API could 
be built on top of the existing API. Does that seem workable to you?

-- 
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.welch@ge.com                  | VAT:GB 927559189

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-07-07 12:51                 ` Alessio Igor Bogani
  2015-07-07 13:04                   ` Dmitry Kalinkin
@ 2015-07-08 13:41                   ` Martyn Welch
  2015-07-08 14:39                     ` Dmitry Kalinkin
  2015-07-08 14:42                     ` [PATCH] vme: lower alignment requirement in pci bridge drivers Dmitry Kalinkin
  1 sibling, 2 replies; 49+ messages in thread
From: Martyn Welch @ 2015-07-08 13:41 UTC (permalink / raw)
  To: Alessio Igor Bogani, Dmitry Kalinkin
  Cc: devel, Greg Kroah-Hartman, Igor Alekseev, LKML, Manohar Vanga

On 07/07/15 13: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).

I think it's perfectly valid to use a single window to dynamically map 
to the address space belonging to one of a number of devices supported 
by a single driver. I think this is almost preferable to mapping a large 
window over a large portion of the VME address space to drive a number 
of devices as (depending on there spacing in the VME address space) the 
latter could cause issues with filling available PCI address space. 
Admittedly this is more of a problem on 32-bit systems, but...

-- 
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.welch@ge.com                  | VAT:GB 927559189

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-07-07 10:52               ` Dmitry Kalinkin
@ 2015-07-08 13:57                 ` Martyn Welch
  2015-07-08 14:47                   ` Dmitry Kalinkin
  0 siblings, 1 reply; 49+ messages in thread
From: Martyn Welch @ 2015-07-08 13:57 UTC (permalink / raw)
  To: Dmitry Kalinkin, Alessio Igor Bogani
  Cc: devel, Greg Kroah-Hartman, Igor Alekseev, LKML, Manohar Vanga



On 07/07/15 11:52, Dmitry Kalinkin wrote:
> 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.

I'm a little concerned by the latency this might cause, especially if 
there is one device which is negatively affected by latency. Handling 
RORA interrupts would be "interesting" if all the windows were 
dynamically allocated at the time at which an interrupt came in.

Martyn

-- 
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.welch@ge.com                  | VAT:GB 927559189

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-07-08 13:41                   ` Martyn Welch
@ 2015-07-08 14:39                     ` Dmitry Kalinkin
  2015-07-08 14:42                     ` [PATCH] vme: lower alignment requirement in pci bridge drivers Dmitry Kalinkin
  1 sibling, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-07-08 14:39 UTC (permalink / raw)
  To: Martyn Welch
  Cc: Alessio Igor Bogani, devel, Greg Kroah-Hartman, Igor Alekseev,
	LKML, Manohar Vanga


> On 08 Jul 2015, at 16:41, Martyn Welch <martyn.welch@ge.com> wrote:
> 
> On 07/07/15 13: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).
> 
> I think it's perfectly valid to use a single window to dynamically map to the address space belonging to one of a number of devices supported by a single driver. I think this is almost preferable to mapping a large window over a large portion of the VME address space to drive a number of devices as (depending on there spacing in the VME address space) the latter could cause issues with filling available PCI address space. Admittedly this is more of a problem on 32-bit systems, but…

Speaking of which. We do have 32 bit SBC’s (Fastwell CPC600) and there we can only map up to 32mb. If we drop an unnecessary alignment requirement, this can be raised up to 63mb. The next comes a hack that is a bit dirty: changing pdev->bus to something like pdev->bus->parent. Then maximal VME window size is only limited by vmalloc memory which on 32bit system can be raised up to ~700 mb by adding vmalloc=768M to kernel argument line.

For the reference, here is the iomem mapping of the working system without the hack (also some window mappings are already done):

% cat /proc/iomem 
00000000-00000fff : reserved
00001000-00097fff : System RAM
00098000-0009ffff : reserved
000a0000-000bffff : PCI Bus 0000:00
  000a0000-000bffff : Video RAM area
000c0000-000c7fff : Video ROM
000d0000-000d0fff : Adapter ROM
000d1000-000d29ff : Adapter ROM
000d3000-000d3fff : Adapter ROM
000d4000-000d4fff : Adapter ROM
000d8000-000dbfff : PCI Bus 0000:00
000dc000-000fffff : reserved
  000f0000-000fffff : System ROM
00100000-3de8ffff : System RAM
  01000000-01822261 : Kernel code
  01822262-01ca4e3f : Kernel data
  01da1000-01e6dfff : Kernel bss
3de90000-3de96fff : ACPI Tables
3de97000-3defffff : ACPI Non-volatile Storage
3df00000-3fffffff : reserved
40000000-febfffff : PCI Bus 0000:00
  d8000000-d807ffff : 0000:00:02.0
  d8080000-d80fffff : 0000:00:02.1
  d8100000-d810000f : 0000:00:1d.4
  d8100400-d81007ff : 0000:00:1d.7
    d8100400-d81007ff : ehci_hcd
  d8200000-dfffffff : PCI Bus 0000:02
    d8200000-d821ffff : 0000:02:00.0
      d8200000-d821ffff : e1000
    d8220000-d823ffff : 0000:02:00.1
      d8220000-d823ffff : e1000
    d8240000-d825ffff : 0000:02:01.0
      d8240000-d825ffff : e1000
    d8260000-d827ffff : 0000:02:01.1
      d8260000-d827ffff : e1000
    d8280000-d82fffff : vme_tsi148.7
    d8300000-d830ffff : vme_tsi148.0
    d8310000-d930ffff : vme_tsi148.3
    d9310000-d931ffff : vme_tsi148.2
    dc000000-dc000fff : 0000:02:02.0
      dc000000-dc000fff : vme_tsi148
  e8000000-efffffff : 0000:00:02.0
  f0000000-f7ffffff : 0000:00:02.1
fec00000-fec003ff : IOAPIC 0
fec10000-fec103ff : IOAPIC 1
fee00000-feefffff : pnp 00:01
  fee00000-fee00fff : Local APIC
    fee00000-fee00fff : pnp 00:02
feff0000-feffffff : reserved
  feff0000-feffffff : pnp 00:01
ff800000-ffffffff : INT0800:00
  ff800000-ffbfffff : reserved
  fffffc00-ffffffff : reserved


^ permalink raw reply	[flat|nested] 49+ messages in thread

* [PATCH] vme: lower alignment requirement in pci bridge drivers
  2015-07-08 13:41                   ` Martyn Welch
  2015-07-08 14:39                     ` Dmitry Kalinkin
@ 2015-07-08 14:42                     ` Dmitry Kalinkin
  1 sibling, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-07-08 14:42 UTC (permalink / raw)
  To: linux-kernel, devel
  Cc: Martyn Welch, Manohar Vanga, Greg Kroah-Hartman, Dmitry Kalinkin

Universe II allows PCI address grannularity of 4K or 64K depending on
the window id. tsi148 only supports 64K. Existing driver implementations
are validating window size against this grannularity and then use that
very size as alignment parameter to pci_bus_alloc_resource.  This
constraint is excessive, alignment by granularity should be enough.

This changes alignment constraint from size to a fixed constraint of
64K.

Signed-off-by: Dmitry Kalinkin <dmitry.kalinkin@gmail.com>
---
 drivers/vme/bridges/vme_ca91cx42.c | 2 +-
 drivers/vme/bridges/vme_tsi148.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/vme/bridges/vme_ca91cx42.c b/drivers/vme/bridges/vme_ca91cx42.c
index 834883d..b79a74a 100644
--- a/drivers/vme/bridges/vme_ca91cx42.c
+++ b/drivers/vme/bridges/vme_ca91cx42.c
@@ -553,7 +553,7 @@ static int ca91cx42_alloc_resource(struct vme_master_resource *image,
 	image->bus_resource.flags = IORESOURCE_MEM;
 
 	retval = pci_bus_alloc_resource(pdev->bus,
-		&image->bus_resource, size, size, PCIBIOS_MIN_MEM,
+		&image->bus_resource, size, 0x10000, PCIBIOS_MIN_MEM,
 		0, NULL, NULL);
 	if (retval) {
 		dev_err(ca91cx42_bridge->parent, "Failed to allocate mem "
diff --git a/drivers/vme/bridges/vme_tsi148.c b/drivers/vme/bridges/vme_tsi148.c
index b0132e0..d1e383b 100644
--- a/drivers/vme/bridges/vme_tsi148.c
+++ b/drivers/vme/bridges/vme_tsi148.c
@@ -768,7 +768,7 @@ static int tsi148_alloc_resource(struct vme_master_resource *image,
 	image->bus_resource.flags = IORESOURCE_MEM;
 
 	retval = pci_bus_alloc_resource(pdev->bus,
-		&image->bus_resource, size, size, PCIBIOS_MIN_MEM,
+		&image->bus_resource, size, 0x10000, PCIBIOS_MIN_MEM,
 		0, NULL, NULL);
 	if (retval) {
 		dev_err(tsi148_bridge->parent, "Failed to allocate mem "
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 49+ messages in thread

* Re: [PATCHv3 08/16] staging: vme_user: provide DMA functionality
  2015-07-08 13:57                 ` Martyn Welch
@ 2015-07-08 14:47                   ` Dmitry Kalinkin
  0 siblings, 0 replies; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-07-08 14:47 UTC (permalink / raw)
  To: Martyn Welch
  Cc: Alessio Igor Bogani, devel, Greg Kroah-Hartman, Igor Alekseev,
	LKML, Manohar Vanga


> On 08 Jul 2015, at 16:57, Martyn Welch <martyn.welch@ge.com> wrote:
> 
> 
> 
> On 07/07/15 11:52, Dmitry Kalinkin wrote:
>> 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.
> 
> I'm a little concerned by the latency this might cause, especially if there is one device which is negatively affected by latency. Handling RORA interrupts would be "interesting" if all the windows were dynamically allocated at the time at which an interrupt came in.
Latency-critical windows can be statically allocated using current resource based API.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Generic VME UIO driver
  2015-07-08 13:22             ` Martyn Welch
@ 2015-07-08 15:02               ` Dmitry Kalinkin
  2015-07-20  8:09                 ` Martyn Welch
  0 siblings, 1 reply; 49+ messages in thread
From: Dmitry Kalinkin @ 2015-07-08 15:02 UTC (permalink / raw)
  To: Martyn Welch
  Cc: Greg Kroah-Hartman, linux-kernel, devel, Manohar Vanga, Igor Alekseev


> On 08 Jul 2015, at 16:22, Martyn Welch <martyn.welch@ge.com> wrote:
> 
> On 06/07/15 18:24, Dmitry Kalinkin wrote:
>>> 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.
> 
> Little I suspect. What it does mean is that there's no generic mechanism for clearing down an interrupt, so a device specific interrupt routine is required, which needs to be in kernel space.
Yet PCI somehow managed to settle with UIO.
Now imagine I am working with a board using vme_user API and I need to
implement interrupts. The PCI case teaches me that I need to write a board specific
UIO driver. My board is ROAK and allows to configure it's interrupt to any level and
any status/id. So I only use a standard vme_irq_request API that generates IACK
cycle for me automatically. I also don’t want to limit my end user with a choice of
interrupt level and status/id and so I make it configurable. In the end I’ve got a very
generic ROAK device driver. What did I do wrong?

Cheers,
Dmitry

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: Generic VME UIO driver
  2015-07-08 15:02               ` Generic VME UIO driver Dmitry Kalinkin
@ 2015-07-20  8:09                 ` Martyn Welch
  0 siblings, 0 replies; 49+ messages in thread
From: Martyn Welch @ 2015-07-20  8:09 UTC (permalink / raw)
  To: Dmitry Kalinkin
  Cc: Greg Kroah-Hartman, linux-kernel, devel, Manohar Vanga, Igor Alekseev



On 08/07/15 16:02, Dmitry Kalinkin wrote:
>
>> On 08 Jul 2015, at 16:22, Martyn Welch <martyn.welch@ge.com> wrote:
>>
>> On 06/07/15 18:24, Dmitry Kalinkin wrote:
>>>> 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.
>>
>> Little I suspect. What it does mean is that there's no generic mechanism for clearing down an interrupt, so a device specific interrupt routine is required, which needs to be in kernel space.
> Yet PCI somehow managed to settle with UIO.

"If, on the other hand, your hardware needs some action to be performed 
after each interrupt, then you must do it in your kernel module."

https://www.kernel.org/doc/htmldocs/uio-howto/adding_irq_handler.html

> Now imagine I am working with a board using vme_user API and I need to
> implement interrupts. The PCI case teaches me that I need to write a board specific
> UIO driver. My board is ROAK and allows to configure it's interrupt to any level and
> any status/id. So I only use a standard vme_irq_request API that generates IACK
> cycle for me automatically. I also don’t want to limit my end user with a choice of
> interrupt level and status/id and so I make it configurable. In the end I’ve got a very
> generic ROAK device driver. What did I do wrong?
>

Nothing by the sounds of it. If you have ROAK hardware, life is a lot 
simpler.

Martyn

> Cheers,
> Dmitry
>

-- 
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.welch@ge.com                  | VAT:GB 927559189

^ permalink raw reply	[flat|nested] 49+ messages in thread

end of thread, other threads:[~2015-07-20  8:09 UTC | newest]

Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-28 12:06 [PATCHv3 00/16] vme DMA and user space driver improvements Dmitry Kalinkin
2015-05-28 12:06 ` [PATCHv3 01/16] Documentation: mention vme_master_mmap() in VME API Dmitry Kalinkin
2015-05-28 12:06 ` [PATCHv3 02/16] vme: tsi148: fix DMA lists longer that one item Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 03/16] vme: tsi148: fix first DMA item mapping Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 04/16] vme: stop DMA transfer on interruption Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 05/16] staging: vme_user: refactor llseek to switch(){} Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 06/16] vme: check for A64 overflow in vme_check_window() Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 07/16] vme: export vme_check_window() Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 08/16] staging: vme_user: provide DMA functionality Dmitry Kalinkin
2015-06-13  0:28   ` Greg Kroah-Hartman
2015-07-06 13:22     ` Martyn Welch
2015-07-06 13:50       ` Dmitry Kalinkin
2015-07-06 14:48         ` Martyn Welch
2015-07-06 17:24           ` Dmitry Kalinkin
2015-07-07  7:13             ` Alessio Igor Bogani
     [not found]             ` <CAPk1OjEX7YX5J=yMPOGyGg7ZT6P-iKtaGRDDv2oARPFUcdnKnQ@mail.gmail.com>
2015-07-07 10:52               ` Dmitry Kalinkin
2015-07-08 13:57                 ` Martyn Welch
2015-07-08 14:47                   ` Dmitry Kalinkin
     [not found]               ` <78FC1849-FFE4-49E5-8421-25D27324F790@gmail.com>
2015-07-07 12:51                 ` Alessio Igor Bogani
2015-07-07 13:04                   ` Dmitry Kalinkin
2015-07-08 13:41                   ` Martyn Welch
2015-07-08 14:39                     ` Dmitry Kalinkin
2015-07-08 14:42                     ` [PATCH] vme: lower alignment requirement in pci bridge drivers Dmitry Kalinkin
2015-07-08 13:28               ` [PATCHv3 08/16] staging: vme_user: provide DMA functionality Martyn Welch
2015-07-08 13:22             ` Martyn Welch
2015-07-08 15:02               ` Generic VME UIO driver Dmitry Kalinkin
2015-07-20  8:09                 ` Martyn Welch
2015-05-28 12:07 ` [PATCHv3 09/16] vme: ca91cx42: return error code on DMA error Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 10/16] vme: ca91cx42: fix LM_CTL address mask Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 11/16] staging: vme_user: remove unused counters Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 12/16] staging: vme_user: remove forward declarations Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 13/16] staging: vme_user: remove open/release Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 14/16] staging: vme_user: remove buf_unalloc helper Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 15/16] vme: tsi148: depend on HAS_DMA for Kconfig Dmitry Kalinkin
2015-05-28 12:07 ` [PATCHv3 16/16] vme: provide uapi header Dmitry Kalinkin
2015-06-13  0:30   ` Greg Kroah-Hartman
2015-05-31  3:06 ` [PATCHv3 00/16] vme DMA and user space driver improvements Greg Kroah-Hartman
2015-06-10 13:09   ` Dmitry Kalinkin
2015-06-13  0:31     ` Greg Kroah-Hartman
2015-06-13  2:04       ` Dmitry Kalinkin
2015-06-13  2:24         ` Greg Kroah-Hartman
2015-06-13  2:30           ` Dmitry Kalinkin
2015-06-13  4:40             ` Greg Kroah-Hartman
2015-06-13 13:34               ` [PATCHv4 0/4] " Dmitry Kalinkin
2015-06-13 13:34                 ` [PATCHv4 1/4] staging: vme_user: remove forward declarations Dmitry Kalinkin
2015-06-13 13:34                 ` [PATCHv4 2/4] staging: vme_user: remove open/release Dmitry Kalinkin
2015-06-13 13:34                 ` [PATCHv4 3/4] staging: vme_user: remove buf_unalloc helper Dmitry Kalinkin
2015-06-13 13:34                 ` [PATCHv4 4/4] staging: vme_user: provide DMA functionality Dmitry Kalinkin
2015-06-13 21:47                 ` [PATCHv4 0/4] vme DMA and user space driver improvements Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.