Re: [PATCH v7 4/4] bus: mhi: Add userspace client interface driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hemant,

I love your patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on staging/staging-testing linus/master next-20201016]
[cannot apply to linux/master v5.9]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Hemant-Kumar/userspace-MHI-client-interface-driver/20201017-140145
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git f3277cbfba763cd2826396521b9296de67cf1bbc
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/6f44d9c0efd29cbd60a4c26843462a573050a520
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hemant-Kumar/userspace-MHI-client-interface-driver/20201017-140145
        git checkout 6f44d9c0efd29cbd60a4c26843462a573050a520
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=m68k 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@xxxxxxxxx>

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:11,
                    from drivers/bus/mhi/uci.c:4:
   include/linux/scatterlist.h: In function 'sg_set_buf':
   arch/m68k/include/asm/page_mm.h:169:49: warning: ordered comparison of pointer with null pointer [-Wextra]
     169 | #define virt_addr_valid(kaddr) ((void *)(kaddr) >= (void *)PAGE_OFFSET && (void *)(kaddr) < high_memory)
         |                                                 ^~
   include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
      78 | # define unlikely(x) __builtin_expect(!!(x), 0)
         |                                          ^
   include/linux/scatterlist.h:143:2: note: in expansion of macro 'BUG_ON'
     143 |  BUG_ON(!virt_addr_valid(buf));
         |  ^~~~~~
   include/linux/scatterlist.h:143:10: note: in expansion of macro 'virt_addr_valid'
     143 |  BUG_ON(!virt_addr_valid(buf));
         |          ^~~~~~~~~~~~~~~
   In file included from include/linux/printk.h:405,
                    from include/linux/kernel.h:15,
                    from drivers/bus/mhi/uci.c:4:
   drivers/bus/mhi/uci.c: In function 'mhi_queue_inbound':
>> drivers/bus/mhi/uci.c:147:16: warning: format '%ld' expects argument of type 'long int', but argument 6 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
     147 |   dev_dbg(dev, "Allocated buf %d of %d size %ld\n", i, nr_trbs,
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:129:15: note: in definition of macro '__dynamic_func_call'
     129 |   func(&id, ##__VA_ARGS__);  \
         |               ^~~~~~~~~~~
   include/linux/dynamic_debug.h:161:2: note: in expansion of macro '_dynamic_func_call'
     161 |  _dynamic_func_call(fmt,__dynamic_dev_dbg,   \
         |  ^~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:2: note: in expansion of macro 'dynamic_dev_dbg'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |  ^~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:23: note: in expansion of macro 'dev_fmt'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                       ^~~~~~~
   drivers/bus/mhi/uci.c:147:3: note: in expansion of macro 'dev_dbg'
     147 |   dev_dbg(dev, "Allocated buf %d of %d size %ld\n", i, nr_trbs,
         |   ^~~~~~~
   drivers/bus/mhi/uci.c:147:47: note: format string is defined here
     147 |   dev_dbg(dev, "Allocated buf %d of %d size %ld\n", i, nr_trbs,
         |                                             ~~^
         |                                               |
         |                                               long int
         |                                             %d
   In file included from include/linux/printk.h:405,
                    from include/linux/kernel.h:15,
                    from drivers/bus/mhi/uci.c:4:
   drivers/bus/mhi/uci.c: In function 'mhi_uci_write':
>> drivers/bus/mhi/uci.c:308:15: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
     308 |  dev_dbg(dev, "%s: to xfer: %lu bytes\n", __func__, count);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:129:15: note: in definition of macro '__dynamic_func_call'
     129 |   func(&id, ##__VA_ARGS__);  \
         |               ^~~~~~~~~~~
   include/linux/dynamic_debug.h:161:2: note: in expansion of macro '_dynamic_func_call'
     161 |  _dynamic_func_call(fmt,__dynamic_dev_dbg,   \
         |  ^~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:2: note: in expansion of macro 'dynamic_dev_dbg'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |  ^~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:23: note: in expansion of macro 'dev_fmt'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                       ^~~~~~~
   drivers/bus/mhi/uci.c:308:2: note: in expansion of macro 'dev_dbg'
     308 |  dev_dbg(dev, "%s: to xfer: %lu bytes\n", __func__, count);
         |  ^~~~~~~
   drivers/bus/mhi/uci.c:308:31: note: format string is defined here
     308 |  dev_dbg(dev, "%s: to xfer: %lu bytes\n", __func__, count);
         |                             ~~^
         |                               |
         |                               long unsigned int
         |                             %u
   In file included from include/linux/printk.h:405,
                    from include/linux/kernel.h:15,
                    from drivers/bus/mhi/uci.c:4:
   drivers/bus/mhi/uci.c:366:15: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
     366 |  dev_dbg(dev, "%s: bytes xferred: %lu\n", __func__, bytes_xfered);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:129:15: note: in definition of macro '__dynamic_func_call'
     129 |   func(&id, ##__VA_ARGS__);  \
         |               ^~~~~~~~~~~
   include/linux/dynamic_debug.h:161:2: note: in expansion of macro '_dynamic_func_call'
     161 |  _dynamic_func_call(fmt,__dynamic_dev_dbg,   \
         |  ^~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:2: note: in expansion of macro 'dynamic_dev_dbg'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |  ^~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:23: note: in expansion of macro 'dev_fmt'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                       ^~~~~~~
   drivers/bus/mhi/uci.c:366:2: note: in expansion of macro 'dev_dbg'
     366 |  dev_dbg(dev, "%s: bytes xferred: %lu\n", __func__, bytes_xfered);
         |  ^~~~~~~
   drivers/bus/mhi/uci.c:366:37: note: format string is defined here
     366 |  dev_dbg(dev, "%s: bytes xferred: %lu\n", __func__, bytes_xfered);
         |                                   ~~^
         |                                     |
         |                                     long unsigned int
         |                                   %u
   In file included from include/linux/printk.h:405,
                    from include/linux/kernel.h:15,
                    from drivers/bus/mhi/uci.c:4:
   drivers/bus/mhi/uci.c: In function 'mhi_uci_read':
   drivers/bus/mhi/uci.c:447:15: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
     447 |  dev_dbg(dev, "Copied %lu of %lu bytes\n", to_copy, uchan->dl_size);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:129:15: note: in definition of macro '__dynamic_func_call'
     129 |   func(&id, ##__VA_ARGS__);  \
         |               ^~~~~~~~~~~
   include/linux/dynamic_debug.h:161:2: note: in expansion of macro '_dynamic_func_call'
     161 |  _dynamic_func_call(fmt,__dynamic_dev_dbg,   \
         |  ^~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:2: note: in expansion of macro 'dynamic_dev_dbg'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |  ^~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:23: note: in expansion of macro 'dev_fmt'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                       ^~~~~~~
   drivers/bus/mhi/uci.c:447:2: note: in expansion of macro 'dev_dbg'
     447 |  dev_dbg(dev, "Copied %lu of %lu bytes\n", to_copy, uchan->dl_size);
         |  ^~~~~~~
   drivers/bus/mhi/uci.c:447:25: note: format string is defined here
     447 |  dev_dbg(dev, "Copied %lu of %lu bytes\n", to_copy, uchan->dl_size);
         |                       ~~^
         |                         |
         |                         long unsigned int
         |                       %u
   In file included from include/linux/printk.h:405,
                    from include/linux/kernel.h:15,
                    from drivers/bus/mhi/uci.c:4:
   drivers/bus/mhi/uci.c:447:15: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'size_t' {aka 'unsigned int'} [-Wformat=]
     447 |  dev_dbg(dev, "Copied %lu of %lu bytes\n", to_copy, uchan->dl_size);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:129:15: note: in definition of macro '__dynamic_func_call'
     129 |   func(&id, ##__VA_ARGS__);  \
         |               ^~~~~~~~~~~
   include/linux/dynamic_debug.h:161:2: note: in expansion of macro '_dynamic_func_call'
     161 |  _dynamic_func_call(fmt,__dynamic_dev_dbg,   \
         |  ^~~~~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:2: note: in expansion of macro 'dynamic_dev_dbg'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |  ^~~~~~~~~~~~~~~
   include/linux/dev_printk.h:115:23: note: in expansion of macro 'dev_fmt'
     115 |  dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                       ^~~~~~~
   drivers/bus/mhi/uci.c:447:2: note: in expansion of macro 'dev_dbg'
     447 |  dev_dbg(dev, "Copied %lu of %lu bytes\n", to_copy, uchan->dl_size);
         |  ^~~~~~~
   drivers/bus/mhi/uci.c:447:32: note: format string is defined here

vim +147 drivers/bus/mhi/uci.c

   120	
   121	static int mhi_queue_inbound(struct uci_dev *udev)
   122	{
   123		struct mhi_device *mhi_dev = udev->mhi_dev;
   124		struct device *dev = &mhi_dev->dev;
   125		int nr_trbs, i, ret = -EIO;
   126		size_t dl_buf_size;
   127		void *buf;
   128		struct uci_buf *ubuf;
   129	
   130		/* dont queue if dl channel is not supported */
   131		if (!udev->mhi_dev->dl_chan)
   132			return 0;
   133	
   134		nr_trbs = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
   135	
   136		for (i = 0; i < nr_trbs; i++) {
   137			buf = kmalloc(udev->mtu, GFP_KERNEL);
   138			if (!buf)
   139				return -ENOMEM;
   140	
   141			dl_buf_size = udev->mtu - sizeof(*ubuf);
   142	
   143			/* save uci_buf info at the end of buf */
   144			ubuf = buf + dl_buf_size;
   145			ubuf->data = buf;
   146	
 > 147			dev_dbg(dev, "Allocated buf %d of %d size %ld\n", i, nr_trbs,
   148				dl_buf_size);
   149	
   150			ret = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, buf, dl_buf_size,
   151					    MHI_EOT);
   152			if (ret) {
   153				kfree(buf);
   154				dev_err(dev, "Failed to queue buffer %d\n", i);
   155				return ret;
   156			}
   157		}
   158	
   159		return ret;
   160	}
   161	
   162	static int mhi_uci_dev_start_chan(struct uci_dev *udev)
   163	{
   164		int ret = 0;
   165		struct uci_chan *uchan;
   166	
   167		mutex_lock(&udev->lock);
   168		if (!udev->uchan || !kref_get_unless_zero(&udev->uchan->ref_count)) {
   169			uchan = kzalloc(sizeof(*uchan), GFP_KERNEL);
   170			if (!uchan) {
   171				ret = -ENOMEM;
   172				goto error_chan_start;
   173			}
   174	
   175			udev->uchan = uchan;
   176			uchan->udev = udev;
   177			init_waitqueue_head(&uchan->ul_wq);
   178			init_waitqueue_head(&uchan->dl_wq);
   179			mutex_init(&uchan->write_lock);
   180			mutex_init(&uchan->read_lock);
   181			spin_lock_init(&uchan->dl_lock);
   182			INIT_LIST_HEAD(&uchan->pending);
   183	
   184			ret = mhi_prepare_for_transfer(udev->mhi_dev);
   185			if (ret) {
   186				dev_err(&udev->mhi_dev->dev, "Error starting transfer channels\n");
   187				goto error_chan_cleanup;
   188			}
   189	
   190			ret = mhi_queue_inbound(udev);
   191			if (ret)
   192				goto error_chan_cleanup;
   193	
   194			kref_init(&uchan->ref_count);
   195		}
   196	
   197		mutex_unlock(&udev->lock);
   198		return 0;
   199	
   200	error_chan_cleanup:
   201		mhi_uci_dev_chan_release(&uchan->ref_count);
   202	error_chan_start:
   203		mutex_unlock(&udev->lock);
   204		return ret;
   205	}
   206	
   207	static void mhi_uci_dev_release(struct kref *ref)
   208	{
   209		struct uci_dev *udev =
   210			container_of(ref, struct uci_dev, ref_count);
   211	
   212		mutex_destroy(&udev->lock);
   213	
   214		kfree(udev);
   215	}
   216	
   217	static int mhi_uci_open(struct inode *inode, struct file *filp)
   218	{
   219		unsigned int minor = iminor(inode);
   220		struct uci_dev *udev = NULL;
   221		int ret;
   222	
   223		mutex_lock(&uci_drv_mutex);
   224		udev = idr_find(&uci_idr, minor);
   225		if (!udev) {
   226			pr_debug("uci dev: minor %d not found\n", minor);
   227			mutex_unlock(&uci_drv_mutex);
   228			return -ENODEV;
   229		}
   230	
   231		kref_get(&udev->ref_count);
   232		mutex_unlock(&uci_drv_mutex);
   233	
   234		ret = mhi_uci_dev_start_chan(udev);
   235		if (ret) {
   236			kref_put(&udev->ref_count, mhi_uci_dev_release);
   237			return ret;
   238		}
   239	
   240		filp->private_data = udev;
   241	
   242		return 0;
   243	}
   244	
   245	static int mhi_uci_release(struct inode *inode, struct file *file)
   246	{
   247		struct uci_dev *udev = file->private_data;
   248	
   249		mutex_lock(&udev->lock);
   250		kref_put(&udev->uchan->ref_count, mhi_uci_dev_chan_release);
   251		mutex_unlock(&udev->lock);
   252	
   253		kref_put(&udev->ref_count, mhi_uci_dev_release);
   254	
   255		return 0;
   256	}
   257	
   258	static __poll_t mhi_uci_poll(struct file *file, poll_table *wait)
   259	{
   260		struct uci_dev *udev = file->private_data;
   261		struct mhi_device *mhi_dev = udev->mhi_dev;
   262		struct device *dev = &mhi_dev->dev;
   263		struct uci_chan *uchan = udev->uchan;
   264		__poll_t mask = 0;
   265	
   266		poll_wait(file, &udev->uchan->ul_wq, wait);
   267		poll_wait(file, &udev->uchan->dl_wq, wait);
   268	
   269		if (!udev->enabled) {
   270			mask = EPOLLERR;
   271			goto done;
   272		}
   273	
   274		spin_lock_bh(&uchan->dl_lock);
   275		if (!list_empty(&uchan->pending) || uchan->cur_buf) {
   276			dev_dbg(dev, "Client can read from node\n");
   277			mask |= EPOLLIN | EPOLLRDNORM;
   278		}
   279		spin_unlock_bh(&uchan->dl_lock);
   280	
   281		if (mhi_get_free_desc_count(mhi_dev, DMA_TO_DEVICE) > 0) {
   282			dev_dbg(dev, "Client can write to node\n");
   283			mask |= EPOLLOUT | EPOLLWRNORM;
   284		}
   285	
   286		dev_dbg(dev, "Client attempted to poll, returning mask 0x%x\n", mask);
   287	
   288	done:
   289		return mask;
   290	}
   291	
   292	static ssize_t mhi_uci_write(struct file *file,
   293				     const char __user *buf,
   294				     size_t count,
   295				     loff_t *offp)
   296	{
   297		struct uci_dev *udev = file->private_data;
   298		struct mhi_device *mhi_dev = udev->mhi_dev;
   299		struct device *dev = &mhi_dev->dev;
   300		struct uci_chan *uchan = udev->uchan;
   301		size_t bytes_xfered = 0;
   302		int ret, nr_avail = 0;
   303	
   304		/* if ul channel is not supported return error */
   305		if (!buf || !count || !mhi_dev->ul_chan)
   306			return -EINVAL;
   307	
 > 308		dev_dbg(dev, "%s: to xfer: %lu bytes\n", __func__, count);
   309	
   310		mutex_lock(&uchan->write_lock);
   311		while (count) {
   312			size_t xfer_size;
   313			void *kbuf;
   314			enum mhi_flags flags;
   315	
   316			/* wait for free descriptors */
   317			ret = wait_event_interruptible(uchan->ul_wq,
   318						       (!udev->enabled) ||
   319					(nr_avail = mhi_get_free_desc_count(mhi_dev,
   320						       DMA_TO_DEVICE)) > 0);
   321	
   322			if (ret == -ERESTARTSYS) {
   323				dev_dbg(dev, "Interrupted by a signal in %s, exiting\n",
   324					__func__);
   325				goto err_mtx_unlock;
   326			}
   327	
   328			if (!udev->enabled) {
   329				ret = -ENODEV;
   330				goto err_mtx_unlock;
   331			}
   332	
   333			xfer_size = min_t(size_t, count, udev->mtu);
   334			kbuf = kmalloc(xfer_size, GFP_KERNEL);
   335			if (!kbuf) {
   336				ret = -ENOMEM;
   337				goto err_mtx_unlock;
   338			}
   339	
   340			ret = copy_from_user(kbuf, buf, xfer_size);
   341			if (ret) {
   342				kfree(kbuf);
   343				ret = -EFAULT;
   344				goto err_mtx_unlock;
   345			}
   346	
   347			/* if ring is full after this force EOT */
   348			if (nr_avail > 1 && (count - xfer_size))
   349				flags = MHI_CHAIN;
   350			else
   351				flags = MHI_EOT;
   352	
   353			ret = mhi_queue_buf(mhi_dev, DMA_TO_DEVICE, kbuf, xfer_size,
   354					    flags);
   355			if (ret) {
   356				kfree(kbuf);
   357				goto err_mtx_unlock;
   358			}
   359	
   360			bytes_xfered += xfer_size;
   361			count -= xfer_size;
   362			buf += xfer_size;
   363		}
   364	
   365		mutex_unlock(&uchan->write_lock);
   366		dev_dbg(dev, "%s: bytes xferred: %lu\n", __func__, bytes_xfered);
   367	
   368		return bytes_xfered;
   369	
   370	err_mtx_unlock:
   371		mutex_unlock(&uchan->write_lock);
   372	
   373		return ret;
   374	}
   375	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx

Attachment: .config.gz
Description: application/gzip


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux