On Sun, May 13, 2007 at 01:25:36PM +0100, Bryan O'Donoghue wrote:

Hmm, you might want to change

unsigned int len = 0; while(len > 0) { usbtty_poll();

space = maxlen - usbtty_output.size; if(space){ /* Do stuff */ } }

to

/* Not tested */ unsigned int len = 0; while(len > 0) { usbtty_poll();

/* Do stuff */ }

in __usbtty_puts()

Ok, I'll test it. This should remove the problem that the system doesn't boot if no connected?

puts() I guess _should_ be a 'best effort'. Better, still, maybe we could add an environment variable which would switch puts() between flow-control and 'best effort', so that people have the choice.

/* Not tested */ unsigned int len = 0; while(len > 0) { usbtty_poll(); space = maxlen;

if(environment_variable_use_tty_flow_control) space = maxlen - usbtty_output.size;

if(space){ /* Do stuff */ } }

If I set environment_variable_use_tty_flow_control to "no", how I can restore it to "yes"?

Thanks,

Rodolfo

On Fri, 18 May 2007 19:15:21 +0200 Rodolfo Giometti giometti@enneenne.com wrote:

...

/* Not tested */ unsigned int len = 0; while(len > 0) { usbtty_poll();

/* Do stuff */ }

in __usbtty_puts()

No, this doesn't resolve the problem.

That's really quite odd. The following should be similar to what you did and it *does* seem to work just fine, for me.

/* * Output a string to the usb client port - implementing flow control */ static void __usbtty_puts (const char *str, int len) { int maxlen = usbtty_output.totalsize; int space, n, retries = 0;

/* break str into chunks < buffer size, if needed */ while (len > 0 && retries < 10000L) { usbtty_poll ();

space = maxlen - usbtty_output.size; /* Empty buffer here, if needed, to ensure space... */ if (space) { write_buffer (&usbtty_output); n = MIN (space, MIN (len, maxlen)); buf_push (&usbtty_output, str, n);

str += n; len -= n; retries = 0; }else{ retries++; } } }

Das U-Boot environment: stdin usbtty stdout usbtty usbtty cdc_acm

Best, Bryan

On Sun, 20 May 2007 18:07:02 +0200 Rodolfo Giometti giometti@enneenne.com wrote:

Hold on wait... I think I've thought of a better way still to send that data..

How about

__puts() { while(usb_configured() == TRUE && data_to_send){ /* do the thing */ } }

I think something like that would be better from the __puts() perspective, then a retry count.... I'll get back to you on that one though, because I may find some way to break that else I'll code up a patch around this idea, and submit at a later date.

Basically the __puts() function would function as a guaranteed delivery if and only-if the device was in the configured state, else it should happily allow the system to boot, for example in the case where the USB cable hasn't been connected.

This would mean that each usbdcore_* would export usb_get_state() or similar and disjoin on state == CONFIGURED, which would allow us not to do naff things like "retries", especially on something like USB bulk endpoints, which are supposed to guarantee data tx.

It's into usbtty_poll() which calls write_buffer() when the USB device get connected (usbtty_configured() is true).

Function write_buffer() calls udc_endpoint_write() who calls driver low level function. This low level function, PXA270 specific, waits all data has been transmitted before returning to the caller, this because I need to know when a packet has been transmetted before sending a new one or I get some data lost during transmission.

If you loop indefinitely at the low-level BULK tx level... how would you deal with ep0 control requests that *must* be responded to immediately ?

You are right that you can't abandon BULK IN data... but, you aren't right to be doing the infinite loop in your function below endpoint_write() since this means you can't deal with a control request if one should happen.

You'll see I have a tx_retry in udc_ep_tx and you *might* be thinking that that means I *give up* data transfer but, that's not so..

write_buffer() won't increment the data unless endpoint_write() says it successfully transmitted the data. Thus if the endpoint_write call stack returns anything other then 0, the implication is that there is TX data *still* to be sent, and that's precisely what a function like __puts() should do with a statement like

while(usb_configured() == TRUE && data_to_send) as the control.

The rationale behind this behavior is that it allows a high level function such as __puts() to make a decision as to provide either guaranteed or best-effort data transport to the host system.

Greetings, Bryan

On Mon, 21 May 2007 00:23:42 +0200 Rodolfo Giometti giometti@enneenne.com wrote:

The problem is that the defice __is__ into configured state (USB cable is connected and /dev/ttyUSB0 is running) but nobody has opened the serial line (no minicom/kermit running).

So, I suppose, the above modification is useless...

No, no, unless I'm *really* forgetting my USB enumeration behavior, a device shouldn't go into the configured state unless and until the host does a SET_CONFIGURATION, so the modification would be right since without the USB cable connected the device should be in the DEFAULT state, not the CONFIGURED state.

usb_20.pdf page - 240 Figure 9.1 see ?

Mmm... I see... I supposed to get no control request once device is connected...

Currently I use a timeout of 2ms but I suppose I should decrease it, shouldn't I? :-o

Possibly... I don't think I'd take issue with the specific timeout but, you *can* get a control request at *any* time... including 50% of the way through a DATA IN transfer to the host... there's nothing in the USB specification which precludes control requests on EP0 at *any* time...

You'd probably "get away" with it if you limited your hardware to functioning only with g_serial but, where would be the fun in doing just g_serial ?

Ok, what do you suggest to do? I'm a bit confused... :)

Possibly the simple solution is

#1: Fix it so SET_CONFIGURATION is the only thing to move the device into the configured state.

#2: Either you or I should write a patch then which looks like this.

/* Not tested */ static void __usbtty_puts (const char *str, int len) { int maxlen = usbtty_output.totalsize; int space, n;

/* break str into chunks < buffer size, if needed */ while (len > 0 && device_instance->device_state == STATE_CONFIGURED) { usbtty_poll ();

space = maxlen - usbtty_output.size; /* Empty buffer here, if needed, to ensure space... */ if (space) { write_buffer (&usbtty_output); n = MIN (space, MIN (len, maxlen)); buf_push (&usbtty_output, str, n);

str += n; len -= n; } } }

I think... that *should* work.

Happy Monday.

Bryan