On Wed, Jun 19, 2024 at 4:39 PM Andrew Jones <andrew.jones@xxxxxxxxx> wrote:
>
> On Wed, Jun 19, 2024 at 01:30:52AM GMT, James Raphael Tiovalen wrote:
> > Add some helper methods to toggle the interrupt enable bits in the SIE
> > register.
> >
> > Signed-off-by: James Raphael Tiovalen <jamestiotio@xxxxxxxxx>
> > ---
> > riscv/Makefile | 1 +
> > lib/riscv/asm/csr.h | 7 +++++++
> > lib/riscv/asm/interrupt.h | 12 ++++++++++++
> > lib/riscv/interrupt.c | 39 +++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 59 insertions(+)
> > create mode 100644 lib/riscv/asm/interrupt.h
> > create mode 100644 lib/riscv/interrupt.c
> >
> > diff --git a/riscv/Makefile b/riscv/Makefile
> > index 919a3ebb..108d4481 100644
> > --- a/riscv/Makefile
> > +++ b/riscv/Makefile
> > @@ -30,6 +30,7 @@ cflatobjs += lib/memregions.o
> > cflatobjs += lib/on-cpus.o
> > cflatobjs += lib/vmalloc.o
> > cflatobjs += lib/riscv/bitops.o
> > +cflatobjs += lib/riscv/interrupt.o
> > cflatobjs += lib/riscv/io.o
> > cflatobjs += lib/riscv/isa.o
> > cflatobjs += lib/riscv/mmu.o
> > diff --git a/lib/riscv/asm/csr.h b/lib/riscv/asm/csr.h
> > index c1777744..da58b0ce 100644
> > --- a/lib/riscv/asm/csr.h
> > +++ b/lib/riscv/asm/csr.h
> > @@ -4,15 +4,22 @@
> > #include <linux/const.h>
> >
> > #define CSR_SSTATUS 0x100
> > +#define CSR_SIE 0x104
> > #define CSR_STVEC 0x105
> > #define CSR_SSCRATCH 0x140
> > #define CSR_SEPC 0x141
> > #define CSR_SCAUSE 0x142
> > #define CSR_STVAL 0x143
> > +#define CSR_SIP 0x144
> > #define CSR_SATP 0x180
> >
> > #define SSTATUS_SIE (_AC(1, UL) << 1)
> >
> > +#define SIE_SSIE (_AC(1, UL) << 1)
> > +#define SIE_STIE (_AC(1, UL) << 5)
> > +#define SIE_SEIE (_AC(1, UL) << 9)
> > +#define SIE_LCOFIE (_AC(1, UL) << 13)
> > +
> > /* Exception cause high bit - is an interrupt if set */
> > #define CAUSE_IRQ_FLAG (_AC(1, UL) << (__riscv_xlen - 1))
> >
> > diff --git a/lib/riscv/asm/interrupt.h b/lib/riscv/asm/interrupt.h
> > new file mode 100644
> > index 00000000..b760afbb
> > --- /dev/null
> > +++ b/lib/riscv/asm/interrupt.h
> > @@ -0,0 +1,12 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +#ifndef _ASMRISCV_INTERRUPT_H_
> > +#define _ASMRISCV_INTERRUPT_H_
> > +
> > +#include <stdbool.h>
> > +
> > +void toggle_software_interrupt(bool enable);
> > +void toggle_timer_interrupt(bool enable);
> > +void toggle_external_interrupt(bool enable);
> > +void toggle_local_cof_interrupt(bool enable);
> > +
> > +#endif /* _ASMRISCV_INTERRUPT_H_ */
> > diff --git a/lib/riscv/interrupt.c b/lib/riscv/interrupt.c
> > new file mode 100644
> > index 00000000..bc0e16f1
> > --- /dev/null
> > +++ b/lib/riscv/interrupt.c
> > @@ -0,0 +1,39 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2024, James Raphael Tiovalen <jamestiotio@xxxxxxxxx>
> > + */
> > +#include <libcflat.h>
> > +#include <asm/csr.h>
> > +#include <asm/interrupt.h>
> > +
> > +void toggle_software_interrupt(bool enable)
> > +{
> > + if (enable)
> > + csr_set(CSR_SIE, SIE_SSIE);
> > + else
> > + csr_clear(CSR_SIE, SIE_SSIE);
> > +}
> > +
> > +void toggle_timer_interrupt(bool enable)
> > +{
> > + if (enable)
> > + csr_set(CSR_SIE, SIE_STIE);
> > + else
> > + csr_clear(CSR_SIE, SIE_STIE);
> > +}
> > +
> > +void toggle_external_interrupt(bool enable)
> > +{
> > + if (enable)
> > + csr_set(CSR_SIE, SIE_SEIE);
> > + else
> > + csr_clear(CSR_SIE, SIE_SEIE);
> > +}
> > +
> > +void toggle_local_cof_interrupt(bool enable)
> > +{
> > + if (enable)
> > + csr_set(CSR_SIE, SIE_LCOFIE);
> > + else
> > + csr_clear(CSR_SIE, SIE_LCOFIE);
> > +}
> > --
> > 2.43.0
> >
>
> Most of this patch seems premature since the series only needs
> toggle_timer_interrupt(). Also, I think lib/riscv/interrupt.c
> is premature because something like toggle_timer_interrupt()
> can be a static inline in a new lib/riscv/asm/timer.h file.
>
Got it. In that case, I will combine the changes with the actual test
since we will be adding only the timer interrupt code.
> And please provide two functions rather than a toggle with
> a parameter, i.e.
>
> timer_interrupt_enable() / timer_interrupt_disable()
>
Sure, will do that.
> Thanks,
> drew
Best regards,
James Raphael Tiovalen
