From: Andrew Jones <ajones@ventanamicro.com>
To: Anup Patel <apatel@ventanamicro.com>
Cc: Sunil V L <sunilvl@ventanamicro.com>,
"Rafael J . Wysocki" <rafael@kernel.org>,
Palmer Dabbelt <palmer@dabbelt.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Alexandre Ghiti <alex@ghiti.fr>, Len Brown <lenb@kernel.org>,
Atish Patra <atish.patra@linux.dev>,
Anup Patel <anup@brainfault.org>, Will Deacon <will@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
linux-acpi@vger.kernel.org, linux-riscv@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] RISC-V: Add common csr_read_num() and csr_write_num() functions
Date: Fri, 15 Aug 2025 15:02:02 -0500 [thread overview]
Message-ID: <20250815-e69bdc695cf78e6ce07e580c@orel> (raw)
In-Reply-To: <20250815161406.76370-3-apatel@ventanamicro.com>
On Fri, Aug 15, 2025 at 09:44:06PM +0530, Anup Patel wrote:
> In RISC-V, there is no CSR read/write instruction which takes CSR
> number via register so add common csr_read_num() and csr_write_num()
> functions which allow accessing certain CSRs by passing CSR number
> as parameter. These common functions will be first used by the
> ACPI CPPC driver and RISC-V PMU driver.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> arch/riscv/include/asm/csr.h | 3 +
> arch/riscv/kernel/Makefile | 1 +
> arch/riscv/kernel/csr.c | 177 +++++++++++++++++++++++++++++++++++
> drivers/acpi/riscv/cppc.c | 17 ++--
> drivers/perf/riscv_pmu.c | 43 +--------
> 5 files changed, 189 insertions(+), 52 deletions(-)
> create mode 100644 arch/riscv/kernel/csr.c
>
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 6fed42e37705..1540626b3540 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -575,6 +575,9 @@
> : "memory"); \
> })
>
> +extern unsigned long csr_read_num(unsigned long csr_num, int *out_err);
> +extern void csr_write_num(unsigned long csr_num, unsigned long val, int *out_err);
My preference would be for an interface with the return/err parameters the
other way around, i.e.
int csr_read_num(unsigned long csr_num, unsigned long *val);
int csr_write_num(unsigned long csr_num, unsigned long val);
and then ensure all callers always check that the return value is zero
before proceeding to use val or assume val was written.
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_RISCV_CSR_H */
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index c7b542573407..0a75e20bde18 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -50,6 +50,7 @@ obj-y += soc.o
> obj-$(CONFIG_RISCV_ALTERNATIVE) += alternative.o
> obj-y += cpu.o
> obj-y += cpufeature.o
> +obj-y += csr.o
> obj-y += entry.o
> obj-y += irq.o
> obj-y += process.o
> diff --git a/arch/riscv/kernel/csr.c b/arch/riscv/kernel/csr.c
> new file mode 100644
> index 000000000000..f7de45bb597c
> --- /dev/null
> +++ b/arch/riscv/kernel/csr.c
> @@ -0,0 +1,177 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025 Ventana Micro Systems Inc.
> + */
> +
> +#define pr_fmt(fmt) "riscv: " fmt
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/printk.h>
> +#include <linux/types.h>
> +#include <asm/csr.h>
> +
> +#define CSR_CUSTOM0_U_RW_BASE 0x800
> +#define CSR_CUSTOM0_U_RW_COUNT 0x100
> +
> +#define CSR_CUSTOM1_U_RO_BASE 0xCC0
> +#define CSR_CUSTOM1_U_RO_COUNT 0x040
> +
> +#define CSR_CUSTOM2_S_RW_BASE 0x5C0
> +#define CSR_CUSTOM2_S_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM3_S_RW_BASE 0x9C0
> +#define CSR_CUSTOM3_S_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM4_S_RO_BASE 0xDC0
> +#define CSR_CUSTOM4_S_RO_COUNT 0x040
> +
> +#define CSR_CUSTOM5_HS_RW_BASE 0x6C0
> +#define CSR_CUSTOM5_HS_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM6_HS_RW_BASE 0xAC0
> +#define CSR_CUSTOM6_HS_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM7_HS_RO_BASE 0xEC0
> +#define CSR_CUSTOM7_HS_RO_COUNT 0x040
> +
> +#define CSR_CUSTOM8_M_RW_BASE 0x7C0
> +#define CSR_CUSTOM8_M_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM9_M_RW_BASE 0xBC0
> +#define CSR_CUSTOM9_M_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM10_M_RO_BASE 0xFC0
> +#define CSR_CUSTOM10_M_RO_COUNT 0x040
> +
> +unsigned long csr_read_num(unsigned long csr_num, int *out_err)
> +{
> +#define switchcase_csr_read(__csr_num) \
> + case (__csr_num): \
> + return csr_read(__csr_num)
> +#define switchcase_csr_read_2(__csr_num) \
> + switchcase_csr_read(__csr_num + 0); \
> + switchcase_csr_read(__csr_num + 1)
> +#define switchcase_csr_read_4(__csr_num) \
> + switchcase_csr_read_2(__csr_num + 0); \
> + switchcase_csr_read_2(__csr_num + 2)
> +#define switchcase_csr_read_8(__csr_num) \
> + switchcase_csr_read_4(__csr_num + 0); \
> + switchcase_csr_read_4(__csr_num + 4)
> +#define switchcase_csr_read_16(__csr_num) \
> + switchcase_csr_read_8(__csr_num + 0); \
> + switchcase_csr_read_8(__csr_num + 8)
> +#define switchcase_csr_read_32(__csr_num) \
> + switchcase_csr_read_16(__csr_num + 0); \
> + switchcase_csr_read_16(__csr_num + 16)
> +#define switchcase_csr_read_64(__csr_num) \
> + switchcase_csr_read_32(__csr_num + 0); \
> + switchcase_csr_read_32(__csr_num + 32)
> +#define switchcase_csr_read_128(__csr_num) \
> + switchcase_csr_read_64(__csr_num + 0); \
> + switchcase_csr_read_64(__csr_num + 64)
> +#define switchcase_csr_read_256(__csr_num) \
> + switchcase_csr_read_128(__csr_num + 0); \
> + switchcase_csr_read_128(__csr_num + 128)
> +
> + if (out_err)
> + *out_err = 0;
> +
> + switch (csr_num) {
> + switchcase_csr_read_32(CSR_CYCLE);
> + switchcase_csr_read_32(CSR_CYCLEH);
> + switchcase_csr_read_256(CSR_CUSTOM0_U_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM1_U_RO_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM2_S_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM3_S_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM4_S_RO_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM5_HS_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM6_HS_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM7_HS_RO_BASE);
> +#ifdef CONFIG_RISCV_M_MODE
> + switchcase_csr_read_64(CSR_CUSTOM8_M_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM9_M_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM10_M_RO_BASE);
> +#endif
> + default:
> + if (out_err)
> + *out_err = -EINVAL;
> + else
> + pr_err("%s: csr 0x%lx not supported\n", __func__, csr_num);
> + break;
> + }
> +
> + return 0;
> +#undef switchcase_csr_read_256
> +#undef switchcase_csr_read_128
> +#undef switchcase_csr_read_64
> +#undef switchcase_csr_read_32
> +#undef switchcase_csr_read_16
> +#undef switchcase_csr_read_8
> +#undef switchcase_csr_read_4
> +#undef switchcase_csr_read_2
> +#undef switchcase_csr_read
> +}
> +EXPORT_SYMBOL_GPL(csr_read_num);
> +
> +void csr_write_num(unsigned long csr_num, unsigned long val, int *out_err)
> +{
> +#define switchcase_csr_write(__csr_num, __val) \
> + case (__csr_num): \
> + csr_write(__csr_num, __val); \
> + break
> +#define switchcase_csr_write_2(__csr_num, __val) \
> + switchcase_csr_write(__csr_num + 0, __val); \
> + switchcase_csr_write(__csr_num + 1, __val)
> +#define switchcase_csr_write_4(__csr_num, __val) \
> + switchcase_csr_write_2(__csr_num + 0, __val); \
> + switchcase_csr_write_2(__csr_num + 2, __val)
> +#define switchcase_csr_write_8(__csr_num, __val) \
> + switchcase_csr_write_4(__csr_num + 0, __val); \
> + switchcase_csr_write_4(__csr_num + 4, __val)
> +#define switchcase_csr_write_16(__csr_num, __val) \
> + switchcase_csr_write_8(__csr_num + 0, __val); \
> + switchcase_csr_write_8(__csr_num + 8, __val)
> +#define switchcase_csr_write_32(__csr_num, __val) \
> + switchcase_csr_write_16(__csr_num + 0, __val); \
> + switchcase_csr_write_16(__csr_num + 16, __val)
> +#define switchcase_csr_write_64(__csr_num, __val) \
> + switchcase_csr_write_32(__csr_num + 0, __val); \
> + switchcase_csr_write_32(__csr_num + 32, __val)
> +#define switchcase_csr_write_128(__csr_num, __val) \
> + switchcase_csr_write_64(__csr_num + 0, __val); \
> + switchcase_csr_write_64(__csr_num + 64, __val)
> +#define switchcase_csr_write_256(__csr_num, __val) \
> + switchcase_csr_write_128(__csr_num + 0, __val); \
> + switchcase_csr_write_128(__csr_num + 128, __val)
> +
> + if (out_err)
> + *out_err = 0;
> +
> + switch (csr_num) {
> + switchcase_csr_write_256(CSR_CUSTOM0_U_RW_BASE, val);
> + switchcase_csr_write_64(CSR_CUSTOM2_S_RW_BASE, val);
> + switchcase_csr_write_64(CSR_CUSTOM3_S_RW_BASE, val);
> + switchcase_csr_write_64(CSR_CUSTOM5_HS_RW_BASE, val);
> + switchcase_csr_write_64(CSR_CUSTOM6_HS_RW_BASE, val);
> +#ifdef CONFIG_RISCV_M_MODE
> + switchcase_csr_write_64(CSR_CUSTOM8_M_RW_BASE, val);
> + switchcase_csr_write_64(CSR_CUSTOM9_M_RW_BASE, val);
> +#endif
> + default:
> + if (out_err)
> + *out_err = -EINVAL;
> + else
> + pr_err("%s: csr 0x%lx not supported\n", __func__, csr_num);
> + break;
> + }
> +#undef switchcase_csr_write_256
> +#undef switchcase_csr_write_128
> +#undef switchcase_csr_write_64
> +#undef switchcase_csr_write_32
> +#undef switchcase_csr_write_16
> +#undef switchcase_csr_write_8
> +#undef switchcase_csr_write_4
> +#undef switchcase_csr_write_2
> +#undef switchcase_csr_write
> +}
> +EXPORT_SYMBOL_GPL(csr_write_num);
> diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
> index 42c1a9052470..fe491937ed25 100644
> --- a/drivers/acpi/riscv/cppc.c
> +++ b/drivers/acpi/riscv/cppc.c
> @@ -65,24 +65,19 @@ static void sbi_cppc_write(void *write_data)
> static void cppc_ffh_csr_read(void *read_data)
> {
> struct sbi_cppc_data *data = (struct sbi_cppc_data *)read_data;
> + int err;
>
> - switch (data->reg) {
> - /* Support only TIME CSR for now */
> - case CSR_TIME:
> - data->ret.value = csr_read(CSR_TIME);
> - data->ret.error = 0;
> - break;
> - default:
> - data->ret.error = -EINVAL;
> - break;
> - }
> + data->ret.value = csr_read_num(data->reg, &err);
> + data->ret.error = err;
> }
>
> static void cppc_ffh_csr_write(void *write_data)
> {
> struct sbi_cppc_data *data = (struct sbi_cppc_data *)write_data;
> + int err;
>
> - data->ret.error = -EINVAL;
> + csr_write_num(data->reg, data->val, &err);
> + data->ret.error = err;
> }
>
> /*
> diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
> index 7644147d50b4..aa053254448d 100644
> --- a/drivers/perf/riscv_pmu.c
> +++ b/drivers/perf/riscv_pmu.c
> @@ -16,6 +16,7 @@
> #include <linux/smp.h>
> #include <linux/sched_clock.h>
>
> +#include <asm/csr.h>
> #include <asm/sbi.h>
>
> static bool riscv_perf_user_access(struct perf_event *event)
> @@ -88,46 +89,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> userpg->cap_user_time_short = 1;
> }
>
> -static unsigned long csr_read_num(int csr_num)
> -{
> -#define switchcase_csr_read(__csr_num, __val) {\
> - case __csr_num: \
> - __val = csr_read(__csr_num); \
> - break; }
> -#define switchcase_csr_read_2(__csr_num, __val) {\
> - switchcase_csr_read(__csr_num + 0, __val) \
> - switchcase_csr_read(__csr_num + 1, __val)}
> -#define switchcase_csr_read_4(__csr_num, __val) {\
> - switchcase_csr_read_2(__csr_num + 0, __val) \
> - switchcase_csr_read_2(__csr_num + 2, __val)}
> -#define switchcase_csr_read_8(__csr_num, __val) {\
> - switchcase_csr_read_4(__csr_num + 0, __val) \
> - switchcase_csr_read_4(__csr_num + 4, __val)}
> -#define switchcase_csr_read_16(__csr_num, __val) {\
> - switchcase_csr_read_8(__csr_num + 0, __val) \
> - switchcase_csr_read_8(__csr_num + 8, __val)}
> -#define switchcase_csr_read_32(__csr_num, __val) {\
> - switchcase_csr_read_16(__csr_num + 0, __val) \
> - switchcase_csr_read_16(__csr_num + 16, __val)}
> -
> - unsigned long ret = 0;
> -
> - switch (csr_num) {
> - switchcase_csr_read_32(CSR_CYCLE, ret)
> - switchcase_csr_read_32(CSR_CYCLEH, ret)
> - default :
> - break;
> - }
> -
> - return ret;
> -#undef switchcase_csr_read_32
> -#undef switchcase_csr_read_16
> -#undef switchcase_csr_read_8
> -#undef switchcase_csr_read_4
> -#undef switchcase_csr_read_2
> -#undef switchcase_csr_read
> -}
> -
> /*
> * Read the CSR of a corresponding counter.
> */
> @@ -139,7 +100,7 @@ unsigned long riscv_pmu_ctr_read_csr(unsigned long csr)
> return -EINVAL;
> }
>
> - return csr_read_num(csr);
> + return csr_read_num(csr, NULL);
> }
>
> u64 riscv_pmu_ctr_get_width_mask(struct perf_event *event)
> --
> 2.43.0
>
Other than the suggestion to flip the interface,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Jones <ajones@ventanamicro.com>
To: Anup Patel <apatel@ventanamicro.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
Alexandre Ghiti <alex@ghiti.fr>,
"Rafael J . Wysocki" <rafael@kernel.org>,
Anup Patel <anup@brainfault.org>,
Atish Patra <atish.patra@linux.dev>,
linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org,
Palmer Dabbelt <palmer@dabbelt.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
linux-riscv@lists.infradead.org, Will Deacon <will@kernel.org>,
Len Brown <lenb@kernel.org>
Subject: Re: [PATCH 2/2] RISC-V: Add common csr_read_num() and csr_write_num() functions
Date: Fri, 15 Aug 2025 15:02:02 -0500 [thread overview]
Message-ID: <20250815-e69bdc695cf78e6ce07e580c@orel> (raw)
In-Reply-To: <20250815161406.76370-3-apatel@ventanamicro.com>
On Fri, Aug 15, 2025 at 09:44:06PM +0530, Anup Patel wrote:
> In RISC-V, there is no CSR read/write instruction which takes CSR
> number via register so add common csr_read_num() and csr_write_num()
> functions which allow accessing certain CSRs by passing CSR number
> as parameter. These common functions will be first used by the
> ACPI CPPC driver and RISC-V PMU driver.
>
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
> arch/riscv/include/asm/csr.h | 3 +
> arch/riscv/kernel/Makefile | 1 +
> arch/riscv/kernel/csr.c | 177 +++++++++++++++++++++++++++++++++++
> drivers/acpi/riscv/cppc.c | 17 ++--
> drivers/perf/riscv_pmu.c | 43 +--------
> 5 files changed, 189 insertions(+), 52 deletions(-)
> create mode 100644 arch/riscv/kernel/csr.c
>
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 6fed42e37705..1540626b3540 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -575,6 +575,9 @@
> : "memory"); \
> })
>
> +extern unsigned long csr_read_num(unsigned long csr_num, int *out_err);
> +extern void csr_write_num(unsigned long csr_num, unsigned long val, int *out_err);
My preference would be for an interface with the return/err parameters the
other way around, i.e.
int csr_read_num(unsigned long csr_num, unsigned long *val);
int csr_write_num(unsigned long csr_num, unsigned long val);
and then ensure all callers always check that the return value is zero
before proceeding to use val or assume val was written.
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_RISCV_CSR_H */
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index c7b542573407..0a75e20bde18 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -50,6 +50,7 @@ obj-y += soc.o
> obj-$(CONFIG_RISCV_ALTERNATIVE) += alternative.o
> obj-y += cpu.o
> obj-y += cpufeature.o
> +obj-y += csr.o
> obj-y += entry.o
> obj-y += irq.o
> obj-y += process.o
> diff --git a/arch/riscv/kernel/csr.c b/arch/riscv/kernel/csr.c
> new file mode 100644
> index 000000000000..f7de45bb597c
> --- /dev/null
> +++ b/arch/riscv/kernel/csr.c
> @@ -0,0 +1,177 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2025 Ventana Micro Systems Inc.
> + */
> +
> +#define pr_fmt(fmt) "riscv: " fmt
> +#include <linux/err.h>
> +#include <linux/export.h>
> +#include <linux/printk.h>
> +#include <linux/types.h>
> +#include <asm/csr.h>
> +
> +#define CSR_CUSTOM0_U_RW_BASE 0x800
> +#define CSR_CUSTOM0_U_RW_COUNT 0x100
> +
> +#define CSR_CUSTOM1_U_RO_BASE 0xCC0
> +#define CSR_CUSTOM1_U_RO_COUNT 0x040
> +
> +#define CSR_CUSTOM2_S_RW_BASE 0x5C0
> +#define CSR_CUSTOM2_S_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM3_S_RW_BASE 0x9C0
> +#define CSR_CUSTOM3_S_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM4_S_RO_BASE 0xDC0
> +#define CSR_CUSTOM4_S_RO_COUNT 0x040
> +
> +#define CSR_CUSTOM5_HS_RW_BASE 0x6C0
> +#define CSR_CUSTOM5_HS_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM6_HS_RW_BASE 0xAC0
> +#define CSR_CUSTOM6_HS_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM7_HS_RO_BASE 0xEC0
> +#define CSR_CUSTOM7_HS_RO_COUNT 0x040
> +
> +#define CSR_CUSTOM8_M_RW_BASE 0x7C0
> +#define CSR_CUSTOM8_M_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM9_M_RW_BASE 0xBC0
> +#define CSR_CUSTOM9_M_RW_COUNT 0x040
> +
> +#define CSR_CUSTOM10_M_RO_BASE 0xFC0
> +#define CSR_CUSTOM10_M_RO_COUNT 0x040
> +
> +unsigned long csr_read_num(unsigned long csr_num, int *out_err)
> +{
> +#define switchcase_csr_read(__csr_num) \
> + case (__csr_num): \
> + return csr_read(__csr_num)
> +#define switchcase_csr_read_2(__csr_num) \
> + switchcase_csr_read(__csr_num + 0); \
> + switchcase_csr_read(__csr_num + 1)
> +#define switchcase_csr_read_4(__csr_num) \
> + switchcase_csr_read_2(__csr_num + 0); \
> + switchcase_csr_read_2(__csr_num + 2)
> +#define switchcase_csr_read_8(__csr_num) \
> + switchcase_csr_read_4(__csr_num + 0); \
> + switchcase_csr_read_4(__csr_num + 4)
> +#define switchcase_csr_read_16(__csr_num) \
> + switchcase_csr_read_8(__csr_num + 0); \
> + switchcase_csr_read_8(__csr_num + 8)
> +#define switchcase_csr_read_32(__csr_num) \
> + switchcase_csr_read_16(__csr_num + 0); \
> + switchcase_csr_read_16(__csr_num + 16)
> +#define switchcase_csr_read_64(__csr_num) \
> + switchcase_csr_read_32(__csr_num + 0); \
> + switchcase_csr_read_32(__csr_num + 32)
> +#define switchcase_csr_read_128(__csr_num) \
> + switchcase_csr_read_64(__csr_num + 0); \
> + switchcase_csr_read_64(__csr_num + 64)
> +#define switchcase_csr_read_256(__csr_num) \
> + switchcase_csr_read_128(__csr_num + 0); \
> + switchcase_csr_read_128(__csr_num + 128)
> +
> + if (out_err)
> + *out_err = 0;
> +
> + switch (csr_num) {
> + switchcase_csr_read_32(CSR_CYCLE);
> + switchcase_csr_read_32(CSR_CYCLEH);
> + switchcase_csr_read_256(CSR_CUSTOM0_U_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM1_U_RO_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM2_S_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM3_S_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM4_S_RO_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM5_HS_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM6_HS_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM7_HS_RO_BASE);
> +#ifdef CONFIG_RISCV_M_MODE
> + switchcase_csr_read_64(CSR_CUSTOM8_M_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM9_M_RW_BASE);
> + switchcase_csr_read_64(CSR_CUSTOM10_M_RO_BASE);
> +#endif
> + default:
> + if (out_err)
> + *out_err = -EINVAL;
> + else
> + pr_err("%s: csr 0x%lx not supported\n", __func__, csr_num);
> + break;
> + }
> +
> + return 0;
> +#undef switchcase_csr_read_256
> +#undef switchcase_csr_read_128
> +#undef switchcase_csr_read_64
> +#undef switchcase_csr_read_32
> +#undef switchcase_csr_read_16
> +#undef switchcase_csr_read_8
> +#undef switchcase_csr_read_4
> +#undef switchcase_csr_read_2
> +#undef switchcase_csr_read
> +}
> +EXPORT_SYMBOL_GPL(csr_read_num);
> +
> +void csr_write_num(unsigned long csr_num, unsigned long val, int *out_err)
> +{
> +#define switchcase_csr_write(__csr_num, __val) \
> + case (__csr_num): \
> + csr_write(__csr_num, __val); \
> + break
> +#define switchcase_csr_write_2(__csr_num, __val) \
> + switchcase_csr_write(__csr_num + 0, __val); \
> + switchcase_csr_write(__csr_num + 1, __val)
> +#define switchcase_csr_write_4(__csr_num, __val) \
> + switchcase_csr_write_2(__csr_num + 0, __val); \
> + switchcase_csr_write_2(__csr_num + 2, __val)
> +#define switchcase_csr_write_8(__csr_num, __val) \
> + switchcase_csr_write_4(__csr_num + 0, __val); \
> + switchcase_csr_write_4(__csr_num + 4, __val)
> +#define switchcase_csr_write_16(__csr_num, __val) \
> + switchcase_csr_write_8(__csr_num + 0, __val); \
> + switchcase_csr_write_8(__csr_num + 8, __val)
> +#define switchcase_csr_write_32(__csr_num, __val) \
> + switchcase_csr_write_16(__csr_num + 0, __val); \
> + switchcase_csr_write_16(__csr_num + 16, __val)
> +#define switchcase_csr_write_64(__csr_num, __val) \
> + switchcase_csr_write_32(__csr_num + 0, __val); \
> + switchcase_csr_write_32(__csr_num + 32, __val)
> +#define switchcase_csr_write_128(__csr_num, __val) \
> + switchcase_csr_write_64(__csr_num + 0, __val); \
> + switchcase_csr_write_64(__csr_num + 64, __val)
> +#define switchcase_csr_write_256(__csr_num, __val) \
> + switchcase_csr_write_128(__csr_num + 0, __val); \
> + switchcase_csr_write_128(__csr_num + 128, __val)
> +
> + if (out_err)
> + *out_err = 0;
> +
> + switch (csr_num) {
> + switchcase_csr_write_256(CSR_CUSTOM0_U_RW_BASE, val);
> + switchcase_csr_write_64(CSR_CUSTOM2_S_RW_BASE, val);
> + switchcase_csr_write_64(CSR_CUSTOM3_S_RW_BASE, val);
> + switchcase_csr_write_64(CSR_CUSTOM5_HS_RW_BASE, val);
> + switchcase_csr_write_64(CSR_CUSTOM6_HS_RW_BASE, val);
> +#ifdef CONFIG_RISCV_M_MODE
> + switchcase_csr_write_64(CSR_CUSTOM8_M_RW_BASE, val);
> + switchcase_csr_write_64(CSR_CUSTOM9_M_RW_BASE, val);
> +#endif
> + default:
> + if (out_err)
> + *out_err = -EINVAL;
> + else
> + pr_err("%s: csr 0x%lx not supported\n", __func__, csr_num);
> + break;
> + }
> +#undef switchcase_csr_write_256
> +#undef switchcase_csr_write_128
> +#undef switchcase_csr_write_64
> +#undef switchcase_csr_write_32
> +#undef switchcase_csr_write_16
> +#undef switchcase_csr_write_8
> +#undef switchcase_csr_write_4
> +#undef switchcase_csr_write_2
> +#undef switchcase_csr_write
> +}
> +EXPORT_SYMBOL_GPL(csr_write_num);
> diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
> index 42c1a9052470..fe491937ed25 100644
> --- a/drivers/acpi/riscv/cppc.c
> +++ b/drivers/acpi/riscv/cppc.c
> @@ -65,24 +65,19 @@ static void sbi_cppc_write(void *write_data)
> static void cppc_ffh_csr_read(void *read_data)
> {
> struct sbi_cppc_data *data = (struct sbi_cppc_data *)read_data;
> + int err;
>
> - switch (data->reg) {
> - /* Support only TIME CSR for now */
> - case CSR_TIME:
> - data->ret.value = csr_read(CSR_TIME);
> - data->ret.error = 0;
> - break;
> - default:
> - data->ret.error = -EINVAL;
> - break;
> - }
> + data->ret.value = csr_read_num(data->reg, &err);
> + data->ret.error = err;
> }
>
> static void cppc_ffh_csr_write(void *write_data)
> {
> struct sbi_cppc_data *data = (struct sbi_cppc_data *)write_data;
> + int err;
>
> - data->ret.error = -EINVAL;
> + csr_write_num(data->reg, data->val, &err);
> + data->ret.error = err;
> }
>
> /*
> diff --git a/drivers/perf/riscv_pmu.c b/drivers/perf/riscv_pmu.c
> index 7644147d50b4..aa053254448d 100644
> --- a/drivers/perf/riscv_pmu.c
> +++ b/drivers/perf/riscv_pmu.c
> @@ -16,6 +16,7 @@
> #include <linux/smp.h>
> #include <linux/sched_clock.h>
>
> +#include <asm/csr.h>
> #include <asm/sbi.h>
>
> static bool riscv_perf_user_access(struct perf_event *event)
> @@ -88,46 +89,6 @@ void arch_perf_update_userpage(struct perf_event *event,
> userpg->cap_user_time_short = 1;
> }
>
> -static unsigned long csr_read_num(int csr_num)
> -{
> -#define switchcase_csr_read(__csr_num, __val) {\
> - case __csr_num: \
> - __val = csr_read(__csr_num); \
> - break; }
> -#define switchcase_csr_read_2(__csr_num, __val) {\
> - switchcase_csr_read(__csr_num + 0, __val) \
> - switchcase_csr_read(__csr_num + 1, __val)}
> -#define switchcase_csr_read_4(__csr_num, __val) {\
> - switchcase_csr_read_2(__csr_num + 0, __val) \
> - switchcase_csr_read_2(__csr_num + 2, __val)}
> -#define switchcase_csr_read_8(__csr_num, __val) {\
> - switchcase_csr_read_4(__csr_num + 0, __val) \
> - switchcase_csr_read_4(__csr_num + 4, __val)}
> -#define switchcase_csr_read_16(__csr_num, __val) {\
> - switchcase_csr_read_8(__csr_num + 0, __val) \
> - switchcase_csr_read_8(__csr_num + 8, __val)}
> -#define switchcase_csr_read_32(__csr_num, __val) {\
> - switchcase_csr_read_16(__csr_num + 0, __val) \
> - switchcase_csr_read_16(__csr_num + 16, __val)}
> -
> - unsigned long ret = 0;
> -
> - switch (csr_num) {
> - switchcase_csr_read_32(CSR_CYCLE, ret)
> - switchcase_csr_read_32(CSR_CYCLEH, ret)
> - default :
> - break;
> - }
> -
> - return ret;
> -#undef switchcase_csr_read_32
> -#undef switchcase_csr_read_16
> -#undef switchcase_csr_read_8
> -#undef switchcase_csr_read_4
> -#undef switchcase_csr_read_2
> -#undef switchcase_csr_read
> -}
> -
> /*
> * Read the CSR of a corresponding counter.
> */
> @@ -139,7 +100,7 @@ unsigned long riscv_pmu_ctr_read_csr(unsigned long csr)
> return -EINVAL;
> }
>
> - return csr_read_num(csr);
> + return csr_read_num(csr, NULL);
> }
>
> u64 riscv_pmu_ctr_get_width_mask(struct perf_event *event)
> --
> 2.43.0
>
Other than the suggestion to flip the interface,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2025-08-15 20:02 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-15 16:14 [PATCH 0/2] Common csr_read_num() and csr_write_num() for RISC-V Anup Patel
2025-08-15 16:14 ` Anup Patel
2025-08-15 16:14 ` [PATCH 1/2] ACPI: RISC-V: Fix FFH_CPPC_CSR error handling Anup Patel
2025-08-15 16:14 ` Anup Patel
2025-08-15 19:36 ` Andrew Jones
2025-08-15 19:36 ` Andrew Jones
2025-08-16 7:30 ` Troy Mitchell
2025-08-16 7:30 ` Troy Mitchell
2025-08-18 4:07 ` Sunil V L
2025-08-18 4:07 ` Sunil V L
2025-08-15 16:14 ` [PATCH 2/2] RISC-V: Add common csr_read_num() and csr_write_num() functions Anup Patel
2025-08-15 16:14 ` Anup Patel
2025-08-15 20:02 ` Andrew Jones [this message]
2025-08-15 20:02 ` Andrew Jones
2025-08-16 8:30 ` Anup Patel
2025-08-16 8:30 ` Anup Patel
2025-08-16 4:24 ` Vivian Wang
2025-08-16 4:24 ` Vivian Wang
2025-08-16 5:59 ` Vivian Wang
2025-08-16 5:59 ` Vivian Wang
2025-08-16 8:42 ` Anup Patel
2025-08-16 8:42 ` Anup Patel
2025-08-16 11:26 ` Vivian Wang
2025-08-16 11:26 ` Vivian Wang
2025-08-18 4:09 ` Sunil V L
2025-08-18 4:09 ` Sunil V L
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250815-e69bdc695cf78e6ce07e580c@orel \
--to=ajones@ventanamicro.com \
--cc=alex@ghiti.fr \
--cc=anup@brainfault.org \
--cc=apatel@ventanamicro.com \
--cc=atish.patra@linux.dev \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=rafael@kernel.org \
--cc=sunilvl@ventanamicro.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.