[PATCH] Add GPIO, I2C and SPI support for the RPi BSP
Alan Cudmore
alan.cudmore at gmail.com
Fri Oct 31 16:05:08 UTC 2014
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.
What is the desired cut off date for 4.11?
Alan
On Fri, Oct 31, 2014 at 11:41 AM, Pavel Pisa <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
> http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20141031/898b9463/attachment-0002.html>
More information about the devel
mailing list