From: Alan Stern <stern@rowland.harvard.edu>
To: Andrzej Pietrasiewicz <andrzej.p@samsung.com>
Cc: Wei.Yang@windriver.com, Michal Nazarewicz <mina86@mina86.com>,
Felipe Balbi <balbi@ti.com>, <gregkh@linuxfoundation.org>,
USB list <linux-usb@vger.kernel.org>,
Kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1] USB:gadget: Fix a warning while loading g_mass_storage
Date: Wed, 4 Jun 2014 11:26:13 -0400 (EDT) [thread overview]
Message-ID: <Pine.LNX.4.44L0.1406041100500.922-100000@iolanthe.rowland.org> (raw)
In-Reply-To: <538F0BBD.2000605@samsung.com>
On Wed, 4 Jun 2014, Andrzej Pietrasiewicz wrote:
> When Sebastian introduced the function registration interface I didn't
> specially like the naming: struct usb_function_instance is something
> different than an instance of struct usb_function.
What is the difference in purpose between usb_function and
usb_function_instance? I can't tell just by reading the header file.
Does one of them get created dynamically when the host sets the
appropriate config?
It's quite noticeable that composite.h does not contain nearly enough
documentation. Only four of the structures defined there have any
kerneldoc, and none of the functions do.
Also, there seems to be some confusion between structures that
represent drivers and those that represent devices (or parts of a
device). For example, struct usb_function contains instance data as
well as driver callbacks.
> The purpose of fsg_alloc_inst() is to create a usb_function_instance
> whose container_of is struct fsg_opts. In fact it is struct fsg_opts
> which is actually allocated; one of its members is struct fsg_common
> which is also allocated - individually for each struct usb_function_instance.
>
> Among traditional gadgets there is no gadget which uses mass storage function
> more than once. On the other hand, when gadgets are created with configfs
> it is forbidden to link a given function more than once into a given
> config,
What is the reason for this restriction?
> that is a struct usb_function_instance can be referenced by more
> than one config, but can be referenced only once in a given config;
> each symbolic link corresponds to an instance of struct usb_function
> (don't confuse with struct usb_function_instance).
It's extremely easy to confuse them, since I don't understand the
differences between them. It even seems like you confused them in this
description: You mentioned "link a given function", "link corresponds
to an instance of struct usb_function", and "struct
usb_function_instance can be referenced by more than one config".
What's the difference between linking a usb_function and referencing a
usb_function_instance? Normally "linking" and "referencing" mean more
or less the same thing.
> So yes, an fsg_common can be shared among instances of struct usb_function,
> but neither with traditional gadgets as they are now nor with configfs
> is it possible to have the same fsg_common referenced more than once
> in a given config.
That's a relief. But it still seems like a bad design. If there can
be only one struct fsg_dev associated with struct fsg_common, why have
separate structures? And if there can be multiple fsg_dev structures
associated with struct fsg_common, why does struct fsg_common contain a
pointer to an fsg_dev (in fact, two of them)?
The issue that started these thoughts was the way fsg_common.new_fsg
gets used, as modified by the patch in the thread's original email.
It's not clear why new_fsg is needed at all.
Alan Stern
next prev parent reply other threads:[~2014-06-04 15:26 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-03 9:37 [PATCH v1] USB:gadget: Fix a warning while loading g_mass_storage Wei.Yang
2014-06-03 14:48 ` Alan Stern
2014-06-04 1:20 ` Yang,Wei
2014-06-04 1:45 ` Peter Chen
2014-06-04 3:16 ` Yang,Wei
2014-06-04 4:41 ` Peter Chen
2014-06-04 13:56 ` Alan Stern
2014-06-04 18:48 ` Paul Zimmerman
2014-06-05 1:30 ` Peter Chen
2014-06-05 14:21 ` Alan Stern
2014-06-04 2:34 ` Yang,Wei
2014-06-04 12:06 ` Andrzej Pietrasiewicz
2014-06-04 15:26 ` Alan Stern [this message]
2014-06-05 10:00 ` Andrzej Pietrasiewicz
2014-06-05 18:10 ` Alan Stern
2014-06-04 4:32 ` [PATCH v2] " Wei.Yang
2014-06-05 18:08 ` Alan Stern
2014-06-09 6:02 ` Yang,Wei
2014-06-09 6:19 ` [PATCH v3] " Wei.Yang
2014-06-13 6:22 ` Yang,Wei
2014-06-13 13:39 ` Alan Stern
2014-06-14 13:10 ` Yang,Wei
2014-06-13 9:44 ` Michal Nazarewicz
2014-06-13 13:43 ` Alan Stern
2014-06-13 14:57 ` Michal Nazarewicz
2014-06-15 2:40 ` [PATCH] " Wei.Yang
2014-06-15 2:42 ` Yang,Wei
2014-06-17 5:59 ` Yang,Wei
2014-06-17 14:18 ` Alan Stern
2014-06-18 1:08 ` Yang,Wei
2014-06-18 11:44 ` Michal Nazarewicz
2014-06-18 14:22 ` Alan Stern
2014-06-19 1:48 ` Yang,Wei
2014-06-17 18:20 ` Michal Nazarewicz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.44L0.1406041100500.922-100000@iolanthe.rowland.org \
--to=stern@rowland.harvard.edu \
--cc=Wei.Yang@windriver.com \
--cc=andrzej.p@samsung.com \
--cc=balbi@ti.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mina86@mina86.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).