Re: [PATCH v2 3/3] usb: gadget: f_fs: make numbers in ep file names the same as ep addresses

[Michal Nazarewicz]

On Fri, Jul 25 2014, Robert Baldyga wrote:
> This patch adds FUNCTIONFS_ADDR_NAMES flag to user flags set in
> descriptors, which makes numbers in endpoint file names the same as
> value of bEndpointAddress in endpoint descriptor. It simplifies endpoint
> handling, because now it can be refered using one unique number.
>
> Numbers are in hexadecimal format to have each name of the same lenght,
> and to simplify debugging. The first digit can be 0 or 8 which means
> OUT or IN endpoint direction, and the second digit is simply hexadecimal
> value of endpoint number (which is between 1 and 15).
>
> It needed to store user flags to the moment of endpoint files creation,
> and for this reason there is new field in struct ffs_data named user_flags.
>
> Signed-off-by: Robert Baldyga <r.baldyga@xxxxxxxxxxx>

I don't like this to be honest.  IMO it adds code for little benefit.

> ---
>  drivers/usb/gadget/f_fs.c           | 11 ++++++++---
>  drivers/usb/gadget/u_fs.h           |  2 ++
>  include/uapi/linux/usb/functionfs.h |  1 +
>  3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
> index a2e18cc..0b8040b 100644
> --- a/drivers/usb/gadget/f_fs.c
> +++ b/drivers/usb/gadget/f_fs.c
> @@ -1550,8 +1550,11 @@ static int ffs_epfiles_create(struct ffs_data *ffs)
>  		epfile->ffs = ffs;
>  		mutex_init(&epfile->mutex);
>  		init_waitqueue_head(&epfile->wait);
> -		sprintf(epfiles->name, "ep%u",
> -			ffs->eps_addrmap[i] & USB_ENDPOINT_NUMBER_MASK);
> +		if (ffs->user_flags & FUNCTIONFS_ADDR_NAMES)
> +			sprintf(epfiles->name, "ep%02x", ffs->eps_addrmap[i]);
> +		else
> +			sprintf(epfiles->name, "ep%u",
> +				ffs->eps_addrmap[i] & USB_ENDPOINT_NUMBER_MASK);
>  		if (!unlikely(ffs_sb_create_file(ffs->sb, epfiles->name, epfile,
>  						 &ffs_epfile_operations,
>  						 &epfile->dentry))) {
> @@ -1912,9 +1915,11 @@ static int __ffs_data_got_descs(struct ffs_data *ffs,
>  		break;
>  	case FUNCTIONFS_DESCRIPTORS_MAGIC_V2:
>  		flags = get_unaligned_le32(data + 8);
> +		ffs->user_flags = flags;
>  		if (flags & ~(FUNCTIONFS_HAS_FS_DESC |
>  			      FUNCTIONFS_HAS_HS_DESC |
> -			      FUNCTIONFS_HAS_SS_DESC)) {
> +			      FUNCTIONFS_HAS_SS_DESC |
> +			      FUNCTIONFS_ADDR_NAMES)) {
>  			ret = -ENOSYS;
>  			goto error;
>  		}
> diff --git a/drivers/usb/gadget/u_fs.h b/drivers/usb/gadget/u_fs.h
> index fe31eba..adc6568 100644
> --- a/drivers/usb/gadget/u_fs.h
> +++ b/drivers/usb/gadget/u_fs.h
> @@ -217,6 +217,8 @@ struct ffs_data {
>  	unsigned			hs_descs_count;
>  	unsigned			ss_descs_count;
>  
> +	unsigned			user_flags;
> +
>  	u8				eps_addrmap[15];
>  
>  	unsigned short			strings_count;
> diff --git a/include/uapi/linux/usb/functionfs.h b/include/uapi/linux/usb/functionfs.h
> index 1ab6f06..a29d910 100644
> --- a/include/uapi/linux/usb/functionfs.h
> +++ b/include/uapi/linux/usb/functionfs.h
> @@ -18,6 +18,7 @@ enum functionfs_flags {
>  	FUNCTIONFS_HAS_FS_DESC = 1,
>  	FUNCTIONFS_HAS_HS_DESC = 2,
>  	FUNCTIONFS_HAS_SS_DESC = 4,
> +	FUNCTIONFS_ADDR_NAMES = 8,
>  };
>  
>  #ifndef __KERNEL__
> -- 
> 1.9.1
>

-- 
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--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/