[PATCH] uio: uio_pci_generic: add memory mappings

[PATCH] uio: uio_pci_generic: add memory mappings

[Firas Ashkar]

import memory resources from underlying pci device, thus allowing
userspace applications to memory map those resources.

Signed-off-by: Firas Ashkar <firas.ashkar@savoirfairelinux.com>
---
:100644 100644 c7d681fef198 809eca95b5bb M	drivers/uio/uio_pci_generic.c
 drivers/uio/uio_pci_generic.c | 52 +++++++++++++++++++++++++++++------
 1 file changed, 43 insertions(+), 9 deletions(-)

diff --git a/drivers/uio/uio_pci_generic.c b/drivers/uio/uio_pci_generic.c
index c7d681fef198..809eca95b5bb 100644
--- a/drivers/uio/uio_pci_generic.c
+++ b/drivers/uio/uio_pci_generic.c
@@ -24,9 +24,9 @@
 #include <linux/slab.h>
 #include <linux/uio_driver.h>
 
-#define DRIVER_VERSION	"0.01.0"
-#define DRIVER_AUTHOR	"Michael S. Tsirkin <mst@redhat.com>"
-#define DRIVER_DESC	"Generic UIO driver for PCI 2.3 devices"
+#define DRIVER_VERSION "0.01.0"
+#define DRIVER_AUTHOR "Michael S. Tsirkin <mst@redhat.com>"
+#define DRIVER_DESC "Generic UIO driver for PCI 2.3 devices"
 
 struct uio_pci_generic_dev {
 	struct uio_info info;
@@ -56,7 +56,8 @@ static int release(struct uio_info *info, struct inode *inode)
 }
 
 /* Interrupt handler. Read/modify/write the command register to disable
- * the interrupt. */
+ * the interrupt.
+ */
 static irqreturn_t irqhandler(int irq, struct uio_info *info)
 {
 	struct uio_pci_generic_dev *gdev = to_uio_pci_generic_dev(info);
@@ -68,11 +69,12 @@ static irqreturn_t irqhandler(int irq, struct uio_info *info)
 	return IRQ_HANDLED;
 }
 
-static int probe(struct pci_dev *pdev,
-			   const struct pci_device_id *id)
+static int probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct uio_pci_generic_dev *gdev;
+	struct uio_mem *uiomem;
 	int err;
+	int i;
 
 	err = pcim_enable_device(pdev);
 	if (err) {
@@ -84,7 +86,8 @@ static int probe(struct pci_dev *pdev,
 	if (pdev->irq && !pci_intx_mask_supported(pdev))
 		return -ENOMEM;
 
-	gdev = devm_kzalloc(&pdev->dev, sizeof(struct uio_pci_generic_dev), GFP_KERNEL);
+	gdev = devm_kzalloc(&pdev->dev, sizeof(struct uio_pci_generic_dev),
+			    GFP_KERNEL);
 	if (!gdev)
 		return -ENOMEM;
 
@@ -97,8 +100,39 @@ static int probe(struct pci_dev *pdev,
 		gdev->info.irq_flags = IRQF_SHARED;
 		gdev->info.handler = irqhandler;
 	} else {
-		dev_warn(&pdev->dev, "No IRQ assigned to device: "
-			 "no support for interrupts?\n");
+		dev_warn(
+			&pdev->dev,
+			"No IRQ assigned to device: no support for interrupts?\n");
+	}
+
+	uiomem = &gdev->info.mem[0];
+	for (i = 0; i < MAX_UIO_MAPS; ++i) {
+		struct resource *r = &pdev->resource[i];
+
+		if (r->flags != (IORESOURCE_SIZEALIGN | IORESOURCE_MEM))
+			continue;
+
+		if (uiomem >= &gdev->info.mem[MAX_UIO_MAPS]) {
+			dev_warn(
+				&pdev->dev,
+				"device has more than " __stringify(
+					MAX_UIO_MAPS) " I/O memory resources.\n");
+			break;
+		}
+
+		uiomem->memtype = UIO_MEM_PHYS;
+		uiomem->addr = r->start & PAGE_MASK;
+		uiomem->offs = r->start & ~PAGE_MASK;
+		uiomem->size =
+			(uiomem->offs + resource_size(r) + PAGE_SIZE - 1) &
+			PAGE_MASK;
+		uiomem->name = r->name;
+		++uiomem;
+	}
+
+	while (uiomem < &gdev->info.mem[MAX_UIO_MAPS]) {
+		uiomem->size = 0;
+		++uiomem;
 	}
 
 	return devm_uio_register_device(&pdev->dev, &gdev->info);
-- 
2.25.1

Re: [PATCH] uio: uio_pci_generic: add memory mappings

[Greg KH]

On Mon, Apr 26, 2021 at 03:03:46PM -0400, Firas Ashkar wrote:
> import memory resources from underlying pci device, thus allowing
> userspace applications to memory map those resources.

You also did other things in this patch that have nothing to do with
this change, why?

Always describe what your patch does properly, otherwise we have to
ignore it.

> 
> Signed-off-by: Firas Ashkar <firas.ashkar@savoirfairelinux.com>
> ---
> :100644 100644 c7d681fef198 809eca95b5bb M	drivers/uio/uio_pci_generic.c
>  drivers/uio/uio_pci_generic.c | 52 +++++++++++++++++++++++++++++------
>  1 file changed, 43 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/uio/uio_pci_generic.c b/drivers/uio/uio_pci_generic.c
> index c7d681fef198..809eca95b5bb 100644
> --- a/drivers/uio/uio_pci_generic.c
> +++ b/drivers/uio/uio_pci_generic.c
> @@ -24,9 +24,9 @@
>  #include <linux/slab.h>
>  #include <linux/uio_driver.h>
>  
> -#define DRIVER_VERSION	"0.01.0"
> -#define DRIVER_AUTHOR	"Michael S. Tsirkin <mst@redhat.com>"
> -#define DRIVER_DESC	"Generic UIO driver for PCI 2.3 devices"
> +#define DRIVER_VERSION "0.01.0"
> +#define DRIVER_AUTHOR "Michael S. Tsirkin <mst@redhat.com>"
> +#define DRIVER_DESC "Generic UIO driver for PCI 2.3 devices"


Like this, why change these lines???

thanks,

greg k-h

Re: [PATCH] uio: uio_pci_generic: add memory mappings

[firas ashkar]

Hi,
The reason for these extra changes is the result of running 
fashkar@barbarian:~/Downloads/linux_mainline$ clang-format -style=file
-i drivers/uio/uio_pci_generic.c

fashkar@barbarian:~/Downloads/linux_mainline$ ./scripts/checkpatch.pl
0001-uio-uio_pci_generic-add-memory-mappings.patch

i shall undo those changes and retry again, ty
-- 
Firas Ashkar
Developpeur Système Embarqué

savoirfairelinux.com  | Montréal, Québec

Tél.: +1 514 276 5468 ext. 118



          

On Tue, 2021-04-27 at 08:20 +0200, Greg KH wrote:
> On Mon, Apr 26, 2021 at 03:03:46PM -0400, Firas Ashkar wrote:
> > import memory resources from underlying pci device, thus allowing
> > userspace applications to memory map those resources.
> 
> You also did other things in this patch that have nothing to do with
> this change, why?
> 
> Always describe what your patch does properly, otherwise we have to
> ignore it.
> 
> > Signed-off-by: Firas Ashkar <firas.ashkar@savoirfairelinux.com>
> > ---
> > :100644 100644 c7d681fef198 809eca95b5bb M	drivers/uio/uio_pci_gen
> > eric.c
> >  drivers/uio/uio_pci_generic.c | 52 +++++++++++++++++++++++++++++
> > ------
> >  1 file changed, 43 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/uio/uio_pci_generic.c
> > b/drivers/uio/uio_pci_generic.c
> > index c7d681fef198..809eca95b5bb 100644
> > --- a/drivers/uio/uio_pci_generic.c
> > +++ b/drivers/uio/uio_pci_generic.c
> > @@ -24,9 +24,9 @@
> >  #include <linux/slab.h>
> >  #include <linux/uio_driver.h>
> >  
> > -#define DRIVER_VERSION	"0.01.0"
> > -#define DRIVER_AUTHOR	"Michael S. Tsirkin <mst@redhat.com>"
> > -#define DRIVER_DESC	"Generic UIO driver for PCI 2.3
> > devices"
> > +#define DRIVER_VERSION "0.01.0"
> > +#define DRIVER_AUTHOR "Michael S. Tsirkin <mst@redhat.com>"
> > +#define DRIVER_DESC "Generic UIO driver for PCI 2.3 devices"
> 
> Like this, why change these lines???
> 
> thanks,
> 
> greg k-h

Re: [PATCH] uio: uio_pci_generic: add memory mappings

[Greg KH]

On Tue, Apr 27, 2021 at 10:06:39AM -0400, firas ashkar wrote:
> Hi,
> The reason for these extra changes is the result of running 
> fashkar@barbarian:~/Downloads/linux_mainline$ clang-format -style=file
> -i drivers/uio/uio_pci_generic.c

Please do not mix coding style changes with real fixes/additions.

And always review what a tool does, some of those changes were wrong.

thanks,

greg k-h