On Wed, Oct 23 2013, Andrzej Pietrasiewicz wrote:
> Prepare for configfs integration. Use the new interface so that f_fs can be
> made a module.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> ---
> drivers/usb/gadget/Kconfig | 1 +
> drivers/usb/gadget/g_ffs.c | 138 ++++++++++++++++++++++++++++----------------
> 2 files changed, 89 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index 851d864..2234001 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -861,6 +861,7 @@ config USB_GADGETFS
> config USB_FUNCTIONFS
> tristate "Function Filesystem"
> select USB_LIBCOMPOSITE
> + select USB_F_FS
> select USB_FUNCTIONFS_GENERIC if !(USB_FUNCTIONFS_ETH || USB_FUNCTIONFS_RNDIS)
> help
> The Function Filesystem (FunctionFS) lets one create USB
> diff --git a/drivers/usb/gadget/g_ffs.c b/drivers/usb/gadget/g_ffs.c
> index 0298018..5e7b43a 100644
> --- a/drivers/usb/gadget/g_ffs.c
> +++ b/drivers/usb/gadget/g_ffs.c
> @@ -13,13 +13,7 @@
> #define pr_fmt(fmt) "g_ffs: " fmt
>
> #include <linux/module.h>
> -/*
> - * kbuild is not very cooperative with respect to linking separately
> - * compiled library objects into one module. So for now we won't use
> - * separate compilation ... ensuring init/exit sections work to shrink
> - * the runtime footprint, and giving us at least some parts of what
> - * a "gcc --combine ... part1.c part2.c part3.c ... " build would.
> - */
> +
> #if defined CONFIG_USB_FUNCTIONFS_ETH || defined CONFIG_USB_FUNCTIONFS_RNDIS
>
> #include <linux/netdevice.h>
> @@ -55,8 +49,7 @@ static struct usb_function *f_rndis;
> # endif
> #endif
>
> -#define USB_FFS_INCLUDED
> -#include "f_fs.c"
> +#include "u_fs.h"
>
> #define DRIVER_NAME "g_ffs"
> #define DRIVER_DESC "USB Function Filesystem"
> @@ -140,6 +133,7 @@ static struct usb_gadget_strings *gfs_dev_strings[] = {
> struct gfs_configuration {
> struct usb_configuration c;
> int (*eth)(struct usb_configuration *c);
> + int num;
> } gfs_configurations[] = {
> #ifdef CONFIG_USB_FUNCTIONFS_RNDIS
> {
> @@ -162,6 +156,11 @@ struct gfs_configuration {
> static int gfs_bind(struct usb_composite_dev *cdev);
> static int gfs_unbind(struct usb_composite_dev *cdev);
> static int gfs_do_config(struct usb_configuration *c);
> +static int functionfs_ready_callback(struct ffs_data *ffs);
> +static void functionfs_closed_callback(struct ffs_data *ffs);
> +static void *functionfs_acquire_dev_callback(const char *dev_name);
> +static void functionfs_release_dev_callback(struct ffs_data *ffs_data);
> +
>
> static __refdata struct usb_composite_driver gfs_driver = {
> .name = DRIVER_NAME,
> @@ -176,7 +175,22 @@ static DEFINE_MUTEX(gfs_lock);
> static unsigned int missing_funcs;
> static bool gfs_registered;
> static bool gfs_single_func;
> -static struct ffs_dev **ffs_tab;
> +static struct usb_function_instance **fi_ffs;
> +static struct usb_function **f_ffs[] = {
> +#ifdef CONFIG_USB_FUNCTIONFS_RNDIS
> + NULL,
> +#endif
> +
> +#ifdef CONFIG_USB_FUNCTIONFS_ETH
> + NULL,
> +#endif
> +
> +#ifdef CONFIG_USB_FUNCTIONFS_GENERIC
> + NULL,
> +#endif
> +};
> +
> +#define N_CONF ARRAY_SIZE(f_ffs)
>
> static int __init gfs_init(void)
> {
> @@ -185,32 +199,53 @@ static int __init gfs_init(void)
>
> ENTER();
>
> + ffs_set_auto_init(false);
> +
> if (func_num < 2) {
> gfs_single_func = true;
> func_num = 1;
> }
>
> - ffs_tab = kcalloc(func_num, sizeof(ffs_tab), GFP_KERNEL);
> - if (!ffs_tab)
> - return -ENOMEM;
> + for (i = 0; i < N_CONF; i++) {
> + f_ffs[i] = kcalloc(func_num, sizeof(f_ffs[i]), GFP_KERNEL);
> + if (!f_ffs[i]) {
> + ret = -ENOMEM;
> + goto no_func;
> + }
> + }
Perhaps:
f_ffs[0] = kcalloc(func_num * N_CONF, sizeof(*f_ffs), GFP_KERNEL);
if (!f_ffs[0]) {
ret = -ENOMEM;
goto no_func;
}
for (i = 1; i < N_CONF; ++i)
f_ffs[i] = f_ffs[0] + i * func_num;
> +
> + fi_ffs = kcalloc(func_num, sizeof(fi_ffs), GFP_KERNEL);
Missing dereference in sizeof:
fi_ffs = kcalloc(func_num, sizeof(*fi_ffs), GFP_KERNEL);
> + if (!fi_ffs) {
> + ret = -ENOMEM;
> + i = N_CONF;
i is already N_CONF.
> + goto no_func;
> + }
>
> for (i = 0; i < func_num; i++) {
> - ffs_tab[i] = ffs_alloc_dev();
> - if (IS_ERR(ffs_tab[i])) {
> - ret = PTR_ERR(ffs_tab[i]);
> + fi_ffs[i] = usb_get_function_instance("ffs");
> + if (IS_ERR(fi_ffs[i])) {
> + ret = PTR_ERR(fi_ffs[i]);
> goto no_dev;
> }
> if (!gfs_single_func)
> - ffs_tab[i]->name = func_names[i];
> + to_f_fs_opts(fi_ffs[i])->dev->name = func_names[i];
> }
>
> missing_funcs = func_num;
>
> - return functionfs_init(NULL);
> + ffs_set_acquire_dev_cb(functionfs_acquire_dev_callback);
> + ffs_set_release_dev_cb(functionfs_release_dev_callback);
> + ffs_set_ready_cb(functionfs_ready_callback);
> + ffs_set_closed_cb(functionfs_closed_callback);
> + return functionfs_init(THIS_MODULE);
> no_dev:
> while (--i >= 0)
> - ffs_free_dev(ffs_tab[i]);
> - kfree(ffs_tab);
> + usb_put_function_instance(fi_ffs[i]);
> + kfree(fi_ffs);
> + i = N_CONF;
> +no_func:
> + while (--i >= 0)
> + kfree(f_ffs[i]);
> return ret;
> }
> module_init(gfs_init);
> @@ -499,18 +532,9 @@ static int gfs_unbind(struct usb_composite_dev *cdev)
> usb_put_function_instance(fi_geth);
> }
> #endif
> -
> - /*
> - * We may have been called in an error recovery from
> - * composite_bind() after gfs_unbind() failure so we need to
> - * check if instance's ffs_data is not NULL since gfs_bind() handles
> - * all error recovery itself. I'd rather we werent called
> - * from composite on orror recovery, but what you're gonna
> - * do...?
> - */
> - for (i = func_num; i--; )
> - if (ffs_tab[i]->ffs_data)
> - functionfs_unbind(ffs_tab[i]->ffs_data);
> + for (i = 0; i < N_CONF; i++)
> + for (j = 0; j < func_num; j++)
> + usb_put_function(f_ffs[i][j]);
By the way, if f_ffs was allocated as a single chunk, this could be
changed into a single loop.
>
> return 0;
> }
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<mpn@xxxxxxxxxx>--<xmpp:mina86@xxxxxxxxxx>--ooO--(_)--Ooo--
Attachment:
signature.asc
Description: PGP signature
