[PATCH 08/15] HiFive1: add irq dispatching function

[Hesham Almatary]

On Fri, Aug 18, 2017 at 1:11 AM, Gedare Bloom <gedare at rtems.org> wrote:
> On Wed, Aug 16, 2017 at 11:12 AM, Denis Obrezkov
> <denisobrezkov at gmail.com> wrote:
>> ---
>>  c/src/lib/libbsp/riscv32/hifive1/irq/irq.c | 90 ++++++++++++++++++++++++++++++
>>  1 file changed, 90 insertions(+)
>>  create mode 100644 c/src/lib/libbsp/riscv32/hifive1/irq/irq.c
>>
>> diff --git a/c/src/lib/libbsp/riscv32/hifive1/irq/irq.c b/c/src/lib/libbsp/riscv32/hifive1/irq/irq.c
>> new file mode 100644
>> index 0000000..6fd4e75
>> --- /dev/null
>> +++ b/c/src/lib/libbsp/riscv32/hifive1/irq/irq.c
>> @@ -0,0 +1,90 @@
>> +/**
>> + * @file
>> + *
>> + * @ingroup riscv_interrupt
>> + *
>> + * @brief Interrupt support.
>> + */
>> +
>> +/*
>> + * RISCV CPU Dependent Source
>> + *
>> + * Copyright (c) 2015 University of York.
>> + * Hesham ALMatary <hmka501 at york.ac.uk>
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + * 1. Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + * 2. Redistributions in binary form must reproduce the above copyright
>> + *    notice, this list of conditions and the following disclaimer in the
>> + *    documentation and/or other materials provided with the distribution.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
>> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
>> + * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
>> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
>> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
>> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
>> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
>> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>> + * SUCH DAMAGE.
>> + */
>> +
>> +#include <bsp/fe310.h>
>> +#include <bsp/irq.h>
>> +#include <bsp/irq-generic.h>
>> +
>> +/* Almost all of the jobs that the following functions should
>> + * do are implemented in cpukit
>> + */
>> +
>> +void bsp_interrupt_handler_default(rtems_vector_number vector)
>> +{
>> +    printk("spurious interrupt: %u\n", vector);
>> +}
>> +
>> +rtems_status_code bsp_interrupt_facility_initialize()
>> +{
>> +  return 0;
>> +}
>> +
>> +rtems_status_code bsp_interrupt_vector_enable(rtems_vector_number vector)
>> +{
>> +  return 0;
>> +}
>> +
>> +rtems_status_code bsp_interrupt_vector_disable(rtems_vector_number vector)
>> +{
>> +  return 0;
>> +}
>> +
>> +static uint32_t cause = 0;
>> +volatile uint64_t * mtimecmp = (volatile uint64_t *)0x02004000;
>> +volatile uint64_t * mtime = (volatile uint64_t *)0x0200bff8;
>> +
Avoid hard-coded values whenever possible, same for the rest of the patches.

> What are these global variables used for?
>
>> +void handle_trap_new ()
> What is the purpose of this function?
>
>> +{
>> +    asm volatile ("csrr %0, mcause": "=r" (cause));
> It would be good to have a static inline function for reading this register.
>
+1 Consider adding a utility .h file to read/write commonly used
registers (e.g. mstatus)
>> +    if (cause & MCAUSE_INT) {
>> +      /* an interrupt occurred */
>> +      if ((cause & MCAUSE_MTIME) == MCAUSE_MTIME) {
I believe you don't need the " == MCAUSE_MTIME"
>> +       /* Timer interrupt */
>> +           (*mtimecmp) = (*mtime) + FE310_CLOCK_PERIOD;
>> +        bsp_interrupt_handler_table[1].handler(bsp_interrupt_handler_table[1].arg);
>> +      } else if ((cause & MCAUSE_MEXT) == MCAUSE_MEXT) {
>> +             /*External interrupt */
>> +          asm volatile ("csrci mie, 0x800");
I assume you're only handling timer interrupts. It would be useful to
tell the user (printk?) that this interrupt type is not handled
proprely yet, and add a FIXME/TODO comment here.
>> +      } else if ((cause & MCAUSE_MSWI) == MCAUSE_MSWI) {
>> +             /* Software interrupt */
>> +             volatile uint32_t * msip_reg = (volatile uint32_t *) 0x02000000;
>> +             *msip_reg = 0;
>> +      }
>> +    } else {
>> +      /* an exception occurred */
>> +    }
>> +
>> +}
>> --
>> 2.1.4
>>
>> _______________________________________________
>> devel mailing list
>> devel at rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel



-- 
Hesham