lkml.org 
[lkml]   [2024]   [Jan]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/3] usb: gadget: function: 9pfs
From
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;
> +}

..



\
 
 \ /
  Last update: 2024-01-16 23:25    [from the cache]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and my Meterkast|Read the blog