[PATCH] Add GPIO, I2C and SPI support for the RPi BSP
Joel Sherrill
joel.sherrill at oarcorp.com
Fri Oct 31 18:12:58 UTC 2014
On 10/31/2014 11:05 AM, Alan Cudmore wrote:
> I'm going to try to look at the Pi BSP warnings soon. I can try to
> help address some of the issues with this patch as well.
>
Once you figure the warning out, I hope it is easy to fix the other
two BSPs with the same issue. :)
> What is the desired cut off date for 4.11?
>
Last year. :)
Chris was joking that we measure releases based on the number of
developers who have children during the gap. If I am counting right,
there are 4 children and a grandchild.
Seriously, just as soon as it is ready. Settling the tools, the dynamic
loader, and getting the Beagle merged are also on the list.
--joel
> Alan
>
> On Fri, Oct 31, 2014 at 11:41 AM, Pavel Pisa <ppisa4lists at pikron.com
> <mailto:ppisa4lists at pikron.com>> wrote:
>
> Hello Andre,
>
> On Friday 31 of October 2014 12:56:40 Andre Marques wrote:
>
> > +/**
> > + * @brief Generic ISR that clears the event register on the
> Raspberry Pi
> > and calls + * an user defined ISR.
> > + *
> > + * @param[in] arg Void pointer to a handler_arguments structure.
> > + */
> > +static void generic_handler(void* arg)
> > +{
> > + handler_arguments* handler_args;
> > + int rv = 0;
> > + int pin = 0;
> > +
> > + handler_args = (handler_arguments*) arg;
> > +
> > + pin = handler_args->pin_number;
> > +
> > + /* If the interrupt was generated by the pin attached to
> this ISR
> > clears it. */ + if ( BCM2835_REG(BCM2835_GPIO_GPEDS0) & (1 <<
> pin) )
> > + BCM2835_REG(BCM2835_GPIO_GPEDS0) &= (1 << pin);
> > +
> > + /* If not lets the next ISR process the interrupt. */
> > + else
> > + return;
> > +
> > + /* If this pin has the deboucing function attached, call it. */
> > + if ( handler_args->debouncing_tick_count > 0 ) {
> > + rv = debounce_switch(pin);
> > +
> > + if ( rv < 0 )
> > + return;
> > + }
> > +
> > + /* Call the user's ISR. */
> > + (handler_args->handler) (handler_args->handler_arg);
> > +}
>
> There has to be mechanism (array of pointers) for registering args
> for each pin handler routine.
>
> > +/**
> > + * @brief Enables interrupts to be generated on a given GPIO pin.
> > + * When fired that interrupt will call the given handler.
> > + *
> > + * @param[in] dev_pin Raspberry Pi GPIO pin label number (not
> its position
> > + * on the header).
> > + * @param[in] interrupt Type of interrupt to enable for the pin.
> > + * @param[in] handler Pointer to a function that will be called
> every time
> > + * @var interrupt is generated. This
> function must have
> > + * no receiving parameters and return void.
> @param[in] arg or context
>
> > + *
> > + * @retval 0 Interrupt successfully enabled for this pin.
> > + * @retval -1 Could not replace the currently active interrupt
> on this
> > pin, or + * unknown @var interrupt.
> > + */
> > +int gpio_enable_interrupt(int dev_pin, gpio_interrupt
> interrupt, void
> > (*handler)(void)) +{
>
>
> int gpio_enable_interrupt(int dev_pin, gpio_interrupt interrupt, void
> (*handler)(void *arg), void *arg)
>
> It would worth to be able to register mutiple handlers for single
> pin for case o emulating shared IRQ line - i.e. ISA bus, then
> information
> about IRQ handled/unhandled state has to be returned by handler -
> but that
> is not critical. Argument is critical as I have already stated.
>
> The handler function prototype should be same as for RTEMS ISR
> registration
>
> typedef void (*rtems_interrupt_handler)(void *);
>
> This allows to use drivers for some generic chips to be connected
> to some external bus same way on different CPU and boards
> architectures
> with same ISR handler when directly connected to some external
> interrupt
> input and use same driver+handler when interrupt is connected
> to GPIO ISR multiplexor.
>
> I have already raised this concern for previous patch version
> - see mail thread
>
> http://article.gmane.org/gmane.os.rtems.user/13211
>
> > + rtems_status_code sc;
> > + rpi_gpio_pin *pin;
> > +
> > + /* Only consider GPIO pins up to 31. */
> > + if ( dev_pin > 31 )
> > + return -1;
> > +
> > + pin = &gpio_pin[dev_pin-1];
> > +
> > + /* If the pin already has an enabled interrupt removes it first,
> > + * as well as its handler. */
> > + if ( pin->enabled_interrupt != NONE ) {
> > + sc = gpio_disable_interrupt(dev_pin);
> > +
> > + if ( sc != RTEMS_SUCCESSFUL )
> > + return -1;
> > + }
> > +
> > + pin->h_args.pin_number = dev_pin;
> > + pin->h_args.handler = handler;
>
> + pin->h_args.handler_arg = arg;
>
>
> > +
> > + pin->h_args.last_isr_tick = rtems_clock_get_ticks_since_boot();
> > +
> > + /* Installs the generic_handler, which will call the user handler
> > received + * a parameter. */
> > + sc = rtems_interrupt_handler_install(BCM2835_IRQ_ID_GPIO_0,
> > + NULL,
> > + RTEMS_INTERRUPT_SHARED,
> > + (rtems_interrupt_handler)
> > generic_handler, +
> &(pin->h_args));
>
> The use of RTEMS_INTERRUPT_SHARED and registering multiple handlers
> for single pin group is significant overhead. It would be much faster
> to register only one handler (when gpio_enable_interrupt is called
> first time).
> Then read interrupt status register (if before HW masking, apply mask)
> and use ffs (find first bit set) in loop to process all pending
> events.
> Each time ffs gives the bit, clear the bit in the local copy,
> retrieve pin description from the array (ffs gives the index)
> and process user provided handler with corresponding user argument.
>
> > + if ( sc != RTEMS_SUCCESSFUL )
> > + return -1;
> > +
> > + switch ( interrupt ) {
> > + case FALLING_EDGE:
> > +
> > + /* Enables asynchronous falling edge detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPAFEN0) |= (1 << dev_pin);
> > +
> > + break;
> > +
> > + case RISING_EDGE:
> > +
> > + /* Enables asynchronous rising edge detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPAREN0) |= (1 << dev_pin);
> > +
> > + break;
> > +
> > + case BOTH_EDGES:
> > +
> > + /* Enables asynchronous falling edge detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPAFEN0) |= (1 << dev_pin);
> > +
> > + /* Enables asynchronous rising edge detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPAREN0) |= (1 << dev_pin);
> > +
> > + break;
> > +
> > + case LOW_LEVEL:
> > +
> > + /* Enables pin low level detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPLEN0) |= (1 << dev_pin);
> > +
> > + break;
> > +
> > + case HIGH_LEVEL:
> > +
> > + /* Enables pin high level detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPHEN0) |= (1 << dev_pin);
> > +
> > + break;
> > +
> > + case BOTH_LEVELS:
> > +
> > + /* Enables pin low level detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPLEN0) |= (1 << dev_pin);
> > +
> > + /* Enables pin high level detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPHEN0) |= (1 << dev_pin);
> > +
> > + break;
> > +
> > + case NONE:
> > + return 0;
> > +
> > + default:
> > + return -1;
> > + }
> > +
> > + pin->enabled_interrupt = interrupt;
> > +
> > + return 0;
> > +}
>
> The whole function and many others require to define some locking
> to can be used from paralley running threads.
>
> There should be functions which do plain enable and disable of IRQ
> generation and then functions to register and unregister handler
> for pin.
> Register can/should probably call enable at the end to be
> consistent with
> rtems_interrupt_handler_install(). The enable and disable functions
> should and in case of RPi can be written (separates set/reset
> registers and
> single CPU core so CPU level IRQ disable is cheap) such way that
> they can be called from ISR context (no mutex locking). On the
> other hand
>
> gpio_enable_interrupt/gpio_interrupt_handler_install has quite little
> other options than to use mutex. Use of CPU level IRQ disable
> is not good for latencies.
>
> There are probably some compiler (SMP for case of multiple cores)
> barriers to ensure that structure is updated before enable
> of IRQ in hardware.
>
> > +/**
> > + * @brief Stops interrupts from being generated from a given
> GPIO pin
> > + * and removes the corresponding handler.
> > + *
> > + * @param[in] dev_pin Raspberry Pi GPIO pin label number (not
> its position
> > + * on the header).
> > + *
> > + * @retval 0 Interrupt successfully disabled for this pin.
> > + * @retval -1 Could not remove the current interrupt handler or
> could not
> > + * recognise the current active interrupt on this pin.
> > + */
> > +int gpio_disable_interrupt(int dev_pin)
> > +{
> > + rtems_status_code sc;
> > + rpi_gpio_pin *pin;
> > +
> > + pin = &gpio_pin[dev_pin-1];
> > +
> > + switch ( pin->enabled_interrupt ) {
> > + case FALLING_EDGE:
> > +
> > + /* Disables asynchronous falling edge detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPAFEN0) &= ~(1 << dev_pin);
> > +
> > + break;
> > +
> > + case RISING_EDGE:
> > +
> > + /* Disables asynchronous rising edge detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPAREN0) &= ~(1 << dev_pin);
> > +
> > + break;
> > +
> > + case BOTH_EDGES:
> > +
> > + /* Disables asynchronous falling edge detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPAFEN0) &= ~(1 << dev_pin);
> > +
> > + /* Disables asynchronous rising edge detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPAREN0) &= ~(1 << dev_pin);
> > +
> > + break;
> > +
> > + case LOW_LEVEL:
> > +
> > + /* Disables pin low level detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPLEN0) &= ~(1 << dev_pin);
> > +
> > + break;
> > +
> > + case HIGH_LEVEL:
> > +
> > + /* Disables pin high level detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPHEN0) &= ~(1 << dev_pin);
> > +
> > + break;
> > +
> > + case BOTH_LEVELS:
> > +
> > + /* Disables pin low level detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPLEN0) &= ~(1 << dev_pin);
> > +
> > + /* Disables pin high level detection. */
> > + BCM2835_REG(BCM2835_GPIO_GPHEN0) &= ~(1 << dev_pin);
> > +
> > + break;
> > +
> > + case NONE:
> > + return 0;
> > +
> > + default:
> > + return -1;
> > + }
> > +
> > + /* Removes the handler. */
> > + sc = rtems_interrupt_handler_remove(BCM2835_IRQ_ID_GPIO_0,
> > + (rtems_interrupt_handler)
> > generic_handler, +
> &(pin->h_args));
> > +
> > + if ( sc != RTEMS_SUCCESSFUL )
> > + return -1;
> > +
> > + pin->enabled_interrupt = NONE;
> > +
> > + return 0;
> > +}
>
>
> You can look how interrupts are used in real-time RPi demo application
> to control DC motor with incremental encoder signals detections by
> software.
> We have succeed to process 25000 interrupts/second with fully
> preemptive
> Linux kernel where actual IRQ handlers are run in kernel
> threads/this means
> than each ISR results in kernel context switch.
>
> http://lintarget.sourceforge.net/rpi-motor-control/index.html
>
> https://github.com/ppisa/rpi-rt-control/blob/master/kernel/modules/rpi_gpio_irc_module.c
>
> RTEMS should be in much better situation there because its context
> switch
> is much lighter - no MMU consideration etc. So same task should
> be portable for RTEMS easily. Even quadrature decoding can be done in
> single handler for both pins because there is no latency between
> HW ISR and its handler running in the kernel thread. So simple
> state decoding could be used by reading values from GPIO input
> register.
>
> Please, try to make API and functions robust and (re)usable.
> If you have problem to see all consequences then I can help with
> some advises or if others agree I would prepare some follow up
> patches to clean the code as my time allows ....
> I have not so big problem with time to rewrite code but time
> for testing is much bigger problem.
>
> Best wishes,
>
> Pavel
>
>
>
>
> _______________________________________________
> devel mailing list
> devel at rtems.org <mailto:devel at rtems.org>
> http://lists.rtems.org/mailman/listinfo/devel
>
>
--
Joel Sherrill, Ph.D. Director of Research & Development
joel.sherrill at OARcorp.com On-Line Applications Research
Ask me about RTEMS: a free RTOS Huntsville AL 35805
Support Available (256) 722-9985
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20141031/a25123cf/attachment-0002.html>
More information about the devel
mailing list