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@rtems.org http://lists.rtems.org/mailman/listinfo/devel