Message-ID: <20120702090907.GC13247@dhcp-172-17-9-228.mtv.corp.google.com>
Date:	Mon, 2 Jul 2012 02:09:08 -0700
From:	Joel Becker <jlbec@...lplan.org>
To:	Andrzej Pietrasiewicz <andrzej.p@...sung.com>
Cc:	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Felipe Balbi <balbi@...com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [RFC 0/2] USB gadget - configfs

On Thu, Jun 21, 2012 at 12:55:27PM +0200, Andrzej Pietrasiewicz wrote:
> Dear All,
> 
> This patch series is a follow-up to this thread:
> 
> http://marc.info/?l=linux-usb&m=132430533824884&w=2

Dear Andrzej,
	I read this thread just now, and I wanted to comment.  I think
configfs fits your needs nicely here.  I'd like to affirm your
supposition that hiding the details in a library is the right way to go.
 configfs aims to provide nice, predictable, transparent user<->kernel
API for creating and configuring objects, but it doesn't have to be what
the end user or administrator sees in everyday use.  Most of the other
configfs clients (ocfs2, dlm, target) wrap configfs usage in their
tools.

> As a prerequisite it adds an operation to configfs. The operation allows
> checking if it is ok to remove a pseudo directory corresponding to a
> configfs item/group.

	I NAK'd that patch because you should be using
configfs_depend_item().  If you have trouble with that, let's talk.

<snip>
 
> I would like to ask some questions. All answers in general, and answers
> from linux-usb and Felipe and Greg KH in particular, are welcome.
> 
> 1. Generally, is this the right way to go?

LGTM.

> 2. Using configfs like this calls for an interface between the generic
>    configfs-related code and function-specific code. I suggested the
>    struct ufg_fn. What do you think?
> 3. Should some parameters still be available through sysfs?

	If they fit, yes.  As a rule of thumb, if userspace directed the
object creation, it should be in configfs.  But if kernelspace created
the object, sysfs is the better location.  There is wiggle room here.
Do not be afraid to use both.  configfs was created because sysfs didn't
fit userspace-directed creation.  configfs is not intended to replace
all of the things sysfs does better.

> 4. Do we need module parameters for USB descriptors like iManufacturer
>    and similar?
> 5. I assumed that the configfs entities are contained in the structures
>    used for configuring the USB functions, e.g. a struct config_group in
>    struct fsg_common, or a struct config_item in a struct fsg_lun. This
>    has implications that the lifetime of the structures is controlled by
>    userspace through configfs, which, in turn, has influence on how
>    the USB functions are implemented. Even though it seems natural,
>    there are some issues. For example an extension to configfs was required
>    in order to disable deleting the luns while the gadget is connected.
>    Is this the right approach? If not, then are there any alternatives?

	Yes, embedding in the structures is correct.  These are
userspace-created objects, and thus userspace has control over their
lifetimes.  That's intentional.  See above about configfs_depend_item()
for blocking unsafe removal.

Joel

-- 

"Hey mister if you're gonna walk on water,
 Could you drop a line my way?"

			http://www.jlbec.org/
			jlbec@...lplan.org
--
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