On Sat, Aug 19, 2017 at 11:08 PM, Hesham Almatary
<heshamelmat...@gmail.com> wrote:
> On Fri, Aug 18, 2017 at 1:11 AM, Gedare Bloom <ged...@rtems.org> wrote:
>> On Wed, Aug 16, 2017 at 11:12 AM, Denis Obrezkov
>> <denisobrez...@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 <hmka...@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"
Keep it. We prefer the explicit check for bit-wise equality to the
flag in RTEMS style:
https://devel.rtems.org/wiki/Developer/Coding/Conventions#LanguageandCompiler
>>> + /* 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@rtems.org
>>> http://lists.rtems.org/mailman/listinfo/devel
>> _______________________________________________
>> devel mailing list
>> devel@rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
>
>
>
> --
> Hesham
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
