Message ID | 20240618173053.364776-4-jamestiotio@gmail.com |
---|---|
State | Handled Elsewhere |
Headers | show |
Series | riscv: sbi: Add support to test timer extension | expand |
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@gmail.com> > --- > 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@gmail.com> > + */ > +#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. And please provide two functions rather than a toggle with a parameter, i.e. timer_interrupt_enable() / timer_interrupt_disable() Thanks, drew
On Wed, Jun 19, 2024 at 4:39 PM Andrew Jones <andrew.jones@linux.dev> 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@gmail.com> > > --- > > 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@gmail.com> > > + */ > > +#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
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@gmail.com> + */ +#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); +}
Add some helper methods to toggle the interrupt enable bits in the SIE register. Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com> --- 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