On Thu, 26 Feb 2004 16:27:01 -0800
David Brownell <[EMAIL PROTECTED]> wrote:

> > Ok, then we may leave ISO out of parenthesis. In general, what we
> > are discussing here are just helper routines, so if some gadget
> > doesn't like them it could examine the available UDC driver itself.
> Yes:  utilities.  Which should make it easier to handle
> "simple" cases (most of today's gadget drivers), but will
> never replace direct access to the "struct usb_gadget"
> as needed by the more complex cases.
Well, I never stated the "utilites" will be mandatory for gadget
drivers. I hate non-replaceable plug-n-pray, due to my experience with
Some OS. I always kept in mind the fact that some drivers may have some
very specific requirements (or very simple requirements - for example
they will know exactly the UDC and the endpoints).

> >> By the way, how net2280 driver fits into this naming scheme? It has
> > "ep-a", "ep-b" and so on until "ep-f".
> In that case the names are from the chip vendor.  There are no
> hardware restrictions on endpoint role, beyond FIFO size and
> DMA support.  They fit in as "no restrictions" cases; they'll
> handle pretty much everything you throw at them.
Yes, but the are out of the naming scheme you proposed: we can't rely
anymore the characters 3 (and possibly 4) are endpoint number. This
makes string analysis a little more complex.
Can't we rename them just to "ep1", "ep2" and so on? I don't think the
chip vendor will be strongly oppsed to that, and it will simplify the
things :) (besides, when I was looking at endpoint names through UDC
drivers (before you explained me the naming scheme) I had the impression
they are free-format strings because of ep-a and such).

> Then we're not really disagreeing.  I'm saying the current API
> suffices for now; you're saying that in the future autoconfig
> code may benefit from some new endpoint bitfields conveying the
> most common hardware restrictions.
The answer is simple: since we're going anyway to change things, why not
change them in such a way that will be useful in future? Your approach
is more conservative, it will retain 100% compatibilty with existing
gadget drivers (is there such a large number of them? I've seen only
three).

If you want to retain backward compatibility, you can just leave the
endpoint names and add the capabilities bitfield. Then an advisory
comment may be added telling to prefer bitfield analysis rather than
using the endpoint name. This way, new drivers will use the more
precise endpoint choosing scheme while old drivers will still work.

Or you're against the bitfield to retain full binary backward
compatibility? What for, in a open-source OS? :)

The reason for a capabilities field is that it allows fine-grained
control over the features of the endpoint that the gadget drivers wants
to use, and it allows the autoconfig routine to find a better match.
For example, there's nothing about DMA in naming convention; some driver
may need it badly (e.g. g_storage), some may not need it that bad
(although it won't be bad to have it anyway) (e.g. g_joystick).

> If I submitted an autoconfig() patch along the lines of what I
> sketched, and updated all the gadget drivers, then you'd want
> to change how it looks at the controllers (to use bitfields).
> Changing that would mean tweaking the autoconfig() logic to use
> such bitfields (in controllers that provide them), then changing
> all the in-tree device controller drivers to provide them.  Yes?
I don't quite understand the statement. If we would have a bitfield, and
there would appear a new controller that would have some new feature
which has to be taken into account during autoconfigure, the only things
that will have to be changed are the new UDC driver (to provide the
feature bit), and the autoconf() utility (to take that bit into
account). The default policy for coping with gadgets unaware of that bit
should be determined by the nature of the feature. If this is a
restrictive feature, and unaware gadget drivers could fail if they get
such a restricted endpoint, the policy should be "don't use the endpoint
if gadget driver haven't explicitly allowed it". Further, some bitfields
may have the mandatory policy, e.g. if driver wants some feature and the
endpoint doesn't have it, don't use this endpoint. Finally, some
bitfields may have the "optional" policy, which means "if there's a
endpoint with this feature, use it, otherwise use any other endpoint
fitting all other requirements".

This may sound complex but in fact it's a matter of just four integer
constants that will have to be applied against endpoint features and
requested features. Something like this:

#define UDC_EP_POLICY_OPTIONAL     (UDC_EP_DMA | UDC_EP_SOMELIMIT)
#define UDC_EP_POLICY_MANDATORY    (UDC_EP_IN | UDC_EP_OUT | \
  UDC_EP_INT | UDC_EP_ISO | UDC_EP_BULK)
#define UDC_EP_POLICY_NEEDEXPLICIT (UDC_EP_SOMELIMIT)

// best-endpoint-finding loop; req_caps is the requested capabilities
// for a particular endpoint; ep is the endpoint structure; best_ep is
// the best endpoint we found so far (initially NULL); also:
// int best_weight = -1, weight;

if ((ep->caps & UDC_EP_POLICY_MANDATORY) != 
    (req_caps & UDC_EP_POLICY_MANDATORY))
        // this endpoint doesn't fit
        continue;

if (ep->caps & ~req_caps & UDC_EP_POLICY_NEEDEXPLICIT)
        // Endpoint has some capability which has to be explicitly
        // requested but have not been specified.
        continue;

// Check if the max transfer limitation is not exceeded
if (UDC_EP_MAXPACKET(req_caps) > UDC_EP_MAXPACKET(ep->caps))
        // This endpoint has a too low limit on max packet size
        continue;

// At this point we know the endpoint fits the requirements, but we
// still have to see if it has more optional features gadget driver
// requested than the endpoint we found before that
weight = compute_fitness (ep->caps & req_caps & UDC_EP_POLICY_OPTIONAL);
if (weight > best_weight) {
        best_weight = weight;
        best_ep = ep;
}

// And the compute_fitness() routine would look like this:
int compute_fitness (int bitmask)
{
        int weight = 0;
        while (bitmask) {
                int old_bitmask = bitmask;
                bitmask &= (bitmask - 1);
                weight += old_bitmask ^ bitmask;
        }
        return weight;
}
// The idea here is to order the feature bits such that most
// important features have a higher weight (e.g. a higher bit number).
// If this is problemativ, the same can be done with a weight table,
// but at a slightly higher price.

I evaluate all of the above code to compile in less than 100 bytes on
any platform. Even with the whole infrastructure it will be very small
(and very flexible). The code to analyze ep->name will be a lot larger
and a lot less flexible (no DMA, no easy way for gadget to specify its
attitude towards different limitations on endpoints).

The joy of bitfields is that you can analyze all fields at once with
simple boolean operations. You must recognize the same code using char*
will be two to ten times larger :-)

> Solved by {who,what}ever creates the header with entries (newer
> kernels changed these symbols...) like the mach_is_xxx() ones:
>     #ifdef CONFIG_USB_GADGET_GOKU
>     #  define gadget_is_goku(g) (strcmp("goku_udc", (g)->name) == 0)
>     #else
>     #  define gadget_is_goku(g) 0
>     #endif
> Or it could look a lot more like the mach_is_xxx stuff and include
> a bunch of #defined symbols for each controller.  That might be nice
> for quirk handling that affects "normal operations", for drivers too
> lazy to check once during init time and set a local flag.
Agree. This cold be added to some global header file that every gadget
will include, and every gadget will have a way to check for quirks
specific to just one UDC.

> Sure, I understand that.  I just want to see the utilities in place
> and working before anything starts to change the controller drivers
> to add a mechanism that we both agreed isn't currently necessary,
> or even beneficial until drivers start autoconfiguring endpoints.
Well, when the autoconf() will be written using char* names, it will be
too late to change anything, that's why I want this clarified now.

> I think it'd make more sense for all of that to be handled by
> module parameters though.
Yes, I have thought of module parameters too but I don't know a easy way
to pass UCS-2 strings as a module parameter. We can limit to ISO8859-1
strings though.

> That'd be cheaper than sysfs support, and would work easily on 2.4
> kernels too.  No need to grow any specialized infrastructure for 
> module params, though it'd be nice to have all the drivers use the
> same module_param names.
Perhaps on 2.4.x we can use module parameters, and on 2.6.x include
additional sysfs support (enabled by a bool in menuconfig) for those
Advanced Guys{tm} needing UCS-2.

--
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

Reply via email to