On Wed, 25 Feb 2004 11:29:52 -0800
David Brownell <[EMAIL PROTECTED]> wrote:
> > Yep, it just uses if()'s instead of #ifdef's, this is the only
> > difference. This doesn't overcome the first limitation you
> > mentioned: the need to modify every gadget driver as new UDC drivers
> > are added.
> Erm, it would overcome it if (as proposed) there were an autoconfig()
> routine that most "bulk-only" function drivers could share.
Only for bulk-only drivers? In this case, g_ether and g_storage don't
qualify as they need an int endpoint. This is half of the drivers I have
in my CVS - and the most useful drivers. Yes, g_serial and g_zero are
bulk-only, so they could use the generic autoconfigure() routine.
Why not extend the autoconfigure() to isochronous and interrupt
endpoints? Is it that complex? My current limited USB knowledge doesn't
allow me to understand what's complex in choosing between iso/int/bulk
endpoints.
> See above. One copy of that particular logic ... NOT (!!) part
> of the function driver (except by init linkage). Reusable library
> code. You understood that later (called it an evil "central
> repository", existence of which is bad); why not here?
Ok, so this was basically a misunderstanding from my side: what you
propose is basically the same, but I'm trying to say that it would be a
lot easier to write UDC-independent autoconfigure code if there would be
some capabilities bitfield rather than having to analyze a free-format
character string. In general, the char *name field can be removed
altogether; in this case you will actually gain some RAM because a
32-bit field is smaller than a 32-bit pointer plus 4-99 characters (if
you're worried about RAM size).
That's it: expose as much information from the UDC driver as it would be
useful to help decisions in a UDC-independent manner. If some UDC has
a new particular limitation (which looks general enough), it would be
useful to add a new caps bit rather than doing if (!strcmp (udc->name,
"blah")) {...}. First, bitfields analyzis results in shorter code.
Second, it is possible one day there will be a new UDC with a similar
limitation.
> Given how different the UDCs are, there MUST be a place to fit
> heuristics in any autoconfiguration scheme.
I agree there can be some totally wild limitations in some UDCs that
are hard to encode into the autoconfigure() code. These cases could
result in ugly if()'s (which could be #ifdef'ed though for those who
don't use that UDC). But I still believe the most autoconfigure part may
be done in a generic way.
> By virtue of the endpoint naming policy, and by restricting that
> logic to "bulk-only" configurations, it's actually possible
> to automate most bulk autoconfig() logic. Something roughly
> like this:
I still don't like the "virtue of endpoint naming policy". See,
mq11xx_udc has endpoints that support either bulk or iso mode. I named
them "ep1in-bulk!iso", you told me to name them just "ep1in". In this
case the information whether the endpoint supports bulk or isochronous
mode is not available at all. So it will have to be a separate case,
while if we would have a bitfield for every endpoint - which modes the
endpoint supports - it could be generic code. Even if it would be
"ep1in-bulk!iso" it is a non-trivial task to parse these strings.
> - Apply heuristics first. (goku: ep2 preferred for bulk IN,
> because of DMA, ep3 for interrupt transfers since it's
> only single-buffered. net2280: use ep-e and ep-f for
> interrupts if maxpacket is 64 bytes or less. And so on.)
Yep, and I would prefer this heuristics to cover 99.99% of all the
cases.
> - Ignore all direction-restricted endpoints where the
> direction is wrong. (If you want IN, ignore "ep3out",
> but "ep2" and "ep4-iso" are OK.)
ok, here is some code:
// let's suppose the first two characters are always "ep"
char *x = ep->name + 2;
while (*x >= '0' && *x <= '9') x++;
if (strncmp (x, "in", 2) || *x == '-')
// in direction is ok
else
// in direction is not ok
// and even with this the above cases doesn't handle marginal cases,
// incorrect endpoint namings and so on
Now with bitfields:
if (ep->caps.in)
// in direction is ok
else
// in direction is not ok
> - Ignore all type-restricted endpoints where the endpoint
> type isn't bulk. (So "ep1-bulk" is OK, so is "ep3",
> but "ep4-iso" is no good.)
again a lot of code to analyze the string, rather than just
if (ep->caps.bulk) ...
> That sounds a lot fancier than necessary. Have you looked at the
> USB class specifications, or most of the current bulk drivers? The
> requirements are: bulk-in, bulk-out. Optionally, a status transfer
> using an interrupt-in (or bulk-in) endpoint. End of requirements.
But if we can do it in an absolutely independent manner, what stops us
from doing it? In fact, what you said above is just the same, the only
thing I don't agree is that we need to parse those strings.
> The endpoint naming convention suffices to report almost all of
> those issues. (Except for the rather arbitrary ones where it's
> better to have simple heuristics.)
Well, it didn't suffice to report the max packet size so you had to add
a separate field for that.
> > I believe all these capabilities can be described by a extra 32-bit
> > word per endpoint, and perhaps an additional 32-bit word for the
> > whole UDC(self-power flag comes to mind). Some endpoint
> > capabilities:
> > - Can be configured as IN
> > - Can be configured as OUT
> > - Can be configured as INT
> > - Can be configured as BULK
> > - Can be configured as ISO
> All of that information is already conveyed by the endpoint naming
> convention.
Not quite. In my case "ep1in" means really "epin-bulk!iso", but is not
explicitly given. With what I propose above this can be given in just 5
bits (rather than a >1 number of 8-bit characters)
> > - Max packet length (as power-of two? 4 bits would be enough) (any
> > UDC with non-power-of-two maximum packet lengths?)
> Already exposed in the "struct usb_ep".
Yes, I know, I just meant this particular limitation could be encoded in
the same bitfield, just like any other particular limit.
> Plus speed-specific issues, and interdependencies between
> endpoint configurations, and quirks that require driver
> level workarounds (like superh_udc endpoint halt processing),
> and probably even some things that pack into nice small
> non-algorithmic bit patterns... :)
So what? Are you going to encode these particular features into the
endpoint naming convention? "ep1in-bulk|iso-hs-haltquirk"? ;-)
Now regarding other things composite gadget drivers will have to share:
since there is just one string table/vendor+product id per UDC, I think
these things should be better moved to the UDC driver (again all UDCs
could share some common code which would handle some of the ep0
requests). The strings themselves and the vendor/product ids could be
loaded into the UDC through some sysfs entry (they can have some default
values and the user-mode code should be able to override them -- this
also will solve the problem with national character strings -- no need
to keep all this mess in the driver). For example, after loading the UDC
driver some embedded system could do something like:
cat /etc/udc-$LANG.conf >/sys/driver/mq11xx_udc/strings
echo 0x413c >/sys/driver/mq11xx_udc/vendor
echo 0x4001 >/sys/driver/mq11xx_udc/product
--
Greetings,
Andrew
-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
