[meta-xilinx] [linux-xlnx] uio as module fails to link

[Hyun Kwon]

Hi JFD,

Thanks for taking a look.

On Tue, 2019-07-09 at 05:26:43 -0700, Jean-Francois Dagenais wrote:
> Quick turn-around!
> 
> > On Jul 8, 2019, at 14:51, Hyun Kwon <hyun.kwon at xilinx.com> wrote:
> > 
> > Hi JFD,
> > 
> > Thanks for reporting the issue.
> > 
> > On Mon, 2019-07-08 at 09:01:03 -0700, Jean-Francois Dagenais wrote:
> >> Anyone tried that? I get:
> >> 
> >> WARNING: modpost: missing MODULE_LICENSE() in drivers/uio/uio_dmabuf.o
> >> see include/linux/module.h for more information
> >> ERROR: "uio_dmabuf_map" [drivers/uio/uio.ko] undefined!
> >> ERROR: "uio_dmabuf_unmap" [drivers/uio/uio.ko] undefined!
> >> ERROR: "uio_dmabuf_cleanup" [drivers/uio/uio.ko] undefined!
> >> 
> >> uio/Makefile shows:
> >> obj-$(CONFIG_UIO)	+= uio.o uio_dmabuf.o
> >> 
> >> I got it to compile by hacking uio.c to #include uio_dmabuf.c instead of the .h but that's probably not the correct way to fix this.
> >> 
> > 
> > The easiest fix would be to merge uio_dmabuf.c into uio.c. Please see
> > the attached patch and let me know if you see any other better way.
> 
> > Hehe, that was essentially what my #include "uio_dmabuf.c" was doing. This patch
> > fixes the problem. See my suggestion below.

Indeed. :-) It doesn't seem to work by fixing the makefile.

> > 
> > Thanks,
> > -hyun
> > 
> > From 84cbe02cf2775dca911c2b734dded794150698a8 Mon Sep 17 00:00:00 2001
> > From: Hyun Kwon <hyun.kwon at xilinx.com>
> > Date: Mon, 8 Jul 2019 11:37:03 -0700
> > Subject: [PATCH 1/1] uio: dmabuf: Merge the dmabuf functions into uio.c
> > 
> > With a separate uio_dmabuf.c, it's tricky to build the uio as
> > a separate kernel module.
> > 
> > Reported-by: Jean-Francois Dagenais <jeff.dagenais at gmail.com>
> > Signed-off-by: Hyun Kwon <hyun.kwon at xilinx.com>
> 
> Tested-by: Jean-Francois Dagenais <jeff.dagenais at gmail.com>
> 

Thanks. I'll take this tag into this patch.

> > ---
> > drivers/uio/Makefile     |   2 +-
> > drivers/uio/uio.c        | 191 ++++++++++++++++++++++++++++++++++++++++++
> > drivers/uio/uio_dmabuf.c | 210 -----------------------------------------------
> > 3 files changed, 192 insertions(+), 211 deletions(-)
> > delete mode 100644 drivers/uio/uio_dmabuf.c
> > 
> > diff --git a/drivers/uio/Makefile b/drivers/uio/Makefile
> > index 7af888a..aea3e17 100644
> > --- a/drivers/uio/Makefile
> > +++ b/drivers/uio/Makefile
> > @@ -1,5 +1,5 @@
> > # SPDX-License-Identifier: GPL-2.0
> > -obj-$(CONFIG_UIO)	+= uio.o uio_dmabuf.o
> > +obj-$(CONFIG_UIO)	+= uio.o
> > obj-$(CONFIG_UIO_CIF)	+= uio_cif.o
> > obj-$(CONFIG_UIO_PDRV_GENIRQ)	+= uio_pdrv_genirq.o
> > obj-$(CONFIG_UIO_DMEM_GENIRQ)	+= uio_dmem_genirq.o
> > diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> > index e054fa7..175fb6b 100644
> > --- a/drivers/uio/uio.c
> > +++ b/drivers/uio/uio.c
> > @@ -26,6 +26,7 @@
> > #include <linux/uio_driver.h>
> > #include <linux/list.h>
> > #include <linux/mutex.h>
> > +#include <linux/dma-buf.h>
> > 
> > #include <uapi/linux/uio/uio.h>
> > 
> > @@ -455,6 +456,196 @@ static irqreturn_t uio_interrupt(int irq, void *dev_id)
> > 	return ret;
> > }
> 
> Would it be better to put all this dmabuf inside a
> #ifdef CONFIG_DMA_SHARED_BUFFER
> maybe? (Or IS_REACHABLE(CONFIG_DMA_SHARED_BUFFER), not sure which formulation works for all cases...)

The Kconfig takes care of it by selecting CONFIG_DMA_SHARED_BUFFER because
I didn't want to add #ifdefs for downstream changes.

Thanks,
-hyun

> 
> > 
> > +struct uio_dmabuf_mem {
> > +	int dbuf_fd;
> > +	struct dma_buf *dbuf;
> > +	struct dma_buf_attachment *dbuf_attach;
> > +	struct sg_table *sgt;
> > +	enum dma_data_direction dir;
> > +	struct list_head list;
> > +};
> > +
> > +long uio_dmabuf_map(struct uio_device *dev, struct list_head *dbufs,
> > +		    struct mutex *dbufs_lock, void __user *user_args)
> > +{
> > +	struct uio_dmabuf_args args;
> > +	struct uio_dmabuf_mem *dbuf_mem;
> > +	struct dma_buf *dbuf;
> > +	struct dma_buf_attachment *dbuf_attach;
> > +	enum dma_data_direction dir;
> > +	struct sg_table *sgt;
> > +	long ret;
> > +
> > +	if (copy_from_user(&args, user_args, sizeof(args))) {
> > +		ret = -EFAULT;
> > +		dev_err(dev->dev.parent, "failed to copy from user\n");
> > +		goto err;
> > +	}
> > +
> > +	dbuf = dma_buf_get(args.dbuf_fd);
> > +	if (IS_ERR(dbuf)) {
> > +		dev_err(dev->dev.parent, "failed to get dmabuf\n");
> > +		return PTR_ERR(dbuf);
> > +	}
> > +
> > +	dbuf_attach = dma_buf_attach(dbuf, dev->dev.parent);
> > +	if (IS_ERR(dbuf_attach)) {
> > +		dev_err(dev->dev.parent, "failed to attach dmabuf\n");
> > +		ret = PTR_ERR(dbuf_attach);
> > +		goto err_put;
> > +	}
> > +
> > [...]
>