OMAPL-L138: using SYSCFG_CHIPINT interrupts

Jan-Marc,

There was an issue on OMAPL where each interrupt sent from the DSP to the ARM was received twice by the ARM. This came about because the SYSCFG module uses a level-type interrupt (interrupt remains asserted until cleared) but the ARM INTC controller expects a pulse-type interrupt.

We developed the following fix (highlighted in yellow bellow):

static irqreturn_t davinci_rproc_callback(int irq, void *p)
{
    if (__raw_readl(syscfg0_base + SYSCFG_CHIPSIG_OFFSET) &
        SYSCFG_CHIPINT0) {
            /*
             * Following can fail if work is pending; but it's OK since the
             * work function will loop to process all incoming messages.
             * schedule_work() calls handle_event with pending bit off.
             */
            (void)schedule_work(&workqueue);

            /* Clear interrupt: */
            __raw_writel(SYSCFG_CHIPINT0,
                    syscfg0_base + SYSCFG_CHIPSIG_CLR_OFFSET);

            irq_data->chip->irq_ack(irq_data);
    }

    return IRQ_HANDLED;
}

The irq_data is obtained as follows:

irq_data = irq_get_irq_data(28);

~Ramsey

Hi Ramsey,

thank you very much for the hint.

I assume that your code snippet is the interrupt handler for the SYSCFG_CHIPINT0 interrupt.

As far as I understood you clear the interrupt source by writing to the SYSCFG_CHIPSIG_CLR register.
Then you explicitly acknowledge the interrupt witch irq_ack().

Perhaps I'm not really understand your solution, but why do you this when the corresponding bit for the SYSCFG_CHIPINT0 interrupt in the SYSCFG_CHIPSIG register is "1"? I mean, that is the expectet interrupt.
I get the second interrupt for SYSCFG_CHIPINT0 with corresponding bit "0" in the SYSCFG_CHIPSIG register!

The following code snippet is my interrupt handler:

static irqreturn_t uio_pdrv_genirq_handler(int irq, struct uio_info *dev_info)
{
    struct uio_pdrv_genirq_platdata *priv = dev_info->priv;
    void __iomem* vaddr = 0;
    u32 chipsig = 0;
    u32 chipsig_clr = 0;

    spin_lock(&priv->lock);
    if ((30 == irq) || (31 == irq)) {
        /* Disable the interrupt in the interrupt controller, and
         * remember the state so we can allow user space to enable it later.
         */
        if (!__test_and_set_bit(UIO_IRQ_DISABLED, &priv->flags)) {
            disable_irq_nosync(irq);
        }
        vaddr = ioremap_nocache(0x01c14000, SZ_4K - 1); // SYSCFG0
        if (0 != vaddr) {
            chipsig = __raw_readl(vaddr + 0x174);

            if ((30 == irq) && (chipsig & (1 << 2))) {
                // clear SYSCFG_CHIPINT2 interrupt
                chipsig_clr = (1 << 2);
                __raw_writel(chipsig_clr, vaddr + 0x178);
            }
            if ((31 == irq) && (chipsig & (1 << 3))) {
                // clear SYSCFG_CHIPINT3 interrupt
                chipsig_clr = (1 << 3);
                __raw_writel(chipsig_clr, vaddr + 0x178);
            }
            iounmap(vaddr);
        }
    }
    else {
        /* Just disable the interrupt in the interrupt controller, and
         * remember the state so we can allow user space to enable it later.
         */
        if (!__test_and_set_bit(UIO_IRQ_DISABLED, &priv->flags)) {
            disable_irq_nosync(irq);
        }
    }
    spin_unlock(&priv->lock);
    return IRQ_HANDLED;
}

How would you modify this code in order to not called twice for a single signalisation?

Best regards
Jan-Marc.

Jan-Marc Stranz said:

Perhaps I'm not really understand your solution, but why do you this when the corresponding bit for the SYSCFG_CHIPINT0 interrupt in the SYSCFG_CHIPSIG register is "1"? I mean, that is the expectet interrupt.

I'm not understanding your question or point.

The code snippet that Ramsey posted is clearing bit 0 of the CHIPSIG register by writing 0x1 to the CHIPSIG_CLR register (SYSCFG_CHIPSIG0 is #defined as BIT(0), which is 1 << 0).  This lowers the actual signal level, but Linux has already recognized the signal level as still high and will call your handler a 2nd time.  Note this comment from da8xx_remoteproc.c:

                /*
                 * ACK intr to AINTC.
                 *
                 * It has already been ack'ed by the kernel before calling
                 * this function, but since the ARM<->DSP interrupts in the
                 * CHIPSIG register are "level" instead of "pulse" variety,
                 * we need to ack it after taking down the level else we'll
                 * be called again immediately after returning.
                 */
                drproc->ack_fxn(drproc->irq_data);

This is from the function da8xx_rproc_callback() in da8xx_remoteproc.c in Linux v4.1.  Note that the implementation of this function in v4.1 is different than what Ramsey posted (probably from Linux v3.1).

Jan-Marc Stranz said:

How would you modify this code in order to not called twice for a single signalisation?

Just put the "ack()" call after where you're writing to the CHIPSIG_CLR register.  Your driver may need to use a different way to access the "ack" function.

Also, are you even using TI-RTOS?  This is the TI-RTOS forum, and from what I can tell you're just doing Linux.

Regards,

- Rob