Here are a few fixes for the uio_dmem_genirq driver which allows for
dynamic allocation/deallocation of UIO memory resources via the
DMA-mapping API.
Damian Hobson-Garcia (4):
drivers: uio_dmem_genirq: Don't mix address spaces for dynamic region
vaddr
drivers: uio_dmem_genirq: Don't use DMA_ERROR_CODE to indicate
unmapped regions
drivers: uio_dmem_genirq: Allow partial success when opening device
drivers: uio: Only allocate new private data when probing device tree
node
Documentation/DocBook/uio-howto.tmpl | 2 +-
drivers/uio/uio_dmem_genirq.c | 25 +++++++++++++++----------
drivers/uio/uio_pdrv_genirq.c | 2 +-
3 files changed, 17 insertions(+), 12 deletions(-)
--
1.7.5.4
Assigning the virtual address returned from dma_alloc_coherent to the the
internal_addr element of uioinfo produces the following sparse errors since
internal_addr is a void __iomem * and dma_alloc_coherent returns void *.
+ drivers/uio/uio_dmem_genirq.c:65:39: sparse: incorrect type in assignment (different address spaces)
drivers/uio/uio_dmem_genirq.c:65:39: expected void [noderef] <asn:2>*internal_addr
drivers/uio/uio_dmem_genirq.c:65:39: got void *[assigned] addr
+ drivers/uio/uio_dmem_genirq.c:93:17: sparse: incorrect type in argument 3 (different address spaces)
drivers/uio/uio_dmem_genirq.c:93:17: expected void *vaddr
drivers/uio/uio_dmem_genirq.c:93:17: got void [noderef] <asn:2>*internal_addr
Store the void * in the driver's private data instead.
Reported-by: Fengguang Wu <[email protected]>
Signed-off-by: Damian Hobson-Garcia <[email protected]>
---
drivers/uio/uio_dmem_genirq.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c
index 4d4dd00..d8bbe07 100644
--- a/drivers/uio/uio_dmem_genirq.c
+++ b/drivers/uio/uio_dmem_genirq.c
@@ -37,6 +37,7 @@ struct uio_dmem_genirq_platdata {
struct platform_device *pdev;
unsigned int dmem_region_start;
unsigned int num_dmem_regions;
+ void *dmem_region_vaddr[MAX_UIO_MAPS];
struct mutex alloc_lock;
unsigned int refcnt;
};
@@ -46,6 +47,7 @@ static int uio_dmem_genirq_open(struct uio_info *info, struct inode *inode)
struct uio_dmem_genirq_platdata *priv = info->priv;
struct uio_mem *uiomem;
int ret = 0;
+ int dmem_region = priv->dmem_region_start;
uiomem = &priv->uioinfo->mem[priv->dmem_region_start];
@@ -61,8 +63,7 @@ static int uio_dmem_genirq_open(struct uio_info *info, struct inode *inode)
ret = -ENOMEM;
break;
}
-
- uiomem->internal_addr = addr;
+ priv->dmem_region_vaddr[dmem_region++] = addr;
++uiomem;
}
priv->refcnt++;
@@ -77,6 +78,7 @@ static int uio_dmem_genirq_release(struct uio_info *info, struct inode *inode)
{
struct uio_dmem_genirq_platdata *priv = info->priv;
struct uio_mem *uiomem;
+ int dmem_region = priv->dmem_region_start;
/* Tell the Runtime PM code that the device has become idle */
pm_runtime_put_sync(&priv->pdev->dev);
@@ -91,7 +93,8 @@ static int uio_dmem_genirq_release(struct uio_info *info, struct inode *inode)
break;
dma_free_coherent(&priv->pdev->dev, uiomem->size,
- uiomem->internal_addr, uiomem->addr);
+ priv->dmem_region_vaddr[dmem_region++],
+ uiomem->addr);
uiomem->addr = DMA_ERROR_CODE;
++uiomem;
}
--
1.7.5.4
DMA_ERROR_CODE is not defined on all architectures and is architecture
specific. Instead, use the constant, ~0 to indicate unmapped regions.
Reported-by: Fengguang Wu <[email protected]>
Reported-by: Geert Uytterhoeven <[email protected]>
Signed-off-by: Damian Hobson-Garcia <[email protected]>
---
Documentation/DocBook/uio-howto.tmpl | 2 +-
drivers/uio/uio_dmem_genirq.c | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/Documentation/DocBook/uio-howto.tmpl b/Documentation/DocBook/uio-howto.tmpl
index fdbf86f..ddb05e9 100644
--- a/Documentation/DocBook/uio-howto.tmpl
+++ b/Documentation/DocBook/uio-howto.tmpl
@@ -771,7 +771,7 @@ framework to set up sysfs files for this region. Simply leave it alone.
<varname>/sys/class/uio/uioX/maps/mapY/*</varname>.
The dynmaic memory regions will be freed when the UIO device file is
closed. When no processes are holding the device file open, the address
- returned to userspace is DMA_ERROR_CODE.
+ returned to userspace is ~0.
</para>
</sect1>
diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c
index d8bbe07..7be8d04 100644
--- a/drivers/uio/uio_dmem_genirq.c
+++ b/drivers/uio/uio_dmem_genirq.c
@@ -29,6 +29,7 @@
#include <linux/of_address.h>
#define DRIVER_NAME "uio_dmem_genirq"
+#define DMEM_MAP_ERROR (~0)
struct uio_dmem_genirq_platdata {
struct uio_info *uioinfo;
@@ -60,6 +61,7 @@ static int uio_dmem_genirq_open(struct uio_info *info, struct inode *inode)
addr = dma_alloc_coherent(&priv->pdev->dev, uiomem->size,
(dma_addr_t *)&uiomem->addr, GFP_KERNEL);
if (!addr) {
+ uiomem->addr = DMEM_MAP_ERROR;
ret = -ENOMEM;
break;
}
@@ -95,7 +97,7 @@ static int uio_dmem_genirq_release(struct uio_info *info, struct inode *inode)
dma_free_coherent(&priv->pdev->dev, uiomem->size,
priv->dmem_region_vaddr[dmem_region++],
uiomem->addr);
- uiomem->addr = DMA_ERROR_CODE;
+ uiomem->addr = DMEM_MAP_ERROR;
++uiomem;
}
@@ -238,7 +240,7 @@ static int uio_dmem_genirq_probe(struct platform_device *pdev)
break;
}
uiomem->memtype = UIO_MEM_PHYS;
- uiomem->addr = DMA_ERROR_CODE;
+ uiomem->addr = DMEM_MAP_ERROR;
uiomem->size = pdata->dynamic_region_sizes[i];
++uiomem;
}
--
1.7.5.4
The uio device should not fail on open just because one memory allocation
fails. The device might export several regions, the failure of some of
which may or may not be a problem for the user space driver. Failing
regions will remain unmapped, and successful regions will be mapped and
exported to user space. Also deals with the case where failing to map
a region after successfully allocating others would not unmap the
successfully allocated regions before dying.
Signed-off-by: Damian Hobson-Garcia <[email protected]>
---
drivers/uio/uio_dmem_genirq.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c
index 7be8d04..bbdf925 100644
--- a/drivers/uio/uio_dmem_genirq.c
+++ b/drivers/uio/uio_dmem_genirq.c
@@ -62,8 +62,6 @@ static int uio_dmem_genirq_open(struct uio_info *info, struct inode *inode)
(dma_addr_t *)&uiomem->addr, GFP_KERNEL);
if (!addr) {
uiomem->addr = DMEM_MAP_ERROR;
- ret = -ENOMEM;
- break;
}
priv->dmem_region_vaddr[dmem_region++] = addr;
++uiomem;
@@ -93,11 +91,13 @@ static int uio_dmem_genirq_release(struct uio_info *info, struct inode *inode)
while (!priv->refcnt && uiomem < &priv->uioinfo->mem[MAX_UIO_MAPS]) {
if (!uiomem->size)
break;
-
- dma_free_coherent(&priv->pdev->dev, uiomem->size,
- priv->dmem_region_vaddr[dmem_region++],
- uiomem->addr);
+ if (priv->dmem_region_vaddr[dmem_region]) {
+ dma_free_coherent(&priv->pdev->dev, uiomem->size,
+ priv->dmem_region_vaddr[dmem_region],
+ uiomem->addr);
+ }
uiomem->addr = DMEM_MAP_ERROR;
+ ++dmem_region;
++uiomem;
}
--
1.7.5.4
The same condition should be used both when allocating and freeing the
driver private data. When dev.of_node is non NULL, allocate a new
private data structure, otherwise use the values from the platform data.
Reported-by: Fengguang Wu <[email protected]>
Signed-off-by: Damian Hobson-Garcia <[email protected]>
---
drivers/uio/uio_dmem_genirq.c | 2 +-
drivers/uio/uio_pdrv_genirq.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c
index bbdf925..252434c 100644
--- a/drivers/uio/uio_dmem_genirq.c
+++ b/drivers/uio/uio_dmem_genirq.c
@@ -153,7 +153,7 @@ static int uio_dmem_genirq_probe(struct platform_device *pdev)
int ret = -EINVAL;
int i;
- if (!uioinfo) {
+ if (pdev->dev.of_node) {
int irq;
/* alloc uioinfo for one device */
diff --git a/drivers/uio/uio_pdrv_genirq.c b/drivers/uio/uio_pdrv_genirq.c
index 42202cd..45fcceb 100644
--- a/drivers/uio/uio_pdrv_genirq.c
+++ b/drivers/uio/uio_pdrv_genirq.c
@@ -102,7 +102,7 @@ static int uio_pdrv_genirq_probe(struct platform_device *pdev)
int ret = -EINVAL;
int i;
- if (!uioinfo) {
+ if (pdev->dev.of_node) {
int irq;
/* alloc uioinfo for one device */
--
1.7.5.4