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

From: kernel test robot
Date: Sat Oct 17 2020 - 03:32:58 EST


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