Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Nick Garnett]

"Robert Bryce" <rob@mobilia-os.com> writes:

> Just jumping in here...just a private email...

I've copied my response to the list since I believe it may be of
interest to the rest of the community.


> 
> 
> > >     Nick> In eCos device drivers are responsible for their own
> > >     Nick> concurrency control. While some drivers might be able to
> > >     Nick> cope with two threads executing in them concurrently, most
> > >     Nick> cannot. They must therefore implement some sort of mutual
> > >     Nick> exclusion to serialize the threads. By remarkable
> > >     Nick> coincidence, a mutex does exactly the right thing.
> [...]
> > >     Nick> A semaphore just would not work in this situation.
> 
> I disagree.  Wasn't there a famous paper decades ago that proved that
> mutexes could be implemented with semaphores and vice versa?  If so (and it
> is), then they provide equal functionality and either/or could be used.
> Then you look at the implementation advantages (not features) to choose what
> to use.

While there are both semaphore and mutex+cv implementations of all the
standard synchronization examples (producer/consumer, resource
management etc.), the semaphore versions tend to be tricky and usually
require a couple of pages of explanation.

Mutex+cv usually permits a very straightforward implementation of any
synchronization activity.

Semaphores are also lacking some features that are useful in real time
systems. Without a concept of an owner they cannot participate in
priority inheritance or similar protocols. Semaphores are independent
of each other and it is difficult to implement a
wait-for-something-to-happen thread queue without opening yourself up
to deadlock or races -- much of the trickiness is aimed at working
around this. Mutex+cv on the other hand makes such things simple with
the linkage between condition variables and a mutex and the semantics
of the condition variable wait operation.

If I had my way, mutexes and condition variables would be the only
synchronization primitives available in the OS.


> > From a real time point of view, IO should be performed at the priority
> > of the requesting thread.
> 
> Alright, this is what spurred my email.  Is/how much is eCos actually
> real-time?  Real-time scheduling (hard or soft) (guarantee-oriented - where
> a thread says it needs so many cycles of execution time to complete by a
> specific real time) is NOT priority-based (performance-oriented) scheduling.
> Honestly, I haven't dived too deep into eCos (just enough to play on my
> iPaq), but I've come across other so-called real-time systems that really
> weren't - they just played with thread priorities to let certain threads
> exhibit real-time characteristics, or appear to be by chance real-time.

eCos is just as real time as its main rivals: VxWorks, ThreadX, pSOS
etc. I have always resisted describing eCos as hard real time,
although it is quite possible to get hard response times with care.

In general, the OS on its own is not real time, only an entire system,
OS+application, can be described as real time. All an RTOS designer
can do is select data structures and algorithms that permit a real
time system to be built. A real time OS cannot make non-real time code
become real time. A good example is the BSD TCP/IP stack, which was
never intended to be used in a real time system, all we can do in eCos
is to ensure that whatever the stack does, it doesn't interfere with
the real time components of the application.

-- 
Nick Garnett                                eCos Kernel Architect
http://www.ecoscentric.com           The eCos and RedBoot experts
Visit us at ESC Silicon Valley, April 4-6 2006,        Booth 2044 
San Jose McEnery Convention Center http://www.embedded.com/esc/sv


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

RE: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Robert Bryce]

Hi Nick,

> If I had my way, mutexes and condition variables would be the only
> synchronization primitives available in the OS.

Hm.  I will agree that one set of primitives is all that an OS needs to
provide.  Keeps the model simpler.  I'm really indifferent on which set,
though I do prefer semaphores.  In the end, it's just not a big deal.

> In general, the OS on its own is not real time, only an entire system,
> OS+application, can be described as real time. [...]
> A real time OS cannot make non-real time code become real time.

Yes!  Conversely, though, an (otherwise) real-time app isn't without a
real-time OS.

I did some work with hierarchical scheduling that could support both
real-time and non-real-time app's simultaneously - concerns are similar to
your example (IP stack) - inter-process dependencies.  There's been some
interesting work in hybrid schedulers too.

Thanks for the discussions.  Interesting.

--Rob


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Bart Veer]

>>>>> "Rob" == Robert Bryce <rob@mobilia-os.com> writes:

    >> If I had my way, mutexes and condition variables would be the only
    >> synchronization primitives available in the OS.

    Rob> Hm. I will agree that one set of primitives is all that an OS
    Rob> needs to provide. Keeps the model simpler. I'm really
    Rob> indifferent on which set, though I do prefer semaphores. In
    Rob> the end, it's just not a big deal.

I would disagree. From a theoretical perspective it is true that
everything can be done with just mutexes and condition variables. That
is a general-purpose combination. Also it is true that having too many
primitives causes confusion and makes maintenance more difficult.
However other primitives, especially semaphores, allow certain common
operations to be performed using less code, less data, fewer cpu
cycles, and less impact on real-time responsiveness. I would not want
semaphores as my only primitive, but I also do not want
mutexes/condition variables as the only one.

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

RE: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Derek Bouius]

I did a double check, and the only difference is the usbs code. Fresh recompiles for both and the same results. 

This shouldn't be just an effect of the usb 'millisecond frame' since it is a high speed device. The data rates are around 10 MB/s
with packet sizes of 512 bytes. So we are getting about 20 packets/msec frame, with the semaphore/complete flag being posted once
every 128 packets (64KB or about 6 milliseconds).

Our system is very sensitive to instruction cache performance, so this could be just a side effect of that too (ie. the code changes
position a bit and is now spanning a page boundary and has to be flushed).

After writing the above I repeated the test with instruction cache disabled. There is very little performance drop (a few percent in
favour of the semaphore code) now, but the system throughput drops 80 percent. 

Derek
 

-----Original Message-----
From: Bart Veer [mailto:bartv@ecoscentric.com] 
Sent: Monday, February 20, 2006 1:09 PM
To: derek.bouius@sympatico.ca
Cc: ecos-discuss@sourceware.org
Subject: Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

>>>>> "Derek" == Derek Bouius <derek.bouius@sympatico.ca> writes:

    Derek> I tested the below patch and it works.
    Derek> The throughput performance change compared to using a
    Derek> semaphore (in our system) is
    Derek> usbs_devtab_cwrite : 17% drop
    Derek> usbs_devtab_cread : 53% drop
    Derek> which is quite significant!

Very significant - but I am rather sceptical. Granted, semaphores will
be more efficient. However for USB communication throughput is
constrained by the host and by USB's millisecond frames. A massive
performance difference like the above would imply that a lot of
transfers are now being pushed into the next frame. That is possible
if the eCos target's performance is borderline when it comes to
feeding data to the host, but it would be a bit of a coincidence. Are
you sure that there are no other differences, e.g. debug vs.
production builds?

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Bart Veer]

>>>>> "Derek" == Derek Bouius <derek.bouius@sympatico.ca> writes:

    Derek> I tested the below patch and it works.
    Derek> The throughput performance change compared to using a
    Derek> semaphore (in our system) is
    Derek> usbs_devtab_cwrite : 17% drop
    Derek> usbs_devtab_cread : 53% drop
    Derek> which is quite significant!

Very significant - but I am rather sceptical. Granted, semaphores will
be more efficient. However for USB communication throughput is
constrained by the host and by USB's millisecond frames. A massive
performance difference like the above would imply that a lot of
transfers are now being pushed into the next frame. That is possible
if the eCos target's performance is borderline when it comes to
feeding data to the host, but it would be a bit of a coincidence. Are
you sure that there are no other differences, e.g. debug vs.
production builds?

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

RE: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Derek Bouius]

I tested the below patch and it works.
The throughput performance change compared to using a semaphore (in our system) is 
usbs_devtab_cwrite : 17% drop
usbs_devtab_cread : 53% drop
which is quite significant!

Derek

-----Original Message-----
From: Bart Veer [mailto:bartv@ecoscentric.com] 
Sent: Monday, February 20, 2006 8:42 AM
To: derek.bouius@sympatico.ca
Cc: andrew@lunn.ch; munz@speag.ch; ecos-discuss@sourceware.org
Subject: Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

>>>>> "Derek" == Derek Bouius <derek.bouius@sympatico.ca> writes:

    Derek> The problem is a race condition between checking the completed flag and executing the callback.
    Derek> I will describe the flow of the usbs_devtab_write:
    Derek> 1. Initialize the variables (signal, mutex, etc)
    Derek> 2. Setup the callback function - (Callback sets the signal) 
    Derek> 3. Start the transfer
    Derek> 4. Lock the mutex.
    Derek> 5. Check if wait.completed
    Derek> ...race condition....
    Derek> 	6. Wait for signal -- waits here forever in some cases if the callback was executed after #5 and before #6
    Derek> 7. Unlock the mutex.
    Derek> ...
    Derek> ..

    Derek> What happens in the lower level driver is that the (#3)
    Derek> start_tx_fn() enables the interrupt. That can fire
    Derek> immediately and call the callback from within the dsr.
    Derek> Executing #5 above can happen inbetween the enable of the
    Derek> interrupt and the actual interrupt.

    Derek> The same process can occur in the read.

I agree there is a race condition in the current code. My preferred
solution would be to replace the mutex/condition variable combo in
this code with a semaphore, as per other ecos-discuss postings. An
alternative approach for now, which works with the existing driver
API, is to add some scheduler locking as well. The patch below is
untested (I don't have any USB-capable hardware setup right now) but
should do the trick.

Bart

Index: usbs.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/usb/slave/current/src/usbs.c,v
retrieving revision 1.5
diff -u -r1.5 usbs.c
--- usbs.c	23 May 2002 23:06:36 -0000	1.5
+++ usbs.c	20 Feb 2006 13:37:06 -0000
@@ -111,9 +111,11 @@
     (*endpoint->start_tx_fn)(endpoint);
     
     cyg_drv_mutex_lock(&wait.lock);
+    cyg_drv_dsr_lock();
     while (!wait.completed) {
         cyg_drv_cond_wait(&wait.signal);
     }
+    cyg_drv_dsr_unlock();
     cyg_drv_mutex_unlock(&wait.lock);
     if (wait.result < 0) {
         result = wait.result;
@@ -155,9 +157,11 @@
     endpoint->complete_data     = (void*) &wait;
     (*endpoint->start_rx_fn)(endpoint);
     cyg_drv_mutex_lock(&wait.lock);
+    cyg_drv_dsr_lock();
     while (!wait.completed) {
         cyg_drv_cond_wait(&wait.signal);
     }
+    cyg_drv_dsr_unlock();
     cyg_drv_mutex_unlock(&wait.lock);
     if (wait.result < 0) {
         result = wait.result;


-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Bart Veer]

>>>>> "Nick" == Nick Garnett <nickg@ecoscentric.com> writes:

    <snip>
    Nick> This corresponds almost exactly to what I would expect to be
    Nick> done, and have implemented in the past. I would use a
    Nick> separate condition variable to implement the gate-keeper to
    Nick> keep response time on the event condition variable low.

    Nick> Also, the completed variable is seldom needed. Instead the
    Nick> thread will be testing whatever changes the ISR/DSR have
    Nick> made to the state of the device. This is particularly
    Nick> important when there might be threads waiting for a variety
    Nick> of different conditions to become true.

True for the case where multiple threads may be manipulating a device,
but not for the simple cases I have come across in USB/SPI/I2C. If the
DSR decides to post the semaphore/condition variable then the thread
knows the I/O has completed. There is no need to check the device
state. In fact, depending on the device there may be no easy way to
detect completion at the thread level. In this circumstance the
completed flag is necessary to avoid race conditions between the DSR
and thread.

    <snip>

    >> A semaphore and a condition variable are the same size
    >> (assuming 32-bit pointers), so the second approach saves space
    >> for the busy and completed flags. That may be two ints, two
    >> bytes, or just two bits in a flags field (although testing bits
    >> is likely to impose a code size penalty). It is still an
    >> overhead.

    Nick> This is not actually true. a condition variable is just a
    Nick> thread queue. A semaphore is a thread queue plus a counter.
    Nick> The counter is effectively the equivalent of the completed
    Nick> variable above.

Actually, a condition variable is thread queue plus pointer to mutex,
whereas a semaphore is thread queue plus counter.

    >> Re. code size, the second approach eliminates four function
    >> calls, four assignments, and two loops. A fairly clear gain. If
    >> there are multiple threads attempting to perform I/O to the
    >> same device then the cyg_drv_cond_broadcast() call will also
    >> result in threads waking up and having to go to sleep again
    >> immediately because the device is still busy. This could be
    >> eliminated using a separate condition variable, but that adds
    >> other overhead.

    Nick> The difference is less than you think. The completed
    Nick> variable is logically unnecessary, the semaphore has the
    Nick> additional counter. Internally semaphore wait is logically
    Nick> equvalent to

    Nick>     while( counter <= 0 )
    Nick>         cyg_drv_cond_wait( sem );
    Nick>     counter--;

    Nick> It just moves the loop elsewhere.

They are logically equivalent, but condition variables are still more
expensive. Inside the driver it is:

   (start the I/O operation)			(start the I/O operation)
   cyg_drv_dsr_lock();			VS	cyg_drv_sem_wait(&wait);
   while (!<test for completion>) {
      cyg_drv_cond_wait(&wait);
   }
   cyg_drv_dsr_unlock();
   (perform any clean-ups)                      (perform any clean-ups);

plus cyg_drv_cond_wait() is itself more expensive than a semaphore
wait because it needs to manipulate the mutex twice.

    >> In terms of real-time behaviour, the condition variable
    >> approach has to lock the scheduler albeit for only a short
    >> time.

    Nick> This will always be necessary, if the thread is to examine
    Nick> and manipulate data shared with the ISR/DSR it is going to
    Nick> have to either disable interrupts or lock the scheduler.

With a semaphore there is no need for the thread to examine shared
state. The semaphore itself acts as the only shared state of interest:
whether or not the operation has completed. Obviously for devices like
serial something more complicated will be needed, but for the
USB/SPI/I2C devices I have come across a semaphore suffices.

    >> More worryingly mutex priority inversion protection is not
    >> going to help. If you have a low priority thread currently
    >> performing I/O, a high priority thread wanting access to the
    >> same device, and an intermediate thread hogging the cpu, then
    >> when the I/O has completed you have a priority inversion
    >> situation: the high priority thread is blocked on a condition
    >> variable, not a semaphore.

    Nick> Mutexes should not be used to queue threads for access to
    Nick> critical sections in which the current owner suspends using
    Nick> something other than a condition variable. Mutexes are for
    Nick> mutual exclusion, and should be treated more like spinlocks.
    Nick> Long-term waiters should be transferred to condition
    Nick> variables, and not left on the mutex.

    Nick> From a real time point of view, IO should be performed at
    Nick> the priority of the requesting thread.

I am not convinced that is universally true. In the case of an I2C bus
where a high priority thread wants to access device A and a low
priority thread wants to access device B, do we really want the high
priority thread blocked indefinitely because some intermediate thread
is hogging the cpu?

I think the answer depends on both the application's requirements and
the implementation of the device driver. If most of the I/O happens
via a DMA engine or in ISRs, with the thread only woken up once the
I/O has completed, then boosting the low priority's thread seems
very sensible. If the driver involves bit-banging and/or polling it
may be better to stick at the lower priority - depending on just how
important it is that the high priority thread can access its device.

Obviously the application can and probably should resolve any problems
by careful use of priorities, but preferring an approach that risks
priority inversion problems seems wrong. Using a mutex/semaphore
combination gives reasonable behaviour, even if it does mean a mutex
is claimed for rather longer than usual.

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Nick Garnett]

Bart Veer <bartv@ecoscentric.com> writes:

> >>>>> "Nick" == Nick Garnett <nickg@ecoscentric.com> writes:
> 
>     <snip>
>     >> Does anybody remember why counting semaphores were left out of the
>     >> driver API?
> 
>     Nick> In eCos device drivers are responsible for their own
>     Nick> concurrency control. While some drivers might be able to
>     Nick> cope with two threads executing in them concurrently, most
>     Nick> cannot. They must therefore implement some sort of mutual
>     Nick> exclusion to serialize the threads. By remarkable
>     Nick> coincidence, a mutex does exactly the right thing.
> 
>     Nick> Once a thread is in the driver it examines its state.
>     Nick> Depending on what it finds, it may need to wait for data to
>     Nick> arrive, or start data transmission, and then wait for it to
>     Nick> finish. The wait takes the form of a loop, testing some
>     Nick> condition of the device driver and calling
>     Nick> cyg_dev_cond_wait().
> 
>     Nick> Waiting on a condition variable unlocks the mutex, which may
>     Nick> allow a second thread through. This thread may want to do
>     Nick> something else in the device (transmit rather than receive,
>     Nick> select, non-blocking IO, ioctl etc.) and may either quickly
>     Nick> exit or wait elsewhere. If it follows the same path as the
>     Nick> first thread it will end up queued behind it on the same
>     Nick> condition variable.
> 
>     Nick> A semaphore just would not work in this situation. For a
>     Nick> start it does not atomically release the mutex, and just
>     Nick> unlocking the mutex before waiting on the semaphore (and
>     Nick> relocking after) opens up a race window. Also, the semaphore
>     Nick> count is not very useful. The fact that the ISR/DSR have run
>     Nick> is encoded in the state of the device driver, which is what
>     Nick> the loop around the condition variable wait should be
>     Nick> testing. For example, in the serial drivers, threads wait
>     Nick> for characters to be inserted into the receive queue, or for
>     Nick> space to become available in the transmit queue.
> 
>     Nick> If the USB drivers are losing wakeups I can only assume that
>     Nick> the wait loops are testing the wrong conditions. It should
>     Nick> not be too hard to work out what the correct condition is
>     Nick> and fix it.
> 
> OK, I think I understand your reasoning. It makes a great deal of
> sense in the context of serial drivers where e.g. data may arrive
> asynchronously and end up in ring buffers. However I am not sure it
> makes as much sense for other kinds of devices where no I/O happens
> until it is explicitly initiated. USB, I2C and SPI all fail into that
> category.

Synchronous devices are simply a degenerate case of the general case
of asynchronous devices. A mechanism that is suitable for asynchronous
devices will work for synchronous devices just as easily.

> 
> To get things working right with condition variables the code would
> have to do something like the following:
> 
> 1) in the device structure:
> 
>        cyg_drv_mutex_t	lock;
>        cyg_drv_cond_t   wait;
>        cyg_bool		busy;
>        cyg_bool		completed;
> 
> 2) at the start of an operation, typically somewhere in the
>    device-specific I/O layer:
> 
>        cyg_drv_mutex_lock(&lock);
>        while (busy) {
>            cyg_drv_cond_wait(&wait);
>        }
>        busy = 1;
>        (call into the device driver to perform the actual I/O)
>        busy = 0;
>        cyg_drv_mutex_unlock(&lock);
>        cyg_drv_cond_signal(&wait);
> 
> 3) inside the device driver, called with lock claimed:
> 
>        completed = 0;
>        (start the I/O operation)
>        cyg_drv_dsr_lock();
>        while (!completed) {
>          cyg_drv_cond_wait(&wait);
>        }
>        cyg_drv_dsr_unlock();
>        (perform any clean-ups that may be necessary)
> 
> 4) in the DSR:
> 
>        completed = 1;
>        cyg_drv_cond_broadcast(&wait);

This corresponds almost exactly to what I would expect to be done, and
have implemented in the past. I would use a separate condition
variable to implement the gate-keeper to keep response time on the
event condition variable low.

Also, the completed variable is seldom needed. Instead the thread will
be testing whatever changes the ISR/DSR have made to the state of the
device. This is particularly important when there might be threads
waiting for a variety of different conditions to become true.

> 
> What I would prefer to see is:
> 
> 1) in the device structure:
> 
>        cyg_drv_mutex_t		lock;
>        cyg_drv_sem_t		wait;
> 
> 2) at the start of an I/O operation:
> 
>        cyg_drv_mutex_lock(&lock);
>        (call into the device driver to perform the actual I/O)
>        cyg_drv_mutex_unlock(&lock);
> 
> 3) inside the device driver:
> 
>        (start the I/O operation)
>        cyg_drv_sem_wait(&wait);
>        (perform any clean-ups that may be necessary);
> 
> 4) in the DSR:
> 
>        cyg_drv_sem_post(&wait);
> 

> 
> A semaphore and a condition variable are the same size (assuming
> 32-bit pointers), so the second approach saves space for the busy and
> completed flags. That may be two ints, two bytes, or just two bits in
> a flags field (although testing bits is likely to impose a code size
> penalty). It is still an overhead.

This is not actually true. a condition variable is just a thread
queue. A semaphore is a thread queue plus a counter. The counter is
effectively the equivalent of the completed variable above.

> 
> Re. code size, the second approach eliminates four function calls,
> four assignments, and two loops. A fairly clear gain. If there are
> multiple threads attempting to perform I/O to the same device then
> the cyg_drv_cond_broadcast() call will also result in threads waking
> up and having to go to sleep again immediately because the device is
> still busy. This could be eliminated using a separate condition
> variable, but that adds other overhead.

The difference is less than you think. The completed variable is
logically unnecessary, the semaphore has the additional
counter. Internally semaphore wait is logically equvalent to

    while( counter <= 0 )
        cyg_drv_cond_wait( sem );
    counter--;

It just moves the loop elsewhere.

There is always a tradeoff to be made between code size, data size,
runtime performance, real time response and maintainability. 

> 
> In terms of real-time behaviour, the condition variable approach has
> to lock the scheduler albeit for only a short time.

This will always be necessary, if the thread is to examine and
manipulate data shared with the ISR/DSR it is going to have to either
disable interrupts or lock the scheduler. 

> More worryingly
> mutex priority inversion protection is not going to help. If you have
> a low priority thread currently performing I/O, a high priority thread
> wanting access to the same device, and an intermediate thread hogging
> the cpu, then when the I/O has completed you have a priority inversion
> situation: the high priority thread is blocked on a condition
> variable, not a semaphore.

Mutexes should not be used to queue threads for access to critical
sections in which the current owner suspends using something other
than a condition variable. Mutexes are for mutual exclusion, and
should be treated more like spinlocks. Long-term waiters should be
transferred to condition variables, and not left on the mutex.

From a real time point of view, IO should be performed at the priority
of the requesting thread.

> 
> So although I accept that there are situations where general purpose
> condition variables will be more appropriate, I believe the semaphore
> approach is better for many types of device. Hence we should
> reconsider adding semaphores to the driver API.

The API was designed to provide a template for how drivers should be
written. Providing too many options will result in drivers being badly
constructed, which may fail under unexpected conditions.

-- 
Nick Garnett                                          eCos Kernel Architect
http://www.ecoscentric.com                     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Bart Veer]

>>>>> "Derek" == Derek Bouius <derek.bouius@sympatico.ca> writes:

    Derek> The problem is a race condition between checking the completed flag and executing the callback.
    Derek> I will describe the flow of the usbs_devtab_write:
    Derek> 1. Initialize the variables (signal, mutex, etc)
    Derek> 2. Setup the callback function - (Callback sets the signal) 
    Derek> 3. Start the transfer
    Derek> 4. Lock the mutex.
    Derek> 5. Check if wait.completed
    Derek> ...race condition....
    Derek> 	6. Wait for signal -- waits here forever in some cases if the callback was executed after #5 and before #6
    Derek> 7. Unlock the mutex.
    Derek> ...
    Derek> ..

    Derek> What happens in the lower level driver is that the (#3)
    Derek> start_tx_fn() enables the interrupt. That can fire
    Derek> immediately and call the callback from within the dsr.
    Derek> Executing #5 above can happen inbetween the enable of the
    Derek> interrupt and the actual interrupt.

    Derek> The same process can occur in the read.

I agree there is a race condition in the current code. My preferred
solution would be to replace the mutex/condition variable combo in
this code with a semaphore, as per other ecos-discuss postings. An
alternative approach for now, which works with the existing driver
API, is to add some scheduler locking as well. The patch below is
untested (I don't have any USB-capable hardware setup right now) but
should do the trick.

Bart

Index: usbs.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/usb/slave/current/src/usbs.c,v
retrieving revision 1.5
diff -u -r1.5 usbs.c
--- usbs.c	23 May 2002 23:06:36 -0000	1.5
+++ usbs.c	20 Feb 2006 13:37:06 -0000
@@ -111,9 +111,11 @@
     (*endpoint->start_tx_fn)(endpoint);
     
     cyg_drv_mutex_lock(&wait.lock);
+    cyg_drv_dsr_lock();
     while (!wait.completed) {
         cyg_drv_cond_wait(&wait.signal);
     }
+    cyg_drv_dsr_unlock();
     cyg_drv_mutex_unlock(&wait.lock);
     if (wait.result < 0) {
         result = wait.result;
@@ -155,9 +157,11 @@
     endpoint->complete_data     = (void*) &wait;
     (*endpoint->start_rx_fn)(endpoint);
     cyg_drv_mutex_lock(&wait.lock);
+    cyg_drv_dsr_lock();
     while (!wait.completed) {
         cyg_drv_cond_wait(&wait.signal);
     }
+    cyg_drv_dsr_unlock();
     cyg_drv_mutex_unlock(&wait.lock);
     if (wait.result < 0) {
         result = wait.result;


-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Bart Veer]

>>>>> "Nick" == Nick Garnett <nickg@ecoscentric.com> writes:

    <snip>
    >> Does anybody remember why counting semaphores were left out of the
    >> driver API?

    Nick> In eCos device drivers are responsible for their own
    Nick> concurrency control. While some drivers might be able to
    Nick> cope with two threads executing in them concurrently, most
    Nick> cannot. They must therefore implement some sort of mutual
    Nick> exclusion to serialize the threads. By remarkable
    Nick> coincidence, a mutex does exactly the right thing.

    Nick> Once a thread is in the driver it examines its state.
    Nick> Depending on what it finds, it may need to wait for data to
    Nick> arrive, or start data transmission, and then wait for it to
    Nick> finish. The wait takes the form of a loop, testing some
    Nick> condition of the device driver and calling
    Nick> cyg_dev_cond_wait().

    Nick> Waiting on a condition variable unlocks the mutex, which may
    Nick> allow a second thread through. This thread may want to do
    Nick> something else in the device (transmit rather than receive,
    Nick> select, non-blocking IO, ioctl etc.) and may either quickly
    Nick> exit or wait elsewhere. If it follows the same path as the
    Nick> first thread it will end up queued behind it on the same
    Nick> condition variable.

    Nick> A semaphore just would not work in this situation. For a
    Nick> start it does not atomically release the mutex, and just
    Nick> unlocking the mutex before waiting on the semaphore (and
    Nick> relocking after) opens up a race window. Also, the semaphore
    Nick> count is not very useful. The fact that the ISR/DSR have run
    Nick> is encoded in the state of the device driver, which is what
    Nick> the loop around the condition variable wait should be
    Nick> testing. For example, in the serial drivers, threads wait
    Nick> for characters to be inserted into the receive queue, or for
    Nick> space to become available in the transmit queue.

    Nick> If the USB drivers are losing wakeups I can only assume that
    Nick> the wait loops are testing the wrong conditions. It should
    Nick> not be too hard to work out what the correct condition is
    Nick> and fix it.

OK, I think I understand your reasoning. It makes a great deal of
sense in the context of serial drivers where e.g. data may arrive
asynchronously and end up in ring buffers. However I am not sure it
makes as much sense for other kinds of devices where no I/O happens
until it is explicitly initiated. USB, I2C and SPI all fail into that
category.

To get things working right with condition variables the code would
have to do something like the following:

1) in the device structure:

       cyg_drv_mutex_t	lock;
       cyg_drv_cond_t   wait;
       cyg_bool		busy;
       cyg_bool		completed;

2) at the start of an operation, typically somewhere in the
   device-specific I/O layer:

       cyg_drv_mutex_lock(&lock);
       while (busy) {
           cyg_drv_cond_wait(&wait);
       }
       busy = 1;
       (call into the device driver to perform the actual I/O)
       busy = 0;
       cyg_drv_mutex_unlock(&lock);
       cyg_drv_cond_signal(&wait);

3) inside the device driver, called with lock claimed:

       completed = 0;
       (start the I/O operation)
       cyg_drv_dsr_lock();
       while (!completed) {
         cyg_drv_cond_wait(&wait);
       }
       cyg_drv_dsr_unlock();
       (perform any clean-ups that may be necessary)

4) in the DSR:

       completed = 1;
       cyg_drv_cond_broadcast(&wait);

(For simplicity this ignores incoming signals aborting the drv
routines. Typically for something like USB or SPI there is no sensible
way to abort a transfer that is in progress, so the I/O is not
interruptible and signals have to be ignored).

What I would prefer to see is:

1) in the device structure:

       cyg_drv_mutex_t		lock;
       cyg_drv_sem_t		wait;

2) at the start of an I/O operation:

       cyg_drv_mutex_lock(&lock);
       (call into the device driver to perform the actual I/O)
       cyg_drv_mutex_unlock(&lock);

3) inside the device driver:

       (start the I/O operation)
       cyg_drv_sem_wait(&wait);
       (perform any clean-ups that may be necessary);

4) in the DSR:

       cyg_drv_sem_post(&wait);

(again allowing for signals will complicate this a bit).

A semaphore and a condition variable are the same size (assuming
32-bit pointers), so the second approach saves space for the busy and
completed flags. That may be two ints, two bytes, or just two bits in
a flags field (although testing bits is likely to impose a code size
penalty). It is still an overhead.

Re. code size, the second approach eliminates four function calls,
four assignments, and two loops. A fairly clear gain. If there are
multiple threads attempting to perform I/O to the same device then
the cyg_drv_cond_broadcast() call will also result in threads waking
up and having to go to sleep again immediately because the device is
still busy. This could be eliminated using a separate condition
variable, but that adds other overhead.

In terms of real-time behaviour, the condition variable approach has
to lock the scheduler albeit for only a short time. More worryingly
mutex priority inversion protection is not going to help. If you have
a low priority thread currently performing I/O, a high priority thread
wanting access to the same device, and an intermediate thread hogging
the cpu, then when the I/O has completed you have a priority inversion
situation: the high priority thread is blocked on a condition
variable, not a semaphore.

So although I accept that there are situations where general purpose
condition variables will be more appropriate, I believe the semaphore
approach is better for many types of device. Hence we should
reconsider adding semaphores to the driver API.

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Nick Garnett]

Bart Veer <bartv@ecoscentric.com> writes:

> >>>>> "Gary" == Gary Thomas <gary@mlbassoc.com> writes:
> 
>     Gary> On Thu, 2006-02-16 at 17:25 +0100, Andrew Lunn wrote:
>     >> On Thu, Feb 16, 2006 at 11:15:25AM -0500, Derek Bouius wrote:
>     >> 
>     >> > I am not registered for the mailing list, but peruse it once in a
>     >> > while, so I am not sure if my mail will go through to it. Feel free
>     >> > to repost it if it doesn't.
>     >> 
>     >> It got through.
>     >> 
>     >> > What we did to fix the locking issue was change the mutex to a
>     >> > semaphore. See the patch. It seems to work reliably.
>     >> 
>     >> Did you understand what the problem was with the condition variable?
>     >> Could you explain it?
> 
>     Gary> This seems like a pretty heavyweight solution.  I think that Bart
>     Gary> may want/need to review this before it's accepted into CVS.
> 
> Yes, although it has been a while since I looked at that code.
> 
> A problem with semaphores is that they are only available in kernel
> configurations. The driver API only has condition variables as a way
> of signalling events from a DSR to a thread. That is fine for
> complicated I/O, but in most of my drivers I only need something
> simple. A counting semaphore in the driver API would make it easier to
> write drivers, and would require less code and data than a
> mutex/condition variable combination.
> 
> Does anybody remember why counting semaphores were left out of the
> driver API?

In eCos device drivers are responsible for their own concurrency
control. While some drivers might be able to cope with two threads
executing in them concurrently, most cannot. They must therefore
implement some sort of mutual exclusion to serialize the threads. By
remarkable coincidence, a mutex does exactly the right thing.

Once a thread is in the driver it examines its state. Depending on
what it finds, it may need to wait for data to arrive, or start data
transmission, and then wait for it to finish. The wait takes the form
of a loop, testing some condition of the device driver and calling
cyg_dev_cond_wait().

Waiting on a condition variable unlocks the mutex, which may allow a
second thread through. This thread may want to do something else in
the device (transmit rather than receive, select, non-blocking IO,
ioctl etc.) and may either quickly exit or wait elsewhere. If it
follows the same path as the first thread it will end up queued behind
it on the same condition variable.

A semaphore just would not work in this situation. For a start it does
not atomically release the mutex, and just unlocking the mutex before
waiting on the semaphore (and relocking after) opens up a race
window. Also, the semaphore count is not very useful. The fact that
the ISR/DSR have run is encoded in the state of the device driver,
which is what the loop around the condition variable wait should be
testing. For example, in the serial drivers, threads wait for
characters to be inserted into the receive queue, or for space to
become available in the transmit queue. 

If the USB drivers are losing wakeups I can only assume that the wait
loops are testing the wrong conditions. It should not be too hard to
work out what the correct condition is and fix it.


-- 
Nick Garnett                                          eCos Kernel Architect
http://www.ecoscentric.com                     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Bart Veer]

>>>>> "Gary" == Gary Thomas <gary@mlbassoc.com> writes:

    Gary> On Thu, 2006-02-16 at 17:25 +0100, Andrew Lunn wrote:
    >> On Thu, Feb 16, 2006 at 11:15:25AM -0500, Derek Bouius wrote:
    >> 
    >> > I am not registered for the mailing list, but peruse it once in a
    >> > while, so I am not sure if my mail will go through to it. Feel free
    >> > to repost it if it doesn't.
    >> 
    >> It got through.
    >> 
    >> > What we did to fix the locking issue was change the mutex to a
    >> > semaphore. See the patch. It seems to work reliably.
    >> 
    >> Did you understand what the problem was with the condition variable?
    >> Could you explain it?

    Gary> This seems like a pretty heavyweight solution.  I think that Bart
    Gary> may want/need to review this before it's accepted into CVS.

Yes, although it has been a while since I looked at that code.

A problem with semaphores is that they are only available in kernel
configurations. The driver API only has condition variables as a way
of signalling events from a DSR to a thread. That is fine for
complicated I/O, but in most of my drivers I only need something
simple. A counting semaphore in the driver API would make it easier to
write drivers, and would require less code and data than a
mutex/condition variable combination.

Does anybody remember why counting semaphores were left out of the
driver API?

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[oliver munz @ s p e a g]

I think cyg_drv_dsr_lock() will block the loop, because You need the DSR to 
update the variable and to release the wait-funcion.

I think, the best option may be to modify the condition cyg_drv_cond_wait() 
so, that it rememers an signal after the initialisation. If so, it would 
work fine.

I don't know, where it is interessting to forget signals...

Oliver


----- Original Message ----- 
From: "Derek Bouius" <derek.bouius@sympatico.ca>
To: "'oliver munz @ s p e a g'" <munz@speag.ch>; <andrew@lunn.ch>
Cc: <ecos-discuss@sourceware.org>
Sent: Thursday, February 16, 2006 9:07 PM
Subject: RE: [ECOS] How to debug synchronisation in the usbs.c in a new 
usb-driver for the ARM at91sam7s...


>I just tried implementing it with event flags, and it works fine too, with 
>no noticable performance difference.
>
> Gary, I have studied the different options but I can't determine why this 
> is a heavier solution. It seems like the only possible
> one.
> Any other method with condition variables ends up in a race condition 
> deadlock. (Please correct me if I'm wrong though!)
>
> Just doing some more reading about the cyg_drv_cond_wait()... "Note that 
> this function performs an implicit scheduler unlock/relock
> sequence, so that it may be used within an explicit 
> cyg_drv_dsr_lock()...cyg_drv_dsr_unlock()  structure."
> What if we did the dsr_lock/unlock around the while loop? I'll have to try 
> it.
>
> Derek


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

RE: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Derek Bouius]

I just tried implementing it with event flags, and it works fine too, with no noticable performance difference. 

Gary, I have studied the different options but I can't determine why this is a heavier solution. It seems like the only possible
one.
Any other method with condition variables ends up in a race condition deadlock. (Please correct me if I'm wrong though!)

Just doing some more reading about the cyg_drv_cond_wait()... "Note that this function performs an implicit scheduler unlock/relock
sequence, so that it may be used within an explicit cyg_drv_dsr_lock()...cyg_drv_dsr_unlock()  structure."
What if we did the dsr_lock/unlock around the while loop? I'll have to try it.
 
Derek
 

-----Original Message-----
From: oliver munz @ s p e a g [mailto:munz@speag.ch] 
Sent: Thursday, February 16, 2006 11:47 AM
To: Derek Bouius; andrew@lunn.ch
Cc: ecos-discuss@sourceware.org
Subject: Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

Thanks for the solution...

Does this work whitout kernel? Would it be better to use event-flags?

Thanks
Oliver Munz


----- Original Message ----- 
From: "Derek Bouius" <derek.bouius@sympatico.ca>
To: <andrew@lunn.ch>; <munz@speag.ch>
Cc: <ecos-discuss@sourceware.org>
Sent: Thursday, February 16, 2006 5:15 PM
Subject: Re: [ECOS] How to debug synchronisation in the usbs.c in a new 
usb-driver for the ARM at91sam7s...


>I am not registered for the mailing list, but peruse it once in a while, so 
>I am not sure if my mail will go through to it. Feel
> free to repost it if it doesn't.
> What we did to fix the locking issue was change the mutex to a semaphore. 
> See the patch. It seems to work reliably.
>
> Cheers,
>
> Derek
> 


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

RE: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Derek Bouius]

The problem is a race condition between checking the completed flag and executing the callback.
I will describe the flow of the usbs_devtab_write:
1. Initialize the variables (signal, mutex, etc)
2. Setup the callback function - (Callback sets the signal) 
3. Start the transfer
4. Lock the mutex.
5. Check if wait.completed
...race condition....
	6. Wait for signal -- waits here forever in some cases if the callback was executed after #5 and before #6
7. Unlock the mutex.
...
..

What happens in the lower level driver is that the (#3) start_tx_fn() enables the interrupt. That can fire immediately and call the
callback from within the dsr. Executing #5 above can happen inbetween the enable of the interrupt and the actual interrupt.

The same process can occur in the read.

Derek


 

-----Original Message-----
From: Andrew Lunn [mailto:andrew@lunn.ch] 
Sent: Thursday, February 16, 2006 11:26 AM
To: Derek Bouius
Cc: andrew@lunn.ch; munz@speag.ch; ecos-discuss@sourceware.org
Subject: Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

On Thu, Feb 16, 2006 at 11:15:25AM -0500, Derek Bouius wrote:

> I am not registered for the mailing list, but peruse it once in a
> while, so I am not sure if my mail will go through to it. Feel free
> to repost it if it doesn't.

It got through.

> What we did to fix the locking issue was change the mutex to a
> semaphore. See the patch. It seems to work reliably.

Did you understand what the problem was with the condition variable?
Could you explain it?

        Thanks
                Andrew




-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[oliver munz @ s p e a g]

Thanks for the solution...

Does this work whitout kernel? Would it be better to use event-flags?

Thanks
Oliver Munz


----- Original Message ----- 
From: "Derek Bouius" <derek.bouius@sympatico.ca>
To: <andrew@lunn.ch>; <munz@speag.ch>
Cc: <ecos-discuss@sourceware.org>
Sent: Thursday, February 16, 2006 5:15 PM
Subject: Re: [ECOS] How to debug synchronisation in the usbs.c in a new 
usb-driver for the ARM at91sam7s...


>I am not registered for the mailing list, but peruse it once in a while, so 
>I am not sure if my mail will go through to it. Feel
> free to repost it if it doesn't.
> What we did to fix the locking issue was change the mutex to a semaphore. 
> See the patch. It seems to work reliably.
>
> Cheers,
>
> Derek
> 


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[oliver munz @ s p e a g]

I think the porblem is, that the condition-variable dont hold the signal. 
Only a thread in the wait, waits for the signal. If the DSR starts between 
the while() compare an the wait, the signal get loosed:


    ...
    while (!wait.completed) {
        DSR start not alowed
        cyg_drv_cond_wait(&wait.signal);
        ....


Thanks for the hints...

----- Original Message ----- 
From: "Andrew Lunn" <andrew@lunn.ch>
To: "Derek Bouius" <derek.bouius@sympatico.ca>
Cc: <andrew@lunn.ch>; <munz@speag.ch>; <ecos-discuss@sourceware.org>
Sent: Thursday, February 16, 2006 5:25 PM
Subject: Re: [ECOS] How to debug synchronisation in the usbs.c in a new 
usb-driver for the ARM at91sam7s...


> On Thu, Feb 16, 2006 at 11:15:25AM -0500, Derek Bouius wrote:
>
>> I am not registered for the mailing list, but peruse it once in a
>> while, so I am not sure if my mail will go through to it. Feel free
>> to repost it if it doesn't.
>
> It got through.
>
>> What we did to fix the locking issue was change the mutex to a
>> semaphore. See the patch. It seems to work reliably.
>
> Did you understand what the problem was with the condition variable?
> Could you explain it?
>
>        Thanks
>                Andrew
>
>
> 


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Gary Thomas]

On Thu, 2006-02-16 at 17:25 +0100, Andrew Lunn wrote:
> On Thu, Feb 16, 2006 at 11:15:25AM -0500, Derek Bouius wrote:
> 
> > I am not registered for the mailing list, but peruse it once in a
> > while, so I am not sure if my mail will go through to it. Feel free
> > to repost it if it doesn't.
> 
> It got through.
> 
> > What we did to fix the locking issue was change the mutex to a
> > semaphore. See the patch. It seems to work reliably.
> 
> Did you understand what the problem was with the condition variable?
> Could you explain it?

This seems like a pretty heavyweight solution.  I think that Bart
may want/need to review this before it's accepted into CVS.

-- 
Gary Thomas <gary@mlbassoc.com>


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Andrew Lunn]

On Thu, Feb 16, 2006 at 11:15:25AM -0500, Derek Bouius wrote:

> I am not registered for the mailing list, but peruse it once in a
> while, so I am not sure if my mail will go through to it. Feel free
> to repost it if it doesn't.

It got through.

> What we did to fix the locking issue was change the mutex to a
> semaphore. See the patch. It seems to work reliably.

Did you understand what the problem was with the condition variable?
Could you explain it?

        Thanks
                Andrew




-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Derek Bouius]

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

I am not registered for the mailing list, but peruse it once in a while, so I am not sure if my mail will go through to it. Feel
free to repost it if it doesn't.
What we did to fix the locking issue was change the mutex to a semaphore. See the patch. It seems to work reliably.
 
Cheers,
 
Derek

[-- Attachment #2: usbs.c.patch --]
[-- Type: application/octet-stream, Size: 2791 bytes --]

--- usbs_orig.c	2006-02-16 10:41:13.000000000 -0500
+++ usbs.c	2006-02-06 13:28:12.000000000 -0500
@@ -70,17 +70,17 @@
 typedef struct usbs_callback_data {
     bool                completed;
     int                 result;
-    cyg_drv_mutex_t     lock;
-    cyg_drv_cond_t      signal;
+    cyg_sem_t           sem;
 } usbs_callback_data;
 
+
 static void
 usbs_devtab_callback(void* arg, int result)
 {
     usbs_callback_data* callback_data = (usbs_callback_data*) arg;
     callback_data->result       = result;
     callback_data->completed    = true;
-    cyg_drv_cond_signal(&(callback_data->signal));
+    cyg_semaphore_post(&(callback_data->sem));
 }
     
 Cyg_ErrNo
@@ -94,8 +94,7 @@
     CYG_REPORT_FUNCTION();
     
     wait.completed      = 0;
-    cyg_drv_mutex_init(&wait.lock);
-    cyg_drv_cond_init(&wait.signal, &wait.lock);
+    cyg_semaphore_init(&wait.sem, 0);
 
     devtab_entry      = (cyg_devtab_entry_t*) handle;
     CYG_CHECK_DATA_PTR( devtab_entry, "A valid endpoint must be supplied");
@@ -110,19 +109,16 @@
     endpoint->complete_data     = (void*) &wait;
     (*endpoint->start_tx_fn)(endpoint);
     
-    cyg_drv_mutex_lock(&wait.lock);
     while (!wait.completed) {
-        cyg_drv_cond_wait(&wait.signal);
+       cyg_semaphore_wait(&wait.sem);
     }
-    cyg_drv_mutex_unlock(&wait.lock);
     if (wait.result < 0) {
         result = wait.result;
     } else {
         *size = wait.result;
     }
     
-    cyg_drv_cond_destroy(&wait.signal);
-    cyg_drv_mutex_destroy(&wait.lock);
+    cyg_semaphore_destroy(&wait.sem);
 
     CYG_REPORT_RETURN();
     return result;
@@ -139,8 +135,7 @@
     CYG_REPORT_FUNCTION();
     
     wait.completed      = 0;
-    cyg_drv_mutex_init(&wait.lock);
-    cyg_drv_cond_init(&wait.signal, &wait.lock);
+    cyg_semaphore_init(&wait.sem, 0);
 
     devtab_entry      = (cyg_devtab_entry_t*) handle;
     CYG_CHECK_DATA_PTR( devtab_entry, "A valid endpoint must be supplied");
@@ -153,20 +148,18 @@
     endpoint->buffer_size       = (int) *size;
     endpoint->complete_fn       = &usbs_devtab_callback;
     endpoint->complete_data     = (void*) &wait;
+
     (*endpoint->start_rx_fn)(endpoint);
-    cyg_drv_mutex_lock(&wait.lock);
     while (!wait.completed) {
-        cyg_drv_cond_wait(&wait.signal);
+       cyg_semaphore_wait(&wait.sem);
     }
-    cyg_drv_mutex_unlock(&wait.lock);
     if (wait.result < 0) {
         result = wait.result;
     } else {
         *size = wait.result;
     }
     
-    cyg_drv_cond_destroy(&wait.signal);
-    cyg_drv_mutex_destroy(&wait.lock);
+    cyg_semaphore_destroy(&wait.sem);
 
     CYG_REPORT_RETURN();
     return result;
@@ -409,6 +402,64 @@
     }
 }
 


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[Andrew Lunn]

On Thu, Feb 16, 2006 at 03:57:54AM +0100, oliver munz @ s p e a g wrote:
> I need a hint, what can go wrong in the usbs.c, or how to debug 
> condition-variables...
> 
> My problem is, that my code works reliable, if i use it whitout 
> kernel-objects, and that it works only a while in the usbs.c infrastructur, 
> and then blocks for ever...
> 
> When the usbs_devtab_cwrite() blocks, i see, that the complete-function is 
> called, but the thread never leave cyg_drv_cond_wait(&wait.signal);...
> 
> Beacuse the blocking is random after 5..20 transmissions, i can't set a 
> breake point on a interessting point, if i halt the system in the gdb i see 
> the empty thread...
> 
> what can be the reason for not realeasing the thread?

You might find kernel instrumentation useful for this problem.

http://ecos.sourceware.org/docs-latest/user-guide/kernel-instrumentation.html

It will log events into a circular buffer which you can later read out
and see what happened. 

        Gruss

                Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] How to debug synchronisation in the usbs.c in a new usb-driver for the ARM at91sam7s...

[oliver munz @ s p e a g]

[-- Attachment #1: Type: text/plain, Size: 2382 bytes --]

I need a hint, what can go wrong in the usbs.c, or how to debug 
condition-variables...

My problem is, that my code works reliable, if i use it whitout 
kernel-objects, and that it works only a while in the usbs.c infrastructur, 
and then blocks for ever...

When the usbs_devtab_cwrite() blocks, i see, that the complete-function is 
called, but the thread never leave cyg_drv_cond_wait(&wait.signal);...

Beacuse the blocking is random after 5..20 transmissions, i can't set a 
breake point on a interessting point, if i halt the system in the gdb i see 
the empty thread...

what can be the reason for not realeasing the thread?

/* THIS WORKS FOR AT LEAST 24H RESTLESS TRANSMISSION */

static void usbs_callback(void *arg, int result){

    *(int *) arg = result;

    if (!result){
        *(int *) arg = -EIO;
    }
}

void usb_write_wo_io(cyg_addrword_t data){

    int retcode;
    const char str[] = "configuration points supported by eCos resulted in 
systems that were faster to build (all"
            "the hard work was coded into the configuration rules) and 
resulted in smaller systems than manual"
            "methods could produce (because the automated rules were more 
all-seeing and all-knowing";

    while (true){

        if (usbs_sam7_ep0.state == USBS_STATE_CONFIGURED){

            retcode = 0;
            usbs_sam7_ep2.buffer = str;
            usbs_sam7_ep2.buffer_size = sizeof(str);
            usbs_sam7_ep2.complete_fn = usbs_callback;
            usbs_sam7_ep2.complete_data = (void *) &retcode;

            usbs_sam7_ep2.start_rx_fn(&usbs_sam7_ep2);

            while (!retcode){
            }
        }
    }
}


/* THIS BLOCKS AFTER A FEW TRANSMISSIONS */

void usb_write(cyg_addrword_t data){

    Cyg_ErrNo retcode;
    cyg_uint32 n;
    const char str[] = "configuration points supported by eCos resulted in 
systems that were faster to build (all"
            "the hard work was coded into the configuration rules) and 
resulted in smaller systems than manual"
            "methods could produce (because the automated rules were more 
all-seeing and all-knowing";

    while (true){

        if (usbs_sam7_ep0.state == USBS_STATE_CONFIGURED){

            n = sizeof(str);
            retcode = cyg_io_write(husbs[2], (void *) str, &n); /* handle to 
the usbs-write-endpoint */
        }
    }
}


Thanks for any hint...
Oliver Munz 

[-- Attachment #2: usbs_at91sam7s.c --]
[-- Type: text/plain, Size: 40984 bytes --]

#include <pkgconf/devs_usb_at91sam7s.h>
#include <cyg/io/usb/usbs_at91sam7s.h>
#include <cyg/hal/AT91SAM7S_REG.h>

#include <cyg/hal/drv_api.h>
#include <cyg/hal/hal_arch.h>
#include <cyg/hal/hal_io.h>
#include <cyg/hal/hal_cache.h>
#include <cyg/hal/hal_platform_ints.h>

#include <cyg/infra/cyg_type.h>
#include <cyg/infra/cyg_ass.h>
#include <cyg/infra/cyg_trac.h>
#include <cyg/infra/diag.h>

#include <cyg/error/codes.h>

#include <cyg/io/usb/usb.h>
#include <cyg/io/usb/usbs.h>

// For memcpy()
#include <string.h>
#include <stdio.h>



#define AT91_UDP_CSR0 (AT91_UDP_CSR)
#define AT91_UDP_FDR0 (AT91_UDP_FDR)

#define pIER (AT91C_BASE_UDP + AT91_UDP_IER)
#define pIDR (AT91C_BASE_UDP + AT91_UDP_IDR)
#define pISR (AT91C_BASE_UDP + AT91_UDP_ISR)
#define pIMR (AT91C_BASE_UDP + AT91_UDP_IMR)
#define pICR (AT91C_BASE_UDP + AT91_UDP_ICR)

#define pCSR0 (AT91C_BASE_UDP + AT91_UDP_CSR0)
#define pFDR0 (AT91C_BASE_UDP + AT91_UDP_FDR0)

#define AT91C_UDP_ALLOWED_IRQs (AT91C_UDP_WAKEUP | AT91C_UDP_ENDBUSRES | AT91C_UDP_EXTRSM | AT91C_UDP_RXRSM | AT91C_UDP_RXSUSP | AT91C_UDP_EPINT0  | AT91C_UDP_EPINT1 | AT91C_UDP_EPINT2 | AT91C_UDP_EPINT3)

#define THERE_IS_A_NEW_PACKET_IN_THE_UDP 0xffff

static const cyg_uint16 usbs_sam7_endpoint_fifo_size[AT91SAM7S_USB_ENDPOINTS] = {
    8,
    64,
    64,
    8
};
static const bool usbs_sam7_endpoint_pingpong[AT91SAM7S_USB_ENDPOINTS] = {
    false,
    true,
    true,
    false
};
static cyg_uint8 *usbs_sam7_endpoint_pbegin[AT91SAM7S_USB_ENDPOINTS] = {0};
static cyg_uint8 *usbs_sam7_endpoint_pend[AT91SAM7S_USB_ENDPOINTS] = {0};
static bool      usbs_sam7_endpoint_bank1[AT91SAM7S_USB_ENDPOINTS] = {false};
//static bool      usbs_sam7_endpoint_send_zero_packet[AT91SAM7S_USB_ENDPOINTS] = {false};
static bool      usbs_sam7_endpoint_transfer[AT91SAM7S_USB_ENDPOINTS] = {false};
static cyg_uint16 usbs_sam7_endpoint_bytes_in_fifo[AT91SAM7S_USB_ENDPOINTS] = {0};
static cyg_uint16 usbs_sam7_endpoint_bytes_recived[AT91SAM7S_USB_ENDPOINTS] = {THERE_IS_A_NEW_PACKET_IN_THE_UDP};



static cyg_interrupt usbs_sam7_intr_data;
static cyg_handle_t  usbs_sam7_intr_handle;

static void usbs_sam7_ep0_start(usbs_control_endpoint*);
static void usbs_sam7_poll(usbs_control_endpoint*);

static void usbs_sam7_endpoint_start(cyg_uint8 epn);
static void usbs_sam7_set_halted(cyg_bool new_value, cyg_uint8 epn);
static void usbs_sam7_endpoint_init(cyg_uint8 epn, cyg_uint8 endpoint_type, cyg_bool enable);


void usbs_start_dummy(usbs_rx_endpoint* ep){
    
    CYG_ASSERT(0, "usbs_start_dummy()");
}

void set_halted_dummy(usbs_rx_endpoint* ep, cyg_bool b){
    
    CYG_ASSERT(0, "set_halted_dummy()");
}


void usbs_sam7_ep1_start(usbs_rx_endpoint *pep){
    usbs_sam7_endpoint_start(1);
}

void usbs_sam7_ep2_start(usbs_rx_endpoint *pep){
    usbs_sam7_endpoint_start(2);
}

void usbs_sam7_ep3_start(usbs_rx_endpoint *pep){
    usbs_sam7_endpoint_start(3);
}



void usbs_sam7_ep1_set_halted(usbs_rx_endpoint *pep, cyg_bool new_value){
    usbs_sam7_set_halted(new_value, 1);
}

void usbs_sam7_ep2_set_halted(usbs_rx_endpoint *pep, cyg_bool new_value){
    usbs_sam7_set_halted(new_value, 2);
}

void usbs_sam7_ep3_set_halted(usbs_rx_endpoint *pep, cyg_bool new_value){
    usbs_sam7_set_halted(new_value, 3);
}



void usbs_sam7_ep1_init(usbs_rx_endpoint *pep, cyg_uint8 endpoint_type, cyg_bool enable){
    usbs_sam7_endpoint_init(1, endpoint_type, enable);
}

void usbs_sam7_ep2_init(usbs_rx_endpoint *pep, cyg_uint8 endpoint_type, cyg_bool enable){
    usbs_sam7_endpoint_init(2, endpoint_type, enable);
}

void usbs_sam7_ep3_init(usbs_rx_endpoint *pep, cyg_uint8 endpoint_type, cyg_bool enable){
    usbs_sam7_endpoint_init(3, endpoint_type, enable);
}


usbs_control_endpoint usbs_sam7_ep0 = {
    state:                  USBS_STATE_POWERED, // The hardware does not distinguish  between detached, attached and powered.
    enumeration_data:       (usbs_enumeration_data*) 0,
    start_fn:               usbs_sam7_ep0_start,
    poll_fn:                usbs_sam7_poll,
    interrupt_vector:       CYGNUM_HAL_INTERRUPT_UDP,
    control_buffer:         { 0, 0, 0, 0, 0, 0, 0, 0 },
    state_change_fn:        (void (*)(usbs_control_endpoint*, void*, usbs_state_change, int)) 0,
    state_change_data:      (void*) 0,
    standard_control_fn:    (usbs_control_return (*)(usbs_control_endpoint*, void*)) 0,
    standard_control_data:  (void*) 0,
    class_control_fn:       (usbs_control_return (*)(usbs_control_endpoint*, void*)) 0,
    class_control_data:     (void*) 0,
    vendor_control_fn:      (usbs_control_return (*)(usbs_control_endpoint*, void*)) 0,
    vendor_control_data:    (void*) 0,
    reserved_control_fn:    (usbs_control_return (*)(usbs_control_endpoint*, void*)) 0,
    reserved_control_data:  (void*) 0,
    buffer:                 (unsigned char*) 0,
    buffer_size:            0,
    fill_buffer_fn:         (void (*)(usbs_control_endpoint*)) 0,
    fill_data:              (void*) 0,
    fill_index:             0,
    complete_fn:            (usbs_control_return (*)(usbs_control_endpoint*, int)) 0
};


usbs_rx_endpoint usbs_sam7_ep1 = {
    start_rx_fn:        usbs_sam7_ep1_start,
    set_halted_fn:      usbs_sam7_ep1_set_halted,
    complete_fn:        (void (*)(void*, int)) 0,
    complete_data:      (void*) 0,
    buffer:             (const unsigned char*) 0,
    buffer_size:        0,
    halted:             0,
};


usbs_rx_endpoint usbs_sam7_ep2 = {
    start_rx_fn:        usbs_sam7_ep2_start,
    set_halted_fn:      usbs_sam7_ep2_set_halted,
    complete_fn:        (void (*)(void*, int)) 0,
    complete_data:      (void*) 0,
    buffer:             (const unsigned char*) 0,
    buffer_size:        0,
    halted:             0,
};


usbs_rx_endpoint usbs_sam7_ep3 = {
    start_rx_fn:        usbs_sam7_ep3_start,
    set_halted_fn:      usbs_sam7_ep3_set_halted,
    complete_fn:        (void (*)(void*, int)) 0,
    complete_data:      (void*) 0,
    buffer:             (const unsigned char*) 0,
    buffer_size:        0,
    halted:             0,
};




const void *usbs_sam7_enpoints[AT91SAM7S_USB_ENDPOINTS] = {
    
    (void *) &usbs_sam7_ep0,
    (void *) &usbs_sam7_ep1,
    (void *) &usbs_sam7_ep2,
    (void *) &usbs_sam7_ep3
};








typedef enum ep0_low_level_status_t {
    
    EP0_LL_IDLE = 0,
    EP0_LL_REQUEST,
    EP0_LL_SEND_READY,
    EP0_LL_ACK,
    EP0_LL_RECIVE_READY,
    EP0_LL_ISOERROR,
    EP0_LL_STALL,
    EP0_LL_SET_ADDRESS,
} ep0_low_level_status_t;

#ifndef _BV
    #define _BV(_bit_) (1 << _bit_)
#endif

#ifndef MIN
    #define MIN(a,b) (((a) < (b)) ? (a) : (b))
#endif





// DEBUG CHÄS
#define MAXPRINTBUFSIZE 3000
cyg_uint32 printbuf_size = MAXPRINTBUFSIZE;
char printbuf[MAXPRINTBUFSIZE];
cyg_uint32 printbuf_next = 0;



#define debug_line_and_funcion debug_printf("%d: %s(); ", __LINE__, __FUNCTION__);


void buf_printf(char *fmt, ...){
    
    va_list argp;
    va_start(argp, fmt);
    printbuf_next = printbuf_next + vsnprintf((char *)((cyg_uint32) printbuf + printbuf_next), printbuf_size - printbuf_next, fmt, argp);
//    diag_vprintf(fmt, argp);
    va_end(argp);
}

#define debug_printf buf_printf



void debug_dump(cyg_uint8 *pbegin, cyg_uint8 *pend){
    
    //debug_line_and_funcion
    
    debug_printf("[%d] ", (cyg_uint32) pend - (cyg_uint32) pbegin);
    
    do {
        debug_printf("%02x ", *pbegin);
        pbegin++;
    }
    while (pbegin < pend);
    
    debug_printf("\n");
}

//#define diag_ //

#if 1
    #define dbg debug_line_and_funcion 
#else
    #define dbg // 
#endif

#if 1
    #define dbgdsr debug_line_and_funcion 
#else
    #define dbgdsr 
#endif

#define dbgf 



void diag_status(usbs_control_endpoint cep, ep0_low_level_status_t s){
    
    cyg_uint32 isr;
    cyg_uint32 csr0;
    
    HAL_READ_UINT32(pISR, isr);
    HAL_READ_UINT32(pCSR0, csr0);
    
    debug_printf("i%08x; c%08x; state: ", isr, csr0);
    switch(cep.state){
        case USBS_STATE_DETACHED:  debug_printf("USBS_STATE_DETACHED"); break;
        case USBS_STATE_ATTACHED:  debug_printf("USBS_STATE_ATTACHED"); break;
        case USBS_STATE_POWERED:   debug_printf("USBS_STATE_POWERED"); break;
        case USBS_STATE_DEFAULT:   debug_printf("USBS_STATE_DEFAULT"); break;
        case USBS_STATE_ADDRESSED: debug_printf("USBS_STATE_ADDRESSED"); break;
        case USBS_STATE_CONFIGURED:debug_printf("USBS_STATE_CONFIGURED"); break;
        case USBS_STATE_MASK:      debug_printf("USBS_STATE_MASK"); break;
        case USBS_STATE_SUSPENDED: debug_printf("USBS_STATE_SUSPENDED"); break;
        default:                   debug_printf("USBS_STATE_ERROR"); break;
    }
    debug_printf("; ll: ");
	switch(s){
    	case EP0_LL_IDLE:         debug_printf("EP0_LL_IDLE"); break;
		case EP0_LL_REQUEST:      debug_printf("EP0_LL_REQUEST"); break;
		case EP0_LL_SEND_READY:   debug_printf("EP0_LL_SEND_READY"); break;
		case EP0_LL_ACK:          debug_printf("EP0_LL_ACK"); break;
		case EP0_LL_RECIVE_READY: debug_printf("EP0_LL_RECIVE_READY"); break;
        case EP0_LL_ISOERROR:     debug_printf("EP0_LL_ISOERROR"); break;
        case EP0_LL_STALL:        debug_printf("EP0_LL_STALL"); break;
		case EP0_LL_SET_ADDRESS:  debug_printf("EP0_LL_SET_ADDRESS"); break;
		default:                  debug_printf("EP0_LL_ERROR"); break;
	}
    debug_printf(";\n");
}


















void usbs_sam7_endpoint_interrupt_enable(cyg_uint8 epn, bool enable){
    
    CYG_ASSERT(epn < 4, "");
    
    if (enable){
        HAL_WRITE_UINT32(pIER, 1 << epn);
    }else{
        HAL_WRITE_UINT32(pIDR, 1 << epn);
    }
}


inline cyg_uint32 hal_get_2_bytes(cyg_addrword_t addr){
	
	cyg_uint32 	temp1, temp2;
		
	HAL_READ_UINT8(addr, temp1);
	HAL_READ_UINT8(addr, temp2);
	
	return temp1 | (temp2 << 8);
}


inline cyg_uint8 hal_get_byte(cyg_addrword_t addr){
	
	cyg_uint8 	temp1;
		
	HAL_READ_UINT32(addr, temp1);
	
	return temp1;
}


inline cyg_uint32 hal_get_uint32(cyg_addrword_t addr){
	
	cyg_uint32 	temp1;
		
	HAL_READ_UINT32(addr, temp1);
	
	return temp1;
}



cyg_uint8 *hal_read_fifo_uint8(cyg_uint8 *pdest, cyg_uint8 *psource, cyg_uint32 size){
    
    cyg_uint8 *preqbyte = pdest;
    cyg_uint8 reqbyte;

    while (preqbyte < (cyg_uint8 *)((cyg_uint32) size + pdest)){
        
        HAL_READ_UINT8(psource, reqbyte);
        *preqbyte = reqbyte;
        preqbyte++;
    }
    
    return preqbyte;
}



cyg_uint8 *hal_write_fifo_uint8(cyg_uint8 *pdest, cyg_uint8 *psource, cyg_uint8 *psource_end){
    
    cyg_uint8 *preqbyte;
    
    for (preqbyte = psource; preqbyte < psource_end; preqbyte++){
        
        HAL_WRITE_UINT8(pdest, (*preqbyte));
    }
    
    return preqbyte;
}


void set_bits(cyg_addrword_t addr, cyg_uint32 set){
    
    cyg_uint32 read;

    HAL_READ_UINT32(addr, read);
    HAL_WRITE_UINT32(addr, read | set);
}


void clear_bits(cyg_addrword_t addr, cyg_uint32 clear){
    
    cyg_uint32 read;

    HAL_READ_UINT32(addr, read);
    HAL_WRITE_UINT32(addr, read & ~clear);
}


cyg_uint32 bits_are_set(cyg_addrword_t addr, cyg_uint32 set){
    
    cyg_uint32 read;

    HAL_READ_UINT32(addr, read);
    
    return (read & set) == set;
}


cyg_uint32 bits_are_cleared(cyg_addrword_t addr, cyg_uint32 clear){
    
    cyg_uint32 read;

    HAL_READ_UINT32(addr, read);
    
    return (read | ~clear) == ~clear;
}




void usbs_change_state(usbs_control_endpoint *pcep, usbs_state_change new_state){
    
    int old_state = pcep->state;
    
    pcep->state = new_state;
    if (pcep->state_change_fn){
        (*pcep->state_change_fn)(pcep, 0, new_state, old_state);
    }
}


void usbs_end_all_transfers(usbs_control_return returncode){
    
    cyg_uint32 epn;
    usbs_rx_endpoint *pep;

    for (epn = 1; epn < AT91SAM7S_USB_ENDPOINTS; epn++){
        
        if (usbs_sam7_endpoint_transfer[epn]){ /* Ready to transmit ? */
    
            pep = (usbs_rx_endpoint *) usbs_sam7_enpoints[epn];
            
            if (pep->complete_fn){
                (*pep->complete_fn)(pep->complete_data, returncode);
            }
            
            usbs_sam7_endpoint_interrupt_enable(epn, false);
            usbs_sam7_endpoint_transfer[epn] = false;
        }
    }    
}


void usbs_state_notify(usbs_control_endpoint *pcep){
    
    static int old_state = USBS_STATE_CHANGE_POWERED;
    int state = pcep->state & USBS_STATE_MASK;
    
    if (pcep->state != old_state){
        
diag_printf(" <state: %x> ", pcep->state);
        
        usbs_end_all_transfers(-EPIPE);

        if (pcep->state & USBS_STATE_MASK == USBS_STATE_DETACHED){
        } else if (state == USBS_STATE_DETACHED){
        } else if (state == USBS_STATE_ATTACHED){
        } else if (state == USBS_STATE_POWERED){
        } else if (state == USBS_STATE_DEFAULT){
            
            HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_GLBSTATE, 0);
            
        } else if (state == USBS_STATE_ADDRESSED){

            HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_GLBSTATE, AT91C_UDP_FADDEN);
            
        } else if (state == USBS_STATE_CONFIGURED){

            HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_GLBSTATE, AT91C_UDP_CONFG);
            
        } else {
            /* ERROR */
        }
        
        if (pcep->state & USBS_STATE_SUSPENDED){
        } else {
        }
        
        if (pcep->state_change_fn){
            (*pcep->state_change_fn)(pcep, 0, pcep->state, old_state);
        }
        
        old_state = pcep->state;
    }
}


usbs_control_return usbs_parse_host_get_command(usbs_control_endpoint *pcep){
    
    usbs_control_return retcode;
    cyg_uint8 dev_req_type = (((usb_devreq *) pcep->control_buffer)->type) & USB_DEVREQ_TYPE_MASK;
    
    pcep->buffer_size = 0;
    pcep->fill_buffer_fn = 0;
    
    if (dev_req_type == USB_DEVREQ_TYPE_STANDARD){
        
        if (!pcep->standard_control_fn){
            return usbs_handle_standard_control(pcep);
        }
        retcode = (*pcep->standard_control_fn)(pcep, pcep->standard_control_data);
        if (retcode == USBS_CONTROL_RETURN_UNKNOWN){
            
            return usbs_handle_standard_control(pcep);
        }
        return retcode;
    }
    else if (dev_req_type == USB_DEVREQ_TYPE_CLASS){
        
        if (!pcep->class_control_fn){
            return USBS_CONTROL_RETURN_STALL;
        }
        return (*pcep->class_control_fn)(pcep, pcep->class_control_data);
    }
    else if (dev_req_type == USB_DEVREQ_TYPE_VENDOR){
        if (!pcep->class_control_fn){
            return USBS_CONTROL_RETURN_STALL;
        }
        return (*pcep->class_control_fn)(pcep, pcep->vendor_control_data);
    }
    else if (dev_req_type == USB_DEVREQ_TYPE_RESERVED){
        if (!pcep->reserved_control_fn){
            return USBS_CONTROL_RETURN_STALL;
        }
        return (*pcep->reserved_control_fn)(pcep, pcep->reserved_control_data);
    }
    
    return USBS_CONTROL_RETURN_STALL;
}













static void usbs_sam7_set_halted(cyg_bool new_value, cyg_uint8 epn){
    
    usbs_rx_endpoint *pep = (usbs_rx_endpoint *) usbs_sam7_enpoints[epn];
    cyg_uint32 *pbegin = usbs_sam7_endpoint_pbegin[epn];
    cyg_uint32 *pend = usbs_sam7_endpoint_pend[epn];

    if (pep->halted != new_value){ /* If somting is to do */
        
//        cyg_drv_interrupt_mask(CYGNUM_HAL_INTERRUPT_UDP);
        
        pep->halted = new_value;
        
        pbegin = pep->buffer;
        pend = pbegin;
        
        if (new_value && usbs_sam7_endpoint_transfer[epn]){
            if (pep->complete_fn && usbs_sam7_endpoint_transfer[epn]){
                (*pep->complete_fn)(pep->complete_data, -EAGAIN);
            }
            usbs_sam7_endpoint_transfer[epn] = false;
        }
        
//        cyg_drv_interrupt_unmask(CYGNUM_HAL_INTERRUPT_UDP);
    }
}





static void usbs_sam7_endpoint_init(cyg_uint8 epn, cyg_uint8 endpoint_type, cyg_bool enable){
    
    usbs_rx_endpoint *pep = (usbs_rx_endpoint *) usbs_sam7_enpoints[epn];
    cyg_uint32 *pCSR = (cyg_uint32 *)((cyg_uint32) pCSR0 + 4 * epn);
//    cyg_uint32 *pFDR = (cyg_uint32 *)((cyg_uint32) pFDR0 + 4 * epn);
//    cyg_uint32 endpoint_size = usbs_sam7_endpoint_fifo_size[epn];
//    cyg_uint32 *pbegin = usbs_sam7_endpoint_pbegin[epn];
//    cyg_uint32 *pend = usbs_sam7_endpoint_pend[epn];
    
    CYG_ASSERT(AT91SAM7S_USB_ENDPOINTS > epn && epn, "Wrong epn");
 
    usbs_sam7_endpoint_interrupt_enable(epn, false);
    
    HAL_WRITE_UINT32(pCSR, ((((cyg_uint32) endpoint_type) & 0x03) << 8) | ((((cyg_uint32) endpoint_type) & 0x80) << 3)); /* Type | In */
//    CYG_ASSERT(bits_are_set(pCSR, 1 << 9), "Wrong enpoint type");

    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_RSTEP, 1 << epn); /* Reset endpoint */
    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_RSTEP, 0);

    pep->halted = false;
    usbs_sam7_endpoint_transfer[epn] = false;
    usbs_sam7_endpoint_bytes_in_fifo[epn] = 0;
    usbs_sam7_endpoint_bytes_recived[epn] = THERE_IS_A_NEW_PACKET_IN_THE_UDP;
    usbs_sam7_endpoint_bank1[epn] = false;
    usbs_sam7_endpoint_transfer[epn] = false;
    
    if (enable){
        set_bits(pCSR, AT91C_UDP_EPEDS);
    }
}


static void usbs_sam7_handle_reset(void){
    
    dbg debug_printf("\n");
      
    usbs_end_all_transfers(-EPIPE);

    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_IDR,     0xffffffff);
    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_ICR,     0xffffffff);
    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_RSTEP,   0xffffffff);
    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_RSTEP,   0x00000000);
    
    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_FADDR,   AT91C_UDP_FEN);
    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_CSR0,    AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_CTRL);
    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_IER,     AT91C_UDP_ALLOWED_IRQs);

#if 1       
    usbs_sam7_endpoint_init(1, USB_ENDPOINT_DESCRIPTOR_ATTR_BULK, true);
    usbs_sam7_endpoint_init(2, USB_ENDPOINT_DESCRIPTOR_ATTR_BULK | USB_ENDPOINT_DESCRIPTOR_ENDPOINT_IN, true);
    usbs_sam7_endpoint_init(3, USB_ENDPOINT_DESCRIPTOR_ATTR_INTERRUPT | USB_ENDPOINT_DESCRIPTOR_ENDPOINT_IN, true);
#else
    usbs_sam7_endpoint_init(1, 0, false);
    usbs_sam7_endpoint_init(2, 0, false);
    usbs_sam7_endpoint_init(3, 0, false);
#endif
}






static void usbs_sam7_ep0_start(usbs_control_endpoint* endpoint){
    
    dbg debug_printf("\n");
    
    usbs_sam7_handle_reset();
   
    // If there is additional platform-specific initialization to
    // perform, do it now. This macro can come from the platform HAL,
    // but may not be available on all platforms.
#ifdef AT91SAM7S_USB_PLATFORM_INIT
    AT91SAM7S_USB_PLATFORM_INIT();
#else
    HAL_WRITE_UINT32(AT91C_BASE_PIOA + AT91_PIO_CODR, AT91C_PIO_PA4);
#endif
}
 


static void usbs_sam7_endpoint_start(cyg_uint8 epn){
    
    usbs_rx_endpoint *pep = (usbs_rx_endpoint *) usbs_sam7_enpoints[epn];
    cyg_uint32 *pCSR = (cyg_uint32 *)((cyg_uint32) pCSR0 + 4 * epn);
    cyg_uint32 *pFDR = (cyg_uint32 *)((cyg_uint32) pFDR0 + 4 * epn);
    cyg_uint16 space = 0;
    cyg_uint16 endpoint_size = usbs_sam7_endpoint_fifo_size[epn];
    cyg_uint16 *pinfifo = &usbs_sam7_endpoint_bytes_in_fifo[epn];
    cyg_uint8  **ppbegin = &usbs_sam7_endpoint_pbegin[epn];
    cyg_uint8  **ppend = &usbs_sam7_endpoint_pend[epn];
    
    CYG_ASSERT(AT91SAM7S_USB_ENDPOINTS > epn && epn, "Wrong epn");
    CYG_ASSERT(bits_are_set(pCSR, 1 << 9), "Wrong enpoint type");
//    CYG_ASSERT(pep->complete_fn, "No complete_fn()");

    if (usbs_sam7_ep0.state != USBS_STATE_CONFIGURED){ /* Halted means nothing to do */
diag_printf("*1");        

        if (pep->complete_fn){
            (*pep->complete_fn)(pep->complete_data, -EPIPE);
        }
        
        return;
    }
    
    if (pep->halted){ /* Halted means nothing to do */
diag_printf("*2");        
        
        if (pep->complete_fn){
            (*pep->complete_fn)(pep->complete_data, -EAGAIN);
        }
        
        return;
    }
    
    if (usbs_sam7_endpoint_transfer[epn]){ /* Transfer in progress */
diag_printf("*3");        
        
        if (pep->complete_fn){
            (*pep->complete_fn)(pep->complete_data, -EIO);
        }
        
        return;
    }
    
    *ppbegin = pep->buffer; /* Set the working pointers */
    *ppend = (cyg_uint32 *)((cyg_uint32) pep->buffer + pep->buffer_size);
    
    if (bits_are_set(pCSR, 0x400)){ /* IN: tx_endpoint */
        
        space = (cyg_uint32) *ppend - (cyg_uint32) *ppbegin;
        if (space == endpoint_size){
            *ppend = *ppbegin; /* Send zero-packet */
        }
        
        *ppbegin = hal_write_fifo_uint8(pFDR, *ppbegin, (cyg_uint32 *)((cyg_uint32) *ppbegin + MIN(space, endpoint_size)));
        set_bits(pCSR, AT91C_UDP_TXPKTRDY);

        if (*ppend == *ppbegin){ /* Last packet ? */
            *ppend = *ppbegin - 1; /* The packet isn't sent yet */
        }
    }

    usbs_sam7_endpoint_transfer[epn] = true;
    usbs_sam7_endpoint_interrupt_enable(epn, true);
}



static bool usbs_sam7_endpoint_isr(cyg_uint8 epn){ /* Move the datas in ISR for high bandwidth */
    
    cyg_uint32      *pCSR = (cyg_uint32 *)((cyg_uint32) pCSR0 + 4 * epn);
    cyg_uint32      *pFDR = (cyg_uint32 *)((cyg_uint32) pFDR0 + 4 * epn);
    cyg_uint16      endpoint_size = usbs_sam7_endpoint_fifo_size[epn];
    cyg_uint32      space = 0;
    cyg_uint8       **ppbegin = &usbs_sam7_endpoint_pbegin[epn];
    cyg_uint8       **ppend = &usbs_sam7_endpoint_pend[epn];
    cyg_uint16      *pinfifo = &usbs_sam7_endpoint_bytes_in_fifo[epn];
    cyg_uint16      *precived = &usbs_sam7_endpoint_bytes_recived[epn];
    
    CYG_ASSERT(AT91SAM7S_USB_ENDPOINTS > epn && epn, "Wrong epn");
    
//diag_printf(" i%d ", epn);

    if (bits_are_set(pCSR, 0x400)){ /* IN: tx_endpoint */

        clear_bits(pCSR, AT91C_UDP_TXCOMP);

        if (bits_are_cleared(pCSR, AT91C_UDP_TXPKTRDY)){ /* Ready to transmit ? */
            
            if (*ppend > *ppbegin){ /* Somthing to send ? */

                space = (cyg_uint32) *ppend - (cyg_uint32) *ppbegin;
                if (space == endpoint_size){
                    *ppend = *ppbegin; /* Send zero-packet */
                }
                
                *ppbegin = hal_write_fifo_uint8(pFDR, *ppbegin, (cyg_uint32 *)((cyg_uint32) *ppbegin + MIN(space, endpoint_size)));
                set_bits(pCSR, AT91C_UDP_TXPKTRDY);
                
                if (*ppend == *ppbegin){ /* Last packet ? */
                    *ppend = *ppbegin - 1; /* The packet isn't sent yet */
                }
                
            } else {
                
                if (*ppend + 1 == *ppbegin){
                    
                    *ppend = *ppbegin; /* Flag for DSR */
                    
                    return true;
                    
                } else {

                    *ppend = *ppbegin - 1; /* Flag for zero-packet */
                    set_bits(pCSR, AT91C_UDP_TXPKTRDY); /* Send no data */
                }
            }
        }

        clear_bits(pCSR, AT91C_UDP_RX_DATA_BK0 | AT91C_UDP_RX_DATA_BK1 | AT91C_UDP_RXSETUP | AT91C_UDP_ISOERROR);
        
    } else { /* OUT: rx_endpoint */
  
        if (!bits_are_cleared(pCSR, AT91C_UDP_RX_DATA_BK0) || AT91C_UDP_RX_DATA_BK1){ /* Sometime something recived ? */
            
            if (*precived == THERE_IS_A_NEW_PACKET_IN_THE_UDP){ /* If there is a new... */
                
                *precived = ((*pCSR) >> 16) & 0x7ff; /* How many bytes ? */
                *pinfifo = *precived; /* FIFO hold datas */
            }
            
            while ((*ppbegin < *ppend) && *pinfifo){ /* If we have buffer-space AND datas in the FIFO */ 
                
                **ppbegin = *pFDR;
                
                (*ppbegin)++;
                (*pinfifo)--;
            }
            
            if (*ppbegin == *ppend){ /* The buffer is full... call the DSR */
    
                return true; /* We can call the complet-function in the DSR */
                    
            }
            
            if (*pinfifo == 0){ /* If the FIFO is empty, then we can release it */
        
                if (usbs_sam7_endpoint_pingpong[epn]){ /* Time to clear the interrupt flag */
                    
                    if (usbs_sam7_endpoint_bank1[epn]){
                        clear_bits(pCSR, AT91C_UDP_RX_DATA_BK1);
                    } else {
                        clear_bits(pCSR, AT91C_UDP_RX_DATA_BK0);
                    }
                    usbs_sam7_endpoint_bank1[epn] = !usbs_sam7_endpoint_bank1[epn];
                    
                } else {
                    clear_bits(pCSR, AT91C_UDP_RX_DATA_BK0);
                } 
                
                if (*precived < endpoint_size){ /* If the last packet was smaller then the endpoint-size... */
                    
                    *ppend = *ppbegin;
                    *precived = THERE_IS_A_NEW_PACKET_IN_THE_UDP; /* Set flag */
    
                    return true; /* We can call the complet-function in the DSR */
                    
                }
                
                *precived = THERE_IS_A_NEW_PACKET_IN_THE_UDP; /* Set flag */
            }
        }
        
        clear_bits(pCSR, AT91C_UDP_TXCOMP | AT91C_UDP_RXSETUP | AT91C_UDP_ISOERROR);
    }
    
    return false;
}


static void usbs_sam7_endpoint_dsr(cyg_uint8 epn){
    
    usbs_rx_endpoint *pep = (usbs_rx_endpoint *) usbs_sam7_enpoints[epn];
    cyg_uint32      *pCSR = (cyg_uint32 *)((cyg_uint32) pCSR0 + 4 * epn);
    cyg_uint8       **ppbegin = &usbs_sam7_endpoint_pbegin[epn];
    cyg_uint8       **ppend = &usbs_sam7_endpoint_pend[epn];
    
    CYG_ASSERT(AT91SAM7S_USB_ENDPOINTS > epn && epn, "Wrong epn");
    CYG_ASSERT(bits_are_set(pCSR, 1 << 9), "Wrong enpoint type");
    CYG_ASSERT(pep->complete_fn, "No complete_fn()");
    
//diag_printf(" d%d ", epn);

    if (usbs_sam7_endpoint_transfer[epn]){ /* Transmiting ? */

        if (*ppend == *ppbegin){ /* Transmitted ? */
            
            pep->buffer_size = (cyg_uint32) *ppbegin - (cyg_uint32) pep->buffer;
            
            if (pep->complete_fn){
                if (!pep->halted){
                    (*pep->complete_fn)(pep->complete_data, pep->buffer_size);
                } else {
                    (*pep->complete_fn)(pep->complete_data, -EAGAIN);
                }
            }
            
            usbs_sam7_endpoint_interrupt_enable(epn, false);
            usbs_sam7_endpoint_transfer[epn] = false;
        }
    }
}






static void usbs_sam7_control_dsr(){
    
    usb_devreq *req = usbs_sam7_ep0.control_buffer; /*  */
    static ep0_low_level_status_t status = EP0_LL_IDLE;
    bool   dev_to_host; /* IN */
    usbs_control_return  usbcode;
    
    static cyg_uint16   length;
        
    static cyg_uint8    *pbuffer = 0;
    static cyg_uint8    *pbuffer_end = 0;
    static cyg_uint32   bytes_to_write = 0;

    static cyg_uint32   address;

    static cyg_uint8    configaddress ;
    
    //dbgdsr debug_printf("BEGIN: "); diag_status(usbs_sam7_ep0, status);
    //diag_printf("o");
    
    while (!bits_are_cleared(pCSR0, AT91C_UDP_TXCOMP | AT91C_UDP_RX_DATA_BK0 | AT91C_UDP_RXSETUP | AT91C_UDP_ISOERROR | AT91C_UDP_RX_DATA_BK1)){
    
        /* ERROR-HANDLING */
        /******************/

        if (bits_are_set(pCSR0, AT91C_UDP_ISOERROR)){ /* ERROR? */
            
//            clear_bits(pCSR0, AT91C_UDP_FORCESTALL); /* We recived the host-request */
            clear_bits(pCSR0, AT91C_UDP_ISOERROR | AT91C_UDP_FORCESTALL); /* We recived the host-request */
            
            pbuffer = usbs_sam7_ep0.buffer;
            pbuffer_end = pbuffer;
            
            if (status == EP0_LL_IDLE){
                
                if (usbs_sam7_ep0.complete_fn){
                    (*usbs_sam7_ep0.complete_fn)(&usbs_sam7_ep0, USBS_CONTROL_RETURN_STALL);
                }
            }

            status = EP0_LL_IDLE;
            
            //dbgdsr debug_printf("AT91C_UDP_ISOERROR "); diag_status(usbs_sam7_ep0, status);
        }

        /* HOSTREQUEST-PARSING */
        /***********************/

        if (bits_are_set(pCSR0, AT91C_UDP_RXSETUP)){ /* wenn wir auf ein request warten */
            
            /* host request */
            
            hal_read_fifo_uint8(req, pFDR0, sizeof(usb_devreq));
    
            length = (req->length_hi << 8) | req->length_lo;
            //value  = req->value_hi;    //value  = (req->value_hi << 8) | req->value_lo;
            usbs_sam7_ep0.buffer_size = 0;
            
            status = EP0_LL_REQUEST;
            
            dev_to_host = req->type & USB_DEVREQ_DIRECTION_IN;
            if (dev_to_host){ /* if next transfer is IN: "host <- dev" ? */
                set_bits(pCSR0, AT91C_UDP_DIR); /* Set IN direction */
            } else {
                clear_bits(pCSR0, AT91C_UDP_DIR); /* Set OUT direction */
            }

            if (req->request == USB_DEVREQ_GET_STATUS && dev_to_host){
                
                //diag_printf("s");

                pbuffer = &usbs_sam7_ep0.state;
                pbuffer_end = pbuffer + sizeof(usbs_sam7_ep0.state);
                
                status = EP0_LL_SEND_READY;

            } else if (req->request == USB_DEVREQ_SET_ADDRESS && !dev_to_host){ /* Special-processing */
                
                //diag_printf("a");
                
                pbuffer = usbs_sam7_ep0.buffer;/* Send ACK */
                pbuffer_end = pbuffer;
                
                status = EP0_LL_SEND_READY; 

            } else {
 
                usbcode = usbs_parse_host_get_command(&usbs_sam7_ep0);
                
                usbs_sam7_ep0.buffer_size = MIN(usbs_sam7_ep0.buffer_size, length);
                length = length - usbs_sam7_ep0.buffer_size;

                pbuffer = usbs_sam7_ep0.buffer;
                pbuffer_end = pbuffer + usbs_sam7_ep0.buffer_size; /* Ready to send... */
    
                if (usbcode == USBS_CONTROL_RETURN_HANDLED){ /* OK */
                    
                    if (dev_to_host){ /* if next transfer is IN: "host <- dev" ? */
                        status = EP0_LL_SEND_READY; 
                    } else {
                        status = EP0_LL_RECIVE_READY; 
                    }
                    
                } else {
                    //diag_printf(" <S0> ");
                    clear_bits(pCSR0, 0x7f);
                    set_bits(pCSR0, AT91C_UDP_FORCESTALL);
                    status = EP0_LL_STALL;
                }
            }
            
            clear_bits(pCSR0, AT91C_UDP_RXSETUP); /* We recived the host-request */
//            while(!bits_are_cleared(pCSR0, AT91C_UDP_RXSETUP)){ /* Test like Atmel USB AN */
//            }
        }
        
        
        
        
        /* INTERRUPT-HANDLING clear flags and do the work and update the state-machine */
        /*******************************************************************************/

        if (bits_are_set(pCSR0, AT91C_UDP_TXCOMP)){  //received an ACK packet
            
            clear_bits(pCSR0, AT91C_UDP_TXCOMP); // Clear corresponding interrupt 
            
//            while(!bits_are_cleared(pCSR0, AT91C_UDP_TXCOMP)){ // Test like Atmel USB AN 
//            }
        }
        
        if (bits_are_cleared(pCSR0, AT91C_UDP_TXPKTRDY)){
            
            if (status == EP0_LL_SEND_READY){
            
                if (pbuffer == pbuffer_end){ // All bytes are sent, send ACK
    
                    status = EP0_LL_ACK;
                    
                    set_bits(pCSR0, AT91C_UDP_TXPKTRDY); // Signal FIFO loaded 
                } else {
                    
                    bytes_to_write = MIN(pbuffer_end - pbuffer, usbs_sam7_endpoint_fifo_size[0]);
                    pbuffer = hal_write_fifo_uint8(pFDR0, pbuffer, (cyg_uint8 *)((cyg_uint32) pbuffer + bytes_to_write)); // Send next few bytes 
                    
                    if (pbuffer == pbuffer_end){ /* Control-Endoints don't need ACK's */
                        
                        if (usbs_sam7_ep0.fill_buffer_fn){ /* More Records ? */
                            
                            //diag_printf(" f ");

                            (*usbs_sam7_ep0.fill_buffer_fn)(&usbs_sam7_ep0);
              
                            pbuffer = usbs_sam7_ep0.buffer;
                            pbuffer_end = pbuffer + usbs_sam7_ep0.buffer_size; /* Ready to send... */
    
                            bytes_to_write = MIN(pbuffer_end - pbuffer, usbs_sam7_endpoint_fifo_size[0] - bytes_to_write);
                            
                            pbuffer = hal_write_fifo_uint8(pFDR0, pbuffer, (cyg_uint8 *)((cyg_uint32) pbuffer + bytes_to_write)); // Send next few bytes 
         
                        } else {
                            
                            status = EP0_LL_IDLE;
                        }
                    }
                    
                    set_bits(pCSR0, AT91C_UDP_TXPKTRDY); // Signal FIFO loaded 
                }
                
            } else if (status == EP0_LL_RECIVE_READY){ /* Maybe we have to send an ACK */
             
                if (pbuffer == pbuffer_end){ // All bytes are recived, send ACK
    
                    status = EP0_LL_ACK;
                    
                    set_bits(pCSR0, AT91C_UDP_TXPKTRDY); // Signal FIFO loaded 
                }
                
            } else if (status == EP0_LL_ACK){

                dbgdsr debug_printf("confirmed ACK "); diag_status(usbs_sam7_ep0, status); 
                
                if (req->request == USB_DEVREQ_SET_ADDRESS){ /* Special-processing */
                    
                    //diag_printf("b");

                    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_FADDR, req->value_lo | AT91C_UDP_FEN);
//                    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_GLBSTATE, 0x00000001);
                    
                    usbs_sam7_ep0.state = USBS_STATE_ADDRESSED;
                    
                }
                
                if (usbs_sam7_ep0.complete_fn){
                    (*usbs_sam7_ep0.complete_fn)(&usbs_sam7_ep0, USBS_CONTROL_RETURN_HANDLED);
                }
                
                status = EP0_LL_IDLE;
                
                usbs_state_notify(&usbs_sam7_ep0);
            }
        }

        if (bits_are_set(pCSR0, AT91C_UDP_RX_DATA_BK0)){

            /* TODO READ FIFO OR SEND ACK */
            
            pbuffer = usbs_sam7_ep0.buffer;
            pbuffer_end = pbuffer; /* Ready to send... */
            
            status = EP0_LL_SEND_READY;

            clear_bits(pCSR0, AT91C_UDP_RX_DATA_BK0);
        }
    }
}



static void usbs_sam7_dsr(cyg_vector_t vector, cyg_ucount32 count, cyg_addrword_t data){
    
    cyg_uint8 n;
    
	CYG_ASSERT(CYGNUM_HAL_INTERRUPT_UDP == vector, "Wrong interrupts");
	CYG_ASSERT(0 == data, "DSR needs no data");
	
    //diag_printf(".");
	
    clear_bits(AT91C_BASE_UDP + AT91_UDP_GLBSTATE, 0x10);

    if (bits_are_set(pISR, AT91C_UDP_WAKEUP)){

        usbs_sam7_ep0.state = USBS_STATE_DEFAULT;
        usbs_state_notify(&usbs_sam7_ep0);
        
        HAL_WRITE_UINT32(pICR, AT91C_UDP_WAKEUP);     		
    }
	if (bits_are_set(pISR, AT91C_UDP_ENDBUSRES)){ // RESET UDP 

        //diag_printf("R");

        usbs_sam7_ep0.state = USBS_STATE_POWERED;
        usbs_state_notify(&usbs_sam7_ep0);
        usbs_sam7_handle_reset();

        HAL_WRITE_UINT32(pCSR0, AT91C_UDP_EPEDS | AT91C_UDP_EPTYPE_CTRL);
        HAL_WRITE_UINT32(pIER, AT91C_UDP_EPINT0);     
        
        usbs_sam7_ep0.state = USBS_STATE_DEFAULT;
        usbs_state_notify(&usbs_sam7_ep0);
        
        HAL_WRITE_UINT32(pICR, AT91C_UDP_ENDBUSRES);     
	}
	if (bits_are_set(pISR, AT91C_UDP_SOFINT)){
        HAL_WRITE_UINT32(pICR, AT91C_UDP_SOFINT);     		
	}
	if (bits_are_set(pISR, AT91C_UDP_EXTRSM)){
        usbs_sam7_ep0.state = usbs_sam7_ep0.state & ~USBS_STATE_SUSPENDED;
        usbs_state_notify(&usbs_sam7_ep0);
        HAL_WRITE_UINT32(pICR, AT91C_UDP_EXTRSM);     		
	}
	if (bits_are_set(pISR, AT91C_UDP_RXRSM)){
        usbs_sam7_ep0.state = usbs_sam7_ep0.state & ~USBS_STATE_SUSPENDED;
        usbs_state_notify(&usbs_sam7_ep0);
        HAL_WRITE_UINT32(pICR, AT91C_UDP_RXRSM);     		
	}
	if (bits_are_set(pISR, AT91C_UDP_RXSUSP)){
        usbs_sam7_ep0.state = usbs_sam7_ep0.state | USBS_STATE_SUSPENDED;
        usbs_state_notify(&usbs_sam7_ep0);
        HAL_WRITE_UINT32(pICR, AT91C_UDP_RXSUSP);     		
	}
	if (bits_are_set(pISR, AT91C_UDP_EPINT0)){
        usbs_sam7_control_dsr();
    }
    for (n = 1; n < AT91SAM7S_USB_ENDPOINTS; n++){
        
        if (usbs_sam7_endpoint_transfer[n]){
            
            usbs_sam7_endpoint_dsr(n);
        }
    }

    cyg_drv_interrupt_unmask(vector);
}



static cyg_uint32 usbs_sam7_isr(cyg_vector_t vector, cyg_addrword_t data){
	
    cyg_uint8 n;
    bool need_dsr = false;
	
    CYG_ASSERT(CYGNUM_HAL_INTERRUPT_UDP == vector, "Wrong interrupts");
    CYG_ASSERT(0 == data, "ISR needs no data");
    
//diag_printf("<<iirq: %x>>\n", *(cyg_uint32 *) pISR & AT91C_UDP_ALLOWED_IRQs);

    for (n = 1; n < AT91SAM7S_USB_ENDPOINTS; n++){ /* Need any data endpoint a data transfer ? */

        if (bits_are_set(pISR, 1 << n)){

            need_dsr = need_dsr || usbs_sam7_endpoint_isr(n); /* Move the datas */
        }
    }
    
    if (bits_are_cleared(pISR, AT91C_UDP_ALLOWED_IRQs & 0xffffff01) && !need_dsr){ /* If we don't need any DSR */
diag_printf("-");	
		cyg_drv_interrupt_acknowledge(vector);
		
		return CYG_ISR_HANDLED;
    }

    cyg_drv_interrupt_mask(vector); /* Call the DSR */
    cyg_drv_interrupt_acknowledge(vector);

diag_printf("+");   
    return CYG_ISR_HANDLED | CYG_ISR_CALL_DSR;
}



// ----------------------------------------------------------------------------
// Polling support. It is not clear that this is going to work particularly
// well since according to the documentation the hardware does not generate
// NAKs automatically - instead the ISR has to set the appropriate bits
// sufficiently quickly to avoid confusing the host.
//
// Calling the isr directly avoids duplicating code, but means that
// cyg_drv_interrupt_acknowledge() will get called when not inside a
// real interrupt handler. This should be harmless.

static void usbs_sam7_poll(usbs_control_endpoint* endpoint){
	
	dbg debug_printf("\n");

    CYG_ASSERT(endpoint == &usbs_sam7_ep0, "Wrong endpoint");
    if (CYG_ISR_CALL_DSR == usbs_sam7_isr(CYGNUM_HAL_INTERRUPT_UDP, 0)) {
        usbs_sam7_dsr(CYGNUM_HAL_INTERRUPT_UDP, 0, 0);
    }
}

// ----------------------------------------------------------------------------
// Initialization
//
// This routine gets called from a prioritized static constructor during
// eCos startup.


void usbs_sam7_init(void){
    
//    return;

    HAL_WRITE_UINT32(AT91C_BASE_PIOA + AT91_PIO_SODR, AT91C_PIO_PA4);
    HAL_WRITE_UINT32(AT91C_BASE_PIOA + AT91_PIO_PER, AT91C_PIO_PA3 | AT91C_PIO_PA3);
    HAL_WRITE_UINT32(AT91C_BASE_PIOA + AT91_PIO_OER, AT91C_PIO_PA4);
    
    usbs_sam7_handle_reset();
    
    cyg_drv_interrupt_create(CYGNUM_HAL_INTERRUPT_UDP,
        6,        // priority
        0,         // data
        &usbs_sam7_isr,
        &usbs_sam7_dsr,
        &usbs_sam7_intr_handle,
        &usbs_sam7_intr_data);
    
    cyg_drv_interrupt_attach(usbs_sam7_intr_handle);    
    cyg_drv_interrupt_unmask(CYGNUM_HAL_INTERRUPT_UDP);
    
    HAL_WRITE_UINT32(AT91C_BASE_UDP + AT91_UDP_TXVC, 0);
    
    usbs_sam7_ep0.state = USBS_STATE_POWERED;
    usbs_state_notify(&usbs_sam7_ep0);
}



[-- Attachment #3: usbs_at91sam7s_data.cxx --]
[-- Type: text/plain, Size: 4292 bytes --]

#include <cyg/infra/diag.h>
#include <cyg/io/devtab.h>
#include <cyg/io/usb/usbs_at91sam7s.h>
#include <pkgconf/devs_usb_at91sam7s.h>

// ----------------------------------------------------------------------------
// Initialization. The goal here is to call usbs_sam7_init()
// early on during system startup, to take care of things like
// registering interrupt handlers etc. which are best done
// during system init.
//
// If the endpoint 0 devtab entry is available then its init()
// function can be used to take care of this. However the devtab
// entries are optional so an alternative mechanism must be
// provided. Unfortunately although it is possible to give
// a C function the constructor attribute, it cannot be given
// an initpri attribute. Instead it is necessary to define a
// dummy C++ class.

extern "C" void usbs_sam7_init(void);

#ifndef CYGVAR_DEVS_USB_AT91SAM7S_EP0_DEVTAB_ENTRY
	#error /* In Moment will ich einen /dev/...-Eintrag */
	
class usbs_sam7_initialization {
	public:
		usbs_sam7_initialization() {
		usbs_sam7_init();
	}
};

static usbs_sam7_initialization usbs_sam7_init_object CYGBLD_ATTRIB_INIT_PRI(CYG_INIT_IO);
#endif



// ----------------------------------------------------------------------------
// The devtab entries. Each of these is optional, many applications
// will want to use the lower-level API rather than go via
// open/read/write/ioctl.

#ifdef CYGVAR_DEVS_USB_AT91SAM7S_EP0_DEVTAB_ENTRY

// For endpoint 0 the only legal operations are get_config() and
// set_config(), and these are provided by the common package.

static bool usbs_sam7_devtab_ep0_init(struct cyg_devtab_entry* tab){
	
    CYG_UNUSED_PARAM(struct cyg_devtab_entry*, tab);
    
    usbs_sam7_init();
    
    return true;
}

CHAR_DEVIO_TABLE(usbs_sam7_ep0_devtab_functions,
        &cyg_devio_cwrite,
        &cyg_devio_cread,
        &cyg_devio_select,
        &usbs_devtab_get_config,
        &usbs_devtab_set_config);
        
CHAR_DEVTAB_ENTRY(usbs_sam7_ep0_devtab_entry,
        CYGDAT_DEVS_USB_AT91SAM7S_DEVTAB_BASENAME "0",
        0,
        &usbs_sam7_ep0_devtab_functions,
        &usbs_sam7_devtab_ep0_init,
        0,
        (void*) &usbs_sam7_ep0);
#else
	#error /* In Moment will ich einen /dev/...-Eintrag */
#endif

// ----------------------------------------------------------------------------
// Common routines for ep1..3


#if defined(CYGVAR_DEVS_USB_AT91SAM7S_EP1_DEVTAB_ENTRY) || defined(CYGVAR_DEVS_USB_AT91SAM7S_EP2_DEVTAB_ENTRY) || defined(CYGVAR_DEVS_USB_AT91SAM7S_EP3_DEVTAB_ENTRY)

static bool usbs_sam7_devtab_dummy_init(struct cyg_devtab_entry* tab){
	
    CYG_UNUSED_PARAM(struct cyg_devtab_entry*, tab);
    
    return true;
}

#endif









CHAR_DEVIO_TABLE(usbs_sam7_ep1_devtab_functions,
        &usbs_devtab_cwrite,
        &usbs_devtab_cread,
        &cyg_devio_select,
        &usbs_devtab_get_config,
        &usbs_devtab_set_config);

CHAR_DEVTAB_ENTRY(usbs_sam7_ep1_devtab_entry,
        CYGDAT_DEVS_USB_AT91SAM7S_DEVTAB_BASENAME "1",
        0,
        &usbs_sam7_ep1_devtab_functions,
        &usbs_sam7_devtab_dummy_init,
        0,
        (void*) &usbs_sam7_ep1);
         
         
                         
CHAR_DEVIO_TABLE(usbs_sam7_ep2_devtab_functions,
        &usbs_devtab_cwrite,
        &usbs_devtab_cread,
        &cyg_devio_select,
        &usbs_devtab_get_config,
        &usbs_devtab_set_config);

CHAR_DEVTAB_ENTRY(usbs_sam7_ep2_devtab_entry,
        CYGDAT_DEVS_USB_AT91SAM7S_DEVTAB_BASENAME "2",
        0,
        &usbs_sam7_ep2_devtab_functions,
        &usbs_sam7_devtab_dummy_init,
        0,
        (void*) &usbs_sam7_ep2);
  
  
                         
CHAR_DEVIO_TABLE(usbs_sam7_ep3_devtab_functions,
        &usbs_devtab_cwrite,
        &usbs_devtab_cread,
        &cyg_devio_select,
        &usbs_devtab_get_config,
        &usbs_devtab_set_config);

CHAR_DEVTAB_ENTRY(usbs_sam7_ep3_devtab_entry,
        CYGDAT_DEVS_USB_AT91SAM7S_DEVTAB_BASENAME "3",
        0,
        &usbs_sam7_ep3_devtab_functions,
        &usbs_sam7_devtab_dummy_init,
        0,
        (void*) &usbs_sam7_ep3);
                         
                         

[-- Attachment #4: usbs_at91sam7s.h --]
[-- Type: text/plain, Size: 745 bytes --]

#ifndef CYGONCE_USBS_AT91SAM7S_H
#define CYGONCE_USBS_AT91SAM7S_H


#include <cyg/io/usb/usbs.h>
#include <pkgconf/devs_usb_at91sam7s.h>

 
 
 
#define AT91SAM7S_USB_ENDPOINTS 4
 
extern usbs_control_endpoint    usbs_sam7_ep0;
extern usbs_rx_endpoint         usbs_sam7_ep1;
extern usbs_rx_endpoint         usbs_sam7_ep2;
extern usbs_rx_endpoint         usbs_sam7_ep3;


extern void usbs_sam7_ep1_init(usbs_rx_endpoint *pep, cyg_uint8 endpoint_type, cyg_bool enable);
extern void usbs_sam7_ep2_init(usbs_rx_endpoint *pep, cyg_uint8 endpoint_type, cyg_bool enable);
extern void usbs_sam7_ep3_init(usbs_rx_endpoint *pep, cyg_uint8 endpoint_type, cyg_bool enable);











#endif /* CYGONCE_USBS_AT91SAM7S_H */


[-- Attachment #5: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss