diff mbox series

[v5,4/4] uio_dmem_genirq: UIO_MEM_DMA_COHERENT conversion

Message ID 20240201233400.3394996-5-cleech@redhat.com (mailing list archive)
State Superseded
Headers show
Series UIO_MEM_DMA_COHERENT for cnic/bnx2/bnx2x | expand

Commit Message

Chris Leech Feb. 1, 2024, 11:34 p.m. UTC 
Conversion of this driver to use UIO_MEM_DMA_COHERENT for
dma_alloc_coherent memory instead of UIO_MEM_PHYS.

Signed-off-by: Chris Leech <cleech@redhat.com>
---
 drivers/uio/uio_dmem_genirq.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

Comments

Simon Horman Feb. 4, 2024, 10:19 a.m. UTC | #1 
On Thu, Feb 01, 2024 at 03:34:00PM -0800, Chris Leech wrote:
> Conversion of this driver to use UIO_MEM_DMA_COHERENT for
> dma_alloc_coherent memory instead of UIO_MEM_PHYS.
> 
> Signed-off-by: Chris Leech <cleech@redhat.com>
> ---
>  drivers/uio/uio_dmem_genirq.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c

...

> @@ -264,7 +257,8 @@ static int uio_dmem_genirq_probe(struct platform_device *pdev)
>  					" dynamic and fixed memory regions.\n");
>  			break;
>  		}
> -		uiomem->memtype = UIO_MEM_PHYS;
> +		uiomem->memtype = UIO_MEM_DMA_COHERENT;
> +		uiomem->dma_device = &pdev->dev,

Hi Chris,

a nit from my side.

Probably the ',' would be better written as a ';' here.
I don't think this is a bug, but using comma like this is
somewhat unexpected and confusing.

Flagged by clang-17 with -Wcomma


>  		uiomem->addr = DMEM_MAP_ERROR;
>  		uiomem->size = pdata->dynamic_region_sizes[i];
>  		++uiomem;
> -- 
> 2.43.0
> 
>
Chris Leech Feb. 5, 2024, 7:53 p.m. UTC | #2 
On Sun, Feb 04, 2024 at 10:19:03AM +0000, Simon Horman wrote:
> On Thu, Feb 01, 2024 at 03:34:00PM -0800, Chris Leech wrote:
...
> > @@ -264,7 +257,8 @@ static int uio_dmem_genirq_probe(struct platform_device *pdev)
> >  					" dynamic and fixed memory regions.\n");
> >  			break;
> >  		}
> > -		uiomem->memtype = UIO_MEM_PHYS;
> > +		uiomem->memtype = UIO_MEM_DMA_COHERENT;
> > +		uiomem->dma_device = &pdev->dev,
> 
> Hi Chris,
> 
> a nit from my side.
> 
> Probably the ',' would be better written as a ';' here.
> I don't think this is a bug, but using comma like this is
> somewhat unexpected and confusing.
> 
> Flagged by clang-17 with -Wcomma

That's an embarrassing typo to slip through.
I'll fix this,and add the kdoc comments for the API additions.

Thanks,
- Chris
diff mbox series

Patch

diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c
index 5313307c2754a..8634eba0ef2ab 100644
--- a/drivers/uio/uio_dmem_genirq.c
+++ b/drivers/uio/uio_dmem_genirq.c
@@ -36,7 +36,6 @@  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;
 };
@@ -50,7 +49,6 @@  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 dmem_region = priv->dmem_region_start;
 
 	uiomem = &priv->uioinfo->mem[priv->dmem_region_start];
 
@@ -61,11 +59,8 @@  static int uio_dmem_genirq_open(struct uio_info *info, struct inode *inode)
 			break;
 
 		addr = dma_alloc_coherent(&priv->pdev->dev, uiomem->size,
-				(dma_addr_t *)&uiomem->addr, GFP_KERNEL);
-		if (!addr) {
-			uiomem->addr = DMEM_MAP_ERROR;
-		}
-		priv->dmem_region_vaddr[dmem_region++] = addr;
+					  &uiomem->dma_addr, GFP_KERNEL);
+		uiomem->addr = addr ? (phys_addr_t) addr : DMEM_MAP_ERROR;
 		++uiomem;
 	}
 	priv->refcnt++;
@@ -80,7 +75,6 @@  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);
@@ -93,13 +87,12 @@  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;
-		if (priv->dmem_region_vaddr[dmem_region]) {
-			dma_free_coherent(&priv->pdev->dev, uiomem->size,
-					priv->dmem_region_vaddr[dmem_region],
-					uiomem->addr);
+		if (uiomem->addr) {
+			dma_free_coherent(uiomem->dma_device, uiomem->size,
+					  (void *) uiomem->addr,
+					  uiomem->dma_addr);
 		}
 		uiomem->addr = DMEM_MAP_ERROR;
-		++dmem_region;
 		++uiomem;
 	}
 
@@ -264,7 +257,8 @@  static int uio_dmem_genirq_probe(struct platform_device *pdev)
 					" dynamic and fixed memory regions.\n");
 			break;
 		}
-		uiomem->memtype = UIO_MEM_PHYS;
+		uiomem->memtype = UIO_MEM_DMA_COHERENT;
+		uiomem->dma_device = &pdev->dev,
 		uiomem->addr = DMEM_MAP_ERROR;
 		uiomem->size = pdata->dynamic_region_sizes[i];
 		++uiomem;