beagleboard / am335x_pru_package

331 stars 181 forks source link

prussdrv: Remove pthreads dependency #21

Closed fhunleth closed 10 years ago

fhunleth commented 10 years ago

This change removes the prussdrv_start_irqthread helper function and hence removes the only pthreads dependency in the am335x_pru_package. None of the examples use the function, so the removal of pthreads is propagated through the example Makefiles. While pthreads is almost certainly available on the Beaglebones that use this library, here are a few reasons why this change is a good one.

  1. Seeing pthreads on the link line is a immediate warning sign that threading is in use, so it's reasonable to review the library to make sure that it plays well with application code. Since the am335x_pru_package doesn't really use threading internally at all, removing pthreads removes any doubt that a library call will do something unexpected with threads.
  2. The API isn't threadsafe, since shared data is not protected by a mutex. It seems a little scary doing much work in the irqthread without careful thought.
  3. While the irq_thread provides the looping structure to wait for PRU interrupts, it didn't have a way to pass messages to it. I.e, to exit the thread or to change a parameter for how the application handles the interrupts. I would imagine that many PRU use cases need some kind of message passing to their interrupt handling threads since the PRU provides some ongoing processing rather than a one-off. I addressed passing messages to my PRU interrupt handler by using select(2) and a message pipe, but I would have done it different if I were using Qt or STL. I think threading is better addressed by the application.
  4. Removing the pthreads dependency lets the library be used with toolchains that don't include pthreads. As far as I know, this is only needed for custom uclibc builds, but it is conceivable that someone may build a very small Linux system that uses the PRU app_loader.

I'm curious if anyone actually uses irqthread, and if they do, whether it would bother them that much to move the pthread creation calls into their code.

jadonk commented 10 years ago

Please verify the master branch looks right after the merge.

fhunleth commented 10 years ago

Everything looks fine with master, and my PRU application still works, so that's good. I did notice that PRU_PRU1toPRU0_Interrupt_bin.h was accidentally added at some point. I'm not sure where, but it could be deleted. Thanks for merging all of the changes.