Messages in this thread |  | | Date | Tue, 16 Jan 2024 22:14:10 +0100 | | Subject | Re: [PATCH 1/3] usb: gadget: function: 9pfs | | From | Christophe JAILLET <> |
| |
Le 16/01/2024 à 02:49, Michael Grzeschik a écrit : > Add the new gadget function for 9pfs transport. This function is > defining an simple 9pfs transport interface that consists of one in and > one out endpoint. The endpoints transmit and receive the 9pfs protocol > payload when mounting a 9p filesystem over usb. > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > --- > Documentation/filesystems/9p.rst | 12 + > drivers/usb/gadget/Kconfig | 11 + > drivers/usb/gadget/function/Makefile | 2 + > drivers/usb/gadget/function/f_9pfs.c | 849 +++++++++++++++++++++++++++++++++++ > 4 files changed, 874 insertions(+)
..
> +static int alloc_requests(struct usb_composite_dev *cdev, > + struct f_usb9pfs *usb9pfs) > +{ > + int result = 0; > + > + usb9pfs->in_req = usb_ep_alloc_request(usb9pfs->in_ep, GFP_ATOMIC); > + if (!usb9pfs->in_req) > + goto fail; > + > + usb9pfs->out_req = usb9pfs_alloc_ep_req(usb9pfs->out_ep, usb9pfs->buflen); > + if (!usb9pfs->out_req) > + goto fail_in; > + > + usb9pfs->in_req->complete = usb9pfs_tx_complete; > + usb9pfs->out_req->complete = usb9pfs_rx_complete; > + > + /* length will be set in complete routine */ > + usb9pfs->in_req->context = usb9pfs->out_req->context = usb9pfs; > + > + return 0; > + > +fail_in: > + usb_ep_free_request(usb9pfs->in_ep, usb9pfs->in_req); > +fail: > + return result;
'result' is never overwritten, so we always return 0. Is it on purpose?
> +}
..
> +static struct usb_function *usb9pfs_alloc(struct usb_function_instance *fi) > +{ > + struct f_usb9pfs_opts *usb9pfs_opts; > + struct f_usb9pfs *usb9pfs; > + > + usb9pfs = kzalloc(sizeof(*usb9pfs), GFP_KERNEL); > + if (!usb9pfs) > + return ERR_PTR(-ENOMEM); > + > + usb9pfs_opts = container_of(fi, struct f_usb9pfs_opts, func_inst); > + > + mutex_lock(&usb9pfs_opts->lock); > + usb9pfs_opts->refcnt++; > + mutex_unlock(&usb9pfs_opts->lock); > + > + usb9pfs->buflen = usb9pfs_opts->buflen; > + > + usb9pfs->function.name = "usb9pfs"; > + usb9pfs->function.bind = usb9pfs_func_bind; > + usb9pfs->function.set_alt = usb9pfs_set_alt; > + usb9pfs->function.disable = usb9pfs_disable; > + usb9pfs->function.strings = usb9pfs_strings; > + > + usb9pfs->function.free_func = usb9pfs_free_func; > + > + mutex_lock(&usb9pfs_ida_lock); > + > + usb9pfs->index = ida_simple_get(&usb9pfs_ida, 0, 100, GFP_KERNEL);
This API will soon be removed. Please use ida_alloc_max() instead.
Also, there is no corresponding ida_free()? (or isa_simple_remove())
> + if (usb9pfs->index < 0) { > + struct usb_function *ret = ERR_PTR(usb9pfs->index); > + > + kfree(usb9pfs); > + mutex_unlock(&usb9pfs_ida_lock); > + return ret; > + } > + > + mutex_unlock(&usb9pfs_ida_lock); > + > + usb9pfs->tag = kasprintf(GFP_KERNEL, "%s%d", usb9pfs->function.name, > + usb9pfs->index);
Same here, it seems to be freed nowhere? I think that both should be added in usb9pfs_free_func().
CJ
> + > + INIT_LIST_HEAD(&usb9pfs->function_list); > + > + mutex_lock(&usb9pfs_lock); > + list_add_tail(&usb9pfs->function_list, &usbg_function_list); > + mutex_unlock(&usb9pfs_lock); > + > + return &usb9pfs->function; > +}
..
|  |