[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <012aaa39-a37b-e682-0e34-9b7d7cd87f75@kernel.org>
Date: Wed, 29 Oct 2025 13:04:59 -0600 (MDT)
From: Paul Walmsley <pjw@...nel.org>
To: Anup Patel <apatel@...tanamicro.com>
cc: Sunil V L <sunilvl@...tanamicro.com>, 
    "Rafael J . Wysocki" <rafael@...nel.org>, 
    Palmer Dabbelt <palmer@...belt.com>, 
    Paul Walmsley <paul.walmsley@...ive.com>, Alexandre Ghiti <alex@...ti.fr>, 
    Len Brown <lenb@...nel.org>, Atish Patra <atish.patra@...ux.dev>, 
    Andrew Jones <ajones@...tanamicro.com>, Anup Patel <anup@...infault.org>, 
    Will Deacon <will@...nel.org>, Mark Rutland <mark.rutland@....com>, 
    linux-acpi@...r.kernel.org, linux-riscv@...ts.infradead.org, 
    linux-kernel@...r.kernel.org, Atish Patra <atishp@...osinc.com>, 
    Nutty Liu <nutty.liu@...mail.com>
Subject: Re: [PATCH v3 1/1] RISC-V: Add common csr_read_num() and csr_write_num()
 functions
Hi Anup,
On Tue, 14 Oct 2025, 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@...tanamicro.com>
> Reviewed-by: Sunil V L <sunilvl@...tanamicro.com>
> Reviewed-by: Andrew Jones <ajones@...tanamicro.com>
> Reviewed-by: Atish Patra <atishp@...osinc.com>
> Reviewed-by: Nutty Liu <nutty.liu@...mail.com>
This patch also (silently) removes the CSR number filtering, e.g.
> 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;
> -	}
... the above code, and:
>  /*
>   * Read the CSR of a corresponding counter.
>   */
>  unsigned long riscv_pmu_ctr_read_csr(unsigned long csr)
>  {
> -	if (csr < CSR_CYCLE || csr > CSR_HPMCOUNTER31H ||
> -	   (csr > CSR_HPMCOUNTER31 && csr < CSR_CYCLEH)) {
> -		pr_err("Invalid performance counter csr %lx\n", csr);
> -		return -EINVAL;
... the above code.
I'm thinking that we probably want to keep the CSR number filtering code 
in; at least, I can't think of a good reason to remove it.  Care to add it 
back in?
- Paul
Powered by blists - more mailing lists
 
