Re: [PATCH v3 13/16] usb/gadget: FunctionFS: convert to new function interface with backward compatibility

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

 



On Fri, Nov 22 2013, Andrzej Pietrasiewicz wrote:
> This is required in order to integrate configfs support.
> f_fs needs to be a separately compiled module and so it needs to use the new
> interface.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>

Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>


> @@ -496,6 +297,7 @@ static ssize_t ffs_ep0_write(struct file *file, const char __user *buf,
>  			mutex_unlock(&ffs->mutex);
>  
>  			ret = ffs_ready(ffs);
> +
>  			if (unlikely(ret < 0)) {
>  				ffs->state = FFS_CLOSING;
>  				return ret;

There are couple of hunks like this one which just make the patch longer
then it should be.  Just an observation.

> @@ -2226,8 +2031,59 @@ static int __ffs_func_bind_do_nums(enum ffs_entity_type type, u8 *valuep,
>  	return 0;
>  }
>  
> -static int ffs_func_bind(struct usb_configuration *c,
> -			 struct usb_function *f)
> +#ifndef USB_FFS_INCLUDED
> +static inline struct f_fs_opts *ffs_do_functionfs_bind(struct usb_function *f,
> +						struct usb_configuration *c)
> +{
> +	struct ffs_function *func = ffs_func_from_usb(f);
> +	struct f_fs_opts *ffs_opts =
> +		container_of(f->fi, struct f_fs_opts, func_inst);
> +	int ret;
> +
> +	ENTER();
> +
> +	/*
> +	 * Legacy gadget triggers binding in functionfs_ready_callback,
> +	 * which already uses locking; taking the same lock here would
> +	 * cause a deadlock.
> +	 *
> +	 * Configfs-enabled gadgets however do need ffs_dev_lock.
> +	 */
> +	if (!ffs_opts->no_configfs)
> +		ffs_dev_lock();

Could this condition be hidden inside ffs_dev_lock?  On the other hand,
since this is a transitional state, maybe it's not worth it to make it
perfect.

> +	ret = ffs_opts->dev->desc_ready ? 0 : -ENODEV;
> +	func->ffs = ffs_opts->dev->ffs_data;
> +	if (!ffs_opts->no_configfs)
> +		ffs_dev_unlock();
> +	if (ret)
> +		return ERR_PTR(ret);

> @@ -2482,11 +2355,17 @@ void ffs_dev_lock(void)
>  {
>  	mutex_lock(&ffs_lock);
>  }
> +#ifndef USB_FFS_INCLUDED
> +EXPORT_SYMBOL(ffs_dev_lock);
> +#endif
>  
>  void ffs_dev_unlock(void)
>  {
>  	mutex_unlock(&ffs_lock);
>  }
> +#ifndef USB_FFS_INCLUDED
> +EXPORT_SYMBOL(ffs_dev_unlock);
> +#endif

Right…  Like I wrote before, I think ffs_lock should be exported and
those functions should be static inline in header file.


-- 
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