[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <xa1tzjfxtti5.fsf@mina86.com>
Date: Fri, 25 Jul 2014 16:14:58 +0200
From: Michal Nazarewicz <mina86@...a86.com>
To: Robert Baldyga <r.baldyga@...sung.com>, balbi@...com
Cc: gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org, m.szyprowski@...sung.com,
andrzej.p@...sung.com, Robert Baldyga <r.baldyga@...sung.com>
Subject: Re: [PATCH v2 3/3] usb: gadget: f_fs: make numbers in ep file names the same as ep addresses
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@...sung.com>
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@...gle.com>--<xmpp:mina86@...ber.org>--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists