| Message ID | 20230130182225.2471414-21-sunilvl@ventanamicro.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | Add basic ACPI support for RISC-V | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Failed to apply to next/pending-fixes or riscv/for-next |
On Mon, Jan 30, 2023 at 11:52:21PM +0530, Sunil V L wrote: > On ACPI based platforms, few details like ISA need to be read > from the ACPI table. Enable cpuinfo on ACPI based systems. > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > --- > arch/riscv/kernel/cpu.c | 36 +++++++++++++++++++++++++++++------- > 1 file changed, 29 insertions(+), 7 deletions(-) > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > index 1b9a5a66e55a..bd6c0fcfe4ce 100644 > --- a/arch/riscv/kernel/cpu.c > +++ b/arch/riscv/kernel/cpu.c > @@ -3,6 +3,7 @@ > * Copyright (C) 2012 Regents of the University of California > */ > > +#include <linux/acpi.h> > #include <linux/cpu.h> > #include <linux/init.h> > #include <linux/seq_file.h> > @@ -256,26 +257,47 @@ static void c_stop(struct seq_file *m, void *v) > { > } > > +#ifdef CONFIG_ACPI > +void acpi_print_hart_info(struct seq_file *m, > + unsigned long cpu) Surely this fits on one line? > +{ > + const char *isa; > + > + if (!acpi_get_riscv_isa(NULL, get_acpi_id_for_cpu(cpu), &isa)) > + print_isa(m, isa); Do you really need to guard this function? Aren't there nop'ed versions of acpi_get_riscv_isa() and get_acpi_id_for_cpu() in acpi.h? IMO, basically any use of ifdeffery you can cleanly remove from a c file is a worthwhile change. > + Extra blank line here FYI. > +} > +#endif > + > static int c_show(struct seq_file *m, void *v) > { > unsigned long cpu_id = (unsigned long)v - 1; > - struct device_node *node = of_get_cpu_node(cpu_id, NULL); > struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id); > + struct device_node *node; > const char *compat, *isa; > > seq_printf(m, "processor\t: %lu\n", cpu_id); > seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id)); > - if (!of_property_read_string(node, "riscv,isa", &isa)) > - print_isa(m, isa); > + > + if (acpi_disabled) { > + node = of_get_cpu_node(cpu_id, NULL); > + if (!of_property_read_string(node, "riscv,isa", &isa)) > + print_isa(m, isa); > + if (!of_property_read_string(node, "compatible", &compat) > + && strcmp(compat, "riscv")) ^^ this should be on the line above TBH the whole series is in need of a checkpatch --strict run IMO, there's a bunch of coding style issues throughout. > + seq_printf(m, "uarch\t\t: %s\n", compat); > + of_node_put(node); > + } > +#ifdef CONFIG_ACPI > + else > + acpi_print_hart_info(m, cpu_id); Delete the ifdeffery here too please :) Cheers, Conor. > +#endif > + > print_mmu(m); > - if (!of_property_read_string(node, "compatible", &compat) > - && strcmp(compat, "riscv")) > - seq_printf(m, "uarch\t\t: %s\n", compat); > seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid); > seq_printf(m, "marchid\t\t: 0x%lx\n", ci->marchid); > seq_printf(m, "mimpid\t\t: 0x%lx\n", ci->mimpid); > seq_puts(m, "\n"); > - of_node_put(node); > > return 0; > } > -- > 2.38.0 >
On Thu, Feb 09, 2023 at 09:13:37PM +0000, Conor Dooley wrote: > On Mon, Jan 30, 2023 at 11:52:21PM +0530, Sunil V L wrote: > > On ACPI based platforms, few details like ISA need to be read > > from the ACPI table. Enable cpuinfo on ACPI based systems. > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > > --- > > arch/riscv/kernel/cpu.c | 36 +++++++++++++++++++++++++++++------- > > 1 file changed, 29 insertions(+), 7 deletions(-) > > > > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c > > index 1b9a5a66e55a..bd6c0fcfe4ce 100644 > > --- a/arch/riscv/kernel/cpu.c > > +++ b/arch/riscv/kernel/cpu.c > > @@ -3,6 +3,7 @@ > > * Copyright (C) 2012 Regents of the University of California > > */ > > > > +#include <linux/acpi.h> > > #include <linux/cpu.h> > > #include <linux/init.h> > > #include <linux/seq_file.h> > > @@ -256,26 +257,47 @@ static void c_stop(struct seq_file *m, void *v) > > { > > } > > > > +#ifdef CONFIG_ACPI > > +void acpi_print_hart_info(struct seq_file *m, > > + unsigned long cpu) > > Surely this fits on one line? > Okay > > +{ > > + const char *isa; > > + > > + if (!acpi_get_riscv_isa(NULL, get_acpi_id_for_cpu(cpu), &isa)) > > + print_isa(m, isa); > > Do you really need to guard this function? Aren't there nop'ed versions > of acpi_get_riscv_isa() and get_acpi_id_for_cpu() in acpi.h? > > IMO, basically any use of ifdeffery you can cleanly remove from a c file > is a worthwhile change. > You are right. Let me remove ifdef. > > + > > Extra blank line here FYI. > > > +} > > +#endif > > + > > static int c_show(struct seq_file *m, void *v) > > { > > unsigned long cpu_id = (unsigned long)v - 1; > > - struct device_node *node = of_get_cpu_node(cpu_id, NULL); > > struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id); > > + struct device_node *node; > > const char *compat, *isa; > > > > seq_printf(m, "processor\t: %lu\n", cpu_id); > > seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id)); > > - if (!of_property_read_string(node, "riscv,isa", &isa)) > > - print_isa(m, isa); > > + > > + if (acpi_disabled) { > > + node = of_get_cpu_node(cpu_id, NULL); > > + if (!of_property_read_string(node, "riscv,isa", &isa)) > > + print_isa(m, isa); > > + if (!of_property_read_string(node, "compatible", &compat) > > + && strcmp(compat, "riscv")) > ^^ this should be on the line above > TBH the whole series is in need of a checkpatch --strict run IMO, > there's a bunch of coding style issues throughout. > I just moved this line as is. Sure, let me fix it. Thanks. > > + seq_printf(m, "uarch\t\t: %s\n", compat); > > + of_node_put(node); > > + } > > +#ifdef CONFIG_ACPI > > + else > > + acpi_print_hart_info(m, cpu_id); > > Delete the ifdeffery here too please :) > Okay Thanks, Sunil
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index 1b9a5a66e55a..bd6c0fcfe4ce 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -3,6 +3,7 @@ * Copyright (C) 2012 Regents of the University of California */ +#include <linux/acpi.h> #include <linux/cpu.h> #include <linux/init.h> #include <linux/seq_file.h> @@ -256,26 +257,47 @@ static void c_stop(struct seq_file *m, void *v) { } +#ifdef CONFIG_ACPI +void acpi_print_hart_info(struct seq_file *m, + unsigned long cpu) +{ + const char *isa; + + if (!acpi_get_riscv_isa(NULL, get_acpi_id_for_cpu(cpu), &isa)) + print_isa(m, isa); + +} +#endif + static int c_show(struct seq_file *m, void *v) { unsigned long cpu_id = (unsigned long)v - 1; - struct device_node *node = of_get_cpu_node(cpu_id, NULL); struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id); + struct device_node *node; const char *compat, *isa; seq_printf(m, "processor\t: %lu\n", cpu_id); seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id)); - if (!of_property_read_string(node, "riscv,isa", &isa)) - print_isa(m, isa); + + if (acpi_disabled) { + node = of_get_cpu_node(cpu_id, NULL); + if (!of_property_read_string(node, "riscv,isa", &isa)) + print_isa(m, isa); + if (!of_property_read_string(node, "compatible", &compat) + && strcmp(compat, "riscv")) + seq_printf(m, "uarch\t\t: %s\n", compat); + of_node_put(node); + } +#ifdef CONFIG_ACPI + else + acpi_print_hart_info(m, cpu_id); +#endif + print_mmu(m); - if (!of_property_read_string(node, "compatible", &compat) - && strcmp(compat, "riscv")) - seq_printf(m, "uarch\t\t: %s\n", compat); seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid); seq_printf(m, "marchid\t\t: 0x%lx\n", ci->marchid); seq_printf(m, "mimpid\t\t: 0x%lx\n", ci->mimpid); seq_puts(m, "\n"); - of_node_put(node); return 0; }
On ACPI based platforms, few details like ISA need to be read from the ACPI table. Enable cpuinfo on ACPI based systems. Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> --- arch/riscv/kernel/cpu.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-)