Re: [PATCH 14/16] usb/gadget: g_ffs: convert to new interface of f_fs

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

 



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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux