On Thu, Nov 22 2012, Andrzej Pietrasiewicz wrote:
> +static void fsg_free(struct usb_function *f)
> +{
> + struct fsg_dev *fsg = fsg_from_func(f);
> +
> kfree(fsg);
> }
kfree(fsg_from_func(f));
if you ask me, but feel free to ignore me.
>
> @@ -2970,22 +2981,65 @@ autoconf_fail:
> }
>
> /****************************** ADD FUNCTION ******************************/
> +static void msg_cleanup(void)
> +{
> + clear_bit(0, &msg_registered);
> +}
> +
> +static int msg_thread_exits(struct fsg_common *common)
> +{
> + msg_cleanup();
> + return 0;
> +}
> +
> +static int fsg_add_function(struct usb_configuration *c, struct usb_function
> *f,
> + struct config_item *item, void *data)
> +{
> + static const struct fsg_operations ops = {
> + .thread_exits = msg_thread_exits,
> + };
> +
> + struct fsg_common *common = to_fsg_common(item);
> + struct fsg_dev *fsg;
> + struct usb_composite_dev *cdev = data;
> + int status;
> +
By the way, you keep leaving trailing tabs on otherwise empty lines.
> + common->ops = &ops;
> + fsg_common_init_cdev(common, cdev);
> +
> + fsg = container_of(f, struct fsg_dev, function);
> + fsg->common = common;
> +
> + status = usb_add_function(c, f);
> + if (status)
> + goto err_ms_get;
> + else
> + fsg_common_get(common);
This could get away w/o goto if the error handling was put inside if
body, ie.:
if (status) {
usb_put_function(f);
fsg_common_put(common);
return status;
}
fsg_common_get(common);
> + set_bit(0, &msg_registered);
> + fsg_common_put(common);
> +
> + return status;
> +
> +err_ms_get:
> + usb_put_function(f);
> + fsg_common_put(common);
> + return status;
> +}
> +
> +/****************************** ALLOCATE FUNCTION *************************/
>
> static struct usb_gadget_strings *fsg_strings_array[] = {
> &fsg_stringtab,
> NULL,
> };
>
> -static int fsg_bind_config(struct usb_composite_dev *cdev,
> - struct usb_configuration *c,
> - struct fsg_common *common)
> +static struct usb_function *fsg_alloc(void)
> {
> struct fsg_dev *fsg;
> - int rc;
>
> fsg = kzalloc(sizeof *fsg, GFP_KERNEL);
> if (unlikely(!fsg))
> - return -ENOMEM;
> + return NULL;
>
> fsg->function.name = FSG_DRIVER_DESC;
> fsg->function.strings = fsg_strings_array;
> @@ -2994,8 +3048,10 @@ static int fsg_bind_config(struct usb_composite_dev
> *cdev,
> fsg->function.setup = fsg_setup;
> fsg->function.set_alt = fsg_set_alt;
> fsg->function.disable = fsg_disable;
> + fsg->function.free_func = fsg_free;
Since I've started nitpicking white space errors, there's tab character
after “free_func”.
> + fsg->function.make_group = alloc_fsg_common;
> + fsg->function.add_function = fsg_add_function;
>
> - fsg->common = common;
> /*
> * Our caller holds a reference to common structure so we
> * don't have to be worry about it being freed until we return
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: m...@google.com>--------------ooO--(_)--Ooo--
pgpSbbKtQEMNf.pgp
Description: PGP signature
