[PATCH 0/2] Common csr_read_num() and csr_write_num() for RISC-V

[PATCH 0/2] Common csr_read_num() and csr_write_num() for RISC-V

[Anup Patel]

Some of the RISC-V drivers (such as RISC-V PMU and ACPI CPPC) need to
access CSR based on CSR number discovered from somewhere. Add common
RISC-V csr_read_num() and csr_write_num() functions under arch/riscv
for such drivers.

These patches can be found in the riscv_csr_read_num_v1 branch at:
https://github.com/avpatel/linux.git

Anup Patel (2):
  ACPI: RISC-V: Fix FFH_CPPC_CSR error handling
  RISC-V: Add common csr_read_num() and csr_write_num() functions

 arch/riscv/include/asm/csr.h |   3 +
 arch/riscv/kernel/Makefile   |   1 +
 arch/riscv/kernel/csr.c      | 177 +++++++++++++++++++++++++++++++++++
 drivers/acpi/riscv/cppc.c    |  21 ++---
 drivers/perf/riscv_pmu.c     |  43 +--------
 5 files changed, 191 insertions(+), 54 deletions(-)
 create mode 100644 arch/riscv/kernel/csr.c

-- 
2.43.0

[PATCH 1/2] ACPI: RISC-V: Fix FFH_CPPC_CSR error handling

[Anup Patel]

The cppc_ffh_csr_read() and cppc_ffh_csr_write() returns Linux error
code in "data->ret.error" so cpc_read_ffh() and cpc_write_ffh() must
not use sbi_err_map_linux_errno() for FFH_CPPC_CSR.

Fixes: 30f3ffbee86b ("ACPI: RISC-V: Add CPPC driver")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
---
 drivers/acpi/riscv/cppc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
index 440cf9fb91aa..42c1a9052470 100644
--- a/drivers/acpi/riscv/cppc.c
+++ b/drivers/acpi/riscv/cppc.c
@@ -119,7 +119,7 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
 
 		*val = data.ret.value;
 
-		return (data.ret.error) ? sbi_err_map_linux_errno(data.ret.error) : 0;
+		return data.ret.error;
 	}
 
 	return -EINVAL;
@@ -148,7 +148,7 @@ int cpc_write_ffh(int cpu, struct cpc_reg *reg, u64 val)
 
 		smp_call_function_single(cpu, cppc_ffh_csr_write, &data, 1);
 
-		return (data.ret.error) ? sbi_err_map_linux_errno(data.ret.error) : 0;
+		return data.ret.error;
 	}
 
 	return -EINVAL;
-- 
2.43.0

[PATCH 2/2] RISC-V: Add common csr_read_num() and csr_write_num() functions

[Anup Patel]

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);
+
 #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

Re: [PATCH 1/2] ACPI: RISC-V: Fix FFH_CPPC_CSR error handling

[Andrew Jones]

On Fri, Aug 15, 2025 at 09:44:05PM +0530, Anup Patel wrote:
> The cppc_ffh_csr_read() and cppc_ffh_csr_write() returns Linux error
> code in "data->ret.error" so cpc_read_ffh() and cpc_write_ffh() must
> not use sbi_err_map_linux_errno() for FFH_CPPC_CSR.
> 
> Fixes: 30f3ffbee86b ("ACPI: RISC-V: Add CPPC driver")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  drivers/acpi/riscv/cppc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
> index 440cf9fb91aa..42c1a9052470 100644
> --- a/drivers/acpi/riscv/cppc.c
> +++ b/drivers/acpi/riscv/cppc.c
> @@ -119,7 +119,7 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
>  
>  		*val = data.ret.value;
>  
> -		return (data.ret.error) ? sbi_err_map_linux_errno(data.ret.error) : 0;
> +		return data.ret.error;
>  	}
>  
>  	return -EINVAL;
> @@ -148,7 +148,7 @@ int cpc_write_ffh(int cpu, struct cpc_reg *reg, u64 val)
>  
>  		smp_call_function_single(cpu, cppc_ffh_csr_write, &data, 1);
>  
> -		return (data.ret.error) ? sbi_err_map_linux_errno(data.ret.error) : 0;
> +		return data.ret.error;
>  	}
>  
>  	return -EINVAL;
> -- 
> 2.43.0
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Re: [PATCH 2/2] RISC-V: Add common csr_read_num() and csr_write_num() functions

[Andrew Jones]

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>

Re: [PATCH 2/2] RISC-V: Add common csr_read_num() and csr_write_num() functions

[Vivian Wang]

On 8/16/25 00:14, 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);
> +
>  #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)
> +

That's... a bit horrendous.

Since we know that each inner case is quite simple, can we just do our
own jump table for that? Each case can be one csrr/csrw and one jal
(possibly pad to 8 bytes), and we can just jump to
label + (csr_num - range_start) * 8. See bottom of this message for an
untested prototype.

Vivian "dramforever" Wang

UNTESTED prototype:

#define CSR_CYCLE 0xc00
#define CSR_CYCLEH 0xc80

/* Force expand argument if macro */
#define str(_x) #_x

unsigned long csr_read_num(long csr_num)
{
	unsigned long res, tmp;

        /*
	 * Generate a jump table for each range. Each (inner) case is 8 bytes of
	 * code, a csrr instruction and a jump, possibly padded, so we just jump
	 * to label_1f + (csr_num - _start) * 8
	 */
#define csr_read_case_range(_start, _size) 					\
	case (_start) ... ((_start) + (_size) - 1):				\
		asm volatile (							\
				"	lla %[tmp], 1f\n"			\
				"	add %[tmp], %[tmp], %[offset]\n"	\
				"	jr %[tmp]\n"				\
				"1:\n"						\
				"	.rept (" str(_size) ")\n"		\
				"	csrr %[res], (" str(_start) " + \\+)\n"	\
				"	j 2f\n"					\
				"	.balign 4\n"				\
				"	.endr\n"				\
				"2:\n"						\
				: [tmp] "=&r"(tmp), [res] "=r"(res)		\
				: [offset] "r"((csr_num - _start) * 8)		\
				: );						\
		break;								\


	switch (csr_num) {
	csr_read_case_range(CSR_CYCLE, 4)
	csr_read_case_range(CSR_CYCLEH, 4)

	/* ... other cases */

	default:
		/* Error handling */
	}

#undef csr_read_case_range

	return res;
}

Re: [PATCH 2/2] RISC-V: Add common csr_read_num() and csr_write_num() functions

[Vivian Wang]

On 8/16/25 12:24, Vivian Wang wrote:
> On 8/16/25 00:14, 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);
>> +
>>  #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)
>> +
> That's... a bit horrendous.
>
> Since we know that each inner case is quite simple, can we just do our
> own jump table for that? Each case can be one csrr/csrw and one jal
> (possibly pad to 8 bytes), and we can just jump to
> label + (csr_num - range_start) * 8. See bottom of this message for an
> untested prototype.
>
> Vivian "dramforever" Wang
>
> UNTESTED prototype:

I want to make it clear by the way that I *don't* think the details are
right with this code. Please do fix stuff like macro parameter parens
and formatting if *anyone* is trying to use this code.

Vivian "dramforever" Wang

> #define CSR_CYCLE 0xc00
> #define CSR_CYCLEH 0xc80
>
> /* Force expand argument if macro */
> #define str(_x) #_x
>
> unsigned long csr_read_num(long csr_num)
> {
>
> [...]

Re: [PATCH 1/2] ACPI: RISC-V: Fix FFH_CPPC_CSR error handling

[Troy Mitchell]

On Fri, Aug 15, 2025 at 09:44:05PM +0530, Anup Patel wrote:
> The cppc_ffh_csr_read() and cppc_ffh_csr_write() returns Linux error
> code in "data->ret.error" so cpc_read_ffh() and cpc_write_ffh() must
> not use sbi_err_map_linux_errno() for FFH_CPPC_CSR.
> 
> Fixes: 30f3ffbee86b ("ACPI: RISC-V: Add CPPC driver")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  drivers/acpi/riscv/cppc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
> index 440cf9fb91aa..42c1a9052470 100644
> --- a/drivers/acpi/riscv/cppc.c
> +++ b/drivers/acpi/riscv/cppc.c
> @@ -119,7 +119,7 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
>  
>  		*val = data.ret.value;
>  
> -		return (data.ret.error) ? sbi_err_map_linux_errno(data.ret.error) : 0;
> +		return data.ret.error;
>  	}
>  
>  	return -EINVAL;
> @@ -148,7 +148,7 @@ int cpc_write_ffh(int cpu, struct cpc_reg *reg, u64 val)
>  
>  		smp_call_function_single(cpu, cppc_ffh_csr_write, &data, 1);
>  
> -		return (data.ret.error) ? sbi_err_map_linux_errno(data.ret.error) : 0;
> +		return data.ret.error;
>  	}
>  
>  	return -EINVAL;
>
Thanks!

Reviewed-by: Troy Mitchell <troy.mitchell@linux.dev>
> -- 
> 2.43.0
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 2/2] RISC-V: Add common csr_read_num() and csr_write_num() functions

[Anup Patel]

On Sat, Aug 16, 2025 at 1:32 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> 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.

I had considered this but the problem with this approach is that
individual switch cases in csr_read_num() become roughly 4
instructions because value read from CSR has to written to a
memory location.

The current approach results in just 2 instructions for each
switch-case. Additionally, the current prototypes of csr_read_num()
and csr_write_num() are closer to csr_read() and csr_write()
respectively.

>
> > +
> >  #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>

Regards,
Anup

Re: [PATCH 2/2] RISC-V: Add common csr_read_num() and csr_write_num() functions

[Anup Patel]

On Sat, Aug 16, 2025 at 9:54 AM Vivian Wang <wangruikang@iscas.ac.cn> wrote:
>
>
> On 8/16/25 00:14, 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);
> > +
> >  #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)
> > +
>
> That's... a bit horrendous.
>
> Since we know that each inner case is quite simple, can we just do our
> own jump table for that? Each case can be one csrr/csrw and one jal
> (possibly pad to 8 bytes), and we can just jump to
> label + (csr_num - range_start) * 8. See bottom of this message for an
> untested prototype.
>
> Vivian "dramforever" Wang
>
> UNTESTED prototype:
>
> #define CSR_CYCLE 0xc00
> #define CSR_CYCLEH 0xc80
>
> /* Force expand argument if macro */
> #define str(_x) #_x
>
> unsigned long csr_read_num(long csr_num)
> {
>         unsigned long res, tmp;
>
>         /*
>          * Generate a jump table for each range. Each (inner) case is 8 bytes of
>          * code, a csrr instruction and a jump, possibly padded, so we just jump
>          * to label_1f + (csr_num - _start) * 8
>          */
> #define csr_read_case_range(_start, _size)                                      \
>         case (_start) ... ((_start) + (_size) - 1):                             \
>                 asm volatile (                                                  \
>                                 "       lla %[tmp], 1f\n"                       \
>                                 "       add %[tmp], %[tmp], %[offset]\n"        \
>                                 "       jr %[tmp]\n"                            \
>                                 "1:\n"                                          \
>                                 "       .rept (" str(_size) ")\n"               \
>                                 "       csrr %[res], (" str(_start) " + \\+)\n" \
>                                 "       j 2f\n"                                 \
>                                 "       .balign 4\n"                            \
>                                 "       .endr\n"                                \
>                                 "2:\n"                                          \
>                                 : [tmp] "=&r"(tmp), [res] "=r"(res)             \
>                                 : [offset] "r"((csr_num - _start) * 8)          \
>                                 : );                                            \
>                 break;                                                          \
>
>
>         switch (csr_num) {
>         csr_read_case_range(CSR_CYCLE, 4)
>         csr_read_case_range(CSR_CYCLEH, 4)
>
>         /* ... other cases */
>
>         default:
>                 /* Error handling */
>         }
>
> #undef csr_read_case_range
>
>         return res;
> }

Clearly you have trust issues with compiler switch-case optimizations
and your proposed solution looks horrendous to me.

Your proposed solution is effectively a nested switch case where
there is a higher-level jump-table generated by the compiler which
selects a range case and within each range case you have a hand
assembled jump table. If you analyse the effective number of
instructions involved in accessing an CSR then it will be much
more than the current approach.

The current approach relies on a compiler optimized jump table
where each switch-case is just two instructions (12 bytes) for
both csr_read_num() and csr_write_num().

Regards,
Anup

Re: [PATCH 2/2] RISC-V: Add common csr_read_num() and csr_write_num() functions

[Vivian Wang]

On 8/16/25 16:42, Anup Patel wrote:
> On Sat, Aug 16, 2025 at 9:54 AM Vivian Wang <wangruikang@iscas.ac.cn> wrote:
>>
>> On 8/16/25 00:14, 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);
>>> +
>>>  #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)
>>> +
>> That's... a bit horrendous.
>>
>> Since we know that each inner case is quite simple, can we just do our
>> own jump table for that? Each case can be one csrr/csrw and one jal
>> (possibly pad to 8 bytes), and we can just jump to
>> label + (csr_num - range_start) * 8. See bottom of this message for an
>> untested prototype.
>>
>> Vivian "dramforever" Wang
>>
>> UNTESTED prototype:
>>
>> #define CSR_CYCLE 0xc00
>> #define CSR_CYCLEH 0xc80
>>
>> /* Force expand argument if macro */
>> #define str(_x) #_x
>>
>> unsigned long csr_read_num(long csr_num)
>> {
>>         unsigned long res, tmp;
>>
>>         /*
>>          * Generate a jump table for each range. Each (inner) case is 8 bytes of
>>          * code, a csrr instruction and a jump, possibly padded, so we just jump
>>          * to label_1f + (csr_num - _start) * 8
>>          */
>> #define csr_read_case_range(_start, _size)                                      \
>>         case (_start) ... ((_start) + (_size) - 1):                             \
>>                 asm volatile (                                                  \
>>                                 "       lla %[tmp], 1f\n"                       \
>>                                 "       add %[tmp], %[tmp], %[offset]\n"        \
>>                                 "       jr %[tmp]\n"                            \
>>                                 "1:\n"                                          \
>>                                 "       .rept (" str(_size) ")\n"               \
>>                                 "       csrr %[res], (" str(_start) " + \\+)\n" \
>>                                 "       j 2f\n"                                 \
>>                                 "       .balign 4\n"                            \
>>                                 "       .endr\n"                                \
>>                                 "2:\n"                                          \
>>                                 : [tmp] "=&r"(tmp), [res] "=r"(res)             \
>>                                 : [offset] "r"((csr_num - _start) * 8)          \
>>                                 : );                                            \
>>                 break;                                                          \
>>
>>
>>         switch (csr_num) {
>>         csr_read_case_range(CSR_CYCLE, 4)
>>         csr_read_case_range(CSR_CYCLEH, 4)
>>
>>         /* ... other cases */
>>
>>         default:
>>                 /* Error handling */
>>         }
>>
>> #undef csr_read_case_range
>>
>>         return res;
>> }
> Clearly you have trust issues with compiler switch-case optimizations
> and your proposed solution looks horrendous to me.

What I am concerned about is less whether the compiler generates better
code, but more that you would need to build out these macros to write
out the cases. I was trying to make it so that you can just have one
macro that would work for any (constant) range.

> Your proposed solution is effectively a nested switch case where
> there is a higher-level jump-table generated by the compiler which
> selects a range case and within each range case you have a hand
> assembled jump table. If you analyse the effective number of
> instructions involved in accessing an CSR then it will be much
> more than the current approach.

If you think it is more important that this is all handled within one
jump table for the fastest generated code possible, no problem.

Vivian "dramforever" Wang

> The current approach relies on a compiler optimized jump table
> where each switch-case is just two instructions (12 bytes) for
> both csr_read_num() and csr_write_num().
>
> Regards,
> Anup

Re: [PATCH 1/2] ACPI: RISC-V: Fix FFH_CPPC_CSR error handling

[Sunil V L]

On Fri, Aug 15, 2025 at 09:44:05PM +0530, Anup Patel wrote:
> The cppc_ffh_csr_read() and cppc_ffh_csr_write() returns Linux error
> code in "data->ret.error" so cpc_read_ffh() and cpc_write_ffh() must
> not use sbi_err_map_linux_errno() for FFH_CPPC_CSR.
> 
> Fixes: 30f3ffbee86b ("ACPI: RISC-V: Add CPPC driver")
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  drivers/acpi/riscv/cppc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
> index 440cf9fb91aa..42c1a9052470 100644
> --- a/drivers/acpi/riscv/cppc.c
> +++ b/drivers/acpi/riscv/cppc.c
> @@ -119,7 +119,7 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
>  
>  		*val = data.ret.value;
>  
> -		return (data.ret.error) ? sbi_err_map_linux_errno(data.ret.error) : 0;
> +		return data.ret.error;
>  	}
>  
>  	return -EINVAL;
> @@ -148,7 +148,7 @@ int cpc_write_ffh(int cpu, struct cpc_reg *reg, u64 val)
>  
>  		smp_call_function_single(cpu, cppc_ffh_csr_write, &data, 1);
>  
> -		return (data.ret.error) ? sbi_err_map_linux_errno(data.ret.error) : 0;
> +		return data.ret.error;
>  	}
>  
>  	return -EINVAL;
> -- 
> 2.43.0
> 
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>

Re: [PATCH 2/2] RISC-V: Add common csr_read_num() and csr_write_num() functions

[Sunil V L]

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);
> +
>  #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);
>
I think it is better to pass valid error pointer here.

>  }
>  
>  u64 riscv_pmu_ctr_get_width_mask(struct perf_event *event)
> -- 
> 2.43.0
> 

Otherwise, LGTM.

Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>