| Message ID | 20230803175916.3174453-10-sunilvl@ventanamicro.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RISC-V: ACPI: Add external interrupt controller support | expand |
On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote: > Using new interface to get the CBO block size information in > RHCT, initialize the variables on ACPI platforms. ... > #include <linux/of.h> > +#include <linux/acpi.h> Can you keep it sorted (to some extent)? > +#include <asm/acpi.h> What do you need this for? > #include <asm/cacheflush.h>
On Fri, Aug 04, 2023 at 08:56:29AM +0300, Andy Shevchenko wrote: > On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote: > > Using new interface to get the CBO block size information in > > RHCT, initialize the variables on ACPI platforms. > > ... > > > #include <linux/of.h> > > +#include <linux/acpi.h> > > Can you keep it sorted (to some extent)? > Sure. > > +#include <asm/acpi.h> > > What do you need this for? > > > #include <asm/cacheflush.h> > When CONFIG_ACPI is disabled, this include is required to get acpi_get_cbo_block_size(). Thanks, Sunil
On Fri, Aug 04, 2023 at 02:50:34PM +0530, Sunil V L wrote: > On Fri, Aug 04, 2023 at 08:56:29AM +0300, Andy Shevchenko wrote: > > On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote: > > > Using new interface to get the CBO block size information in > > > RHCT, initialize the variables on ACPI platforms. ... > > > +#include <asm/acpi.h> > > > > What do you need this for? > > > > > #include <asm/cacheflush.h> > > > When CONFIG_ACPI is disabled, this include is required to get > acpi_get_cbo_block_size(). How is it useful without ACPI being enabled? If it's indeed (in which I do not believe), better to make sure you have it avaiable independently on CONFIG_ACPI. Otherwise, just put #ifdef CONFIG_ACPI around the call.
On Fri, Aug 04, 2023 at 05:59:51PM +0300, Andy Shevchenko wrote: > On Fri, Aug 04, 2023 at 02:50:34PM +0530, Sunil V L wrote: > > On Fri, Aug 04, 2023 at 08:56:29AM +0300, Andy Shevchenko wrote: > > > On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote: > > > > Using new interface to get the CBO block size information in > > > > RHCT, initialize the variables on ACPI platforms. > > ... > > > > > +#include <asm/acpi.h> > > > > > > What do you need this for? > > > > > > > #include <asm/cacheflush.h> > > > > > When CONFIG_ACPI is disabled, this include is required to get > > acpi_get_cbo_block_size(). > > How is it useful without ACPI being enabled? It is not, as evidenced by the `return -EINVAL;`. > If it's indeed > (in which I do not believe), better to make sure you have it > avaiable independently on CONFIG_ACPI. Otherwise, just put > #ifdef CONFIG_ACPI around the call. Let's not litter the code with ifdeffery please where it can be easily avoided.
On Fri, Aug 04, 2023 at 04:19:27PM +0100, Conor Dooley wrote: > On Fri, Aug 04, 2023 at 05:59:51PM +0300, Andy Shevchenko wrote: > > On Fri, Aug 04, 2023 at 02:50:34PM +0530, Sunil V L wrote: > > > On Fri, Aug 04, 2023 at 08:56:29AM +0300, Andy Shevchenko wrote: > > > > On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote: ... > > > > > +#include <asm/acpi.h> > > > > > > > > What do you need this for? > > > > > > > > > #include <asm/cacheflush.h> > > > > > > > When CONFIG_ACPI is disabled, this include is required to get > > > acpi_get_cbo_block_size(). > > > > How is it useful without ACPI being enabled? > > It is not, as evidenced by the `return -EINVAL;`. > > > If it's indeed > > (in which I do not believe), better to make sure you have it > > avaiable independently on CONFIG_ACPI. Otherwise, just put > > #ifdef CONFIG_ACPI around the call. > > Let's not litter the code with ifdeffery please where it can be easily > avoided. Including asm/acpi.h looks to me as a "let's avoid it with a hack that it is uglier than ifdeffery". Sorry, but ifdeffery for ACPI, with all my full agreement with the statement that it's not good, is the correct way to fix this.
On Fri, Aug 04, 2023 at 07:52:56PM +0300, Andy Shevchenko wrote: > On Fri, Aug 04, 2023 at 04:19:27PM +0100, Conor Dooley wrote: > > On Fri, Aug 04, 2023 at 05:59:51PM +0300, Andy Shevchenko wrote: > > > On Fri, Aug 04, 2023 at 02:50:34PM +0530, Sunil V L wrote: > > > > On Fri, Aug 04, 2023 at 08:56:29AM +0300, Andy Shevchenko wrote: > > > > > On Thu, Aug 03, 2023 at 11:29:04PM +0530, Sunil V L wrote: ... > > > > > > +#include <asm/acpi.h> > > > > > > > > > > What do you need this for? > > > > > > > > > > > #include <asm/cacheflush.h> > > > > > > > > > When CONFIG_ACPI is disabled, this include is required to get > > > > acpi_get_cbo_block_size(). > > > > > > How is it useful without ACPI being enabled? > > > > It is not, as evidenced by the `return -EINVAL;`. > > > > > If it's indeed > > > (in which I do not believe), better to make sure you have it > > > avaiable independently on CONFIG_ACPI. Otherwise, just put > > > #ifdef CONFIG_ACPI around the call. > > > > Let's not litter the code with ifdeffery please where it can be easily > > avoided. > > Including asm/acpi.h looks to me as a "let's avoid it with a hack that it > is uglier than ifdeffery". Sorry, but ifdeffery for ACPI, with all my full > agreement with the statement that it's not good, is the correct way to fix > this. On the other hand this is an arch code and I see precedents of using the headers together, alas, it seems not better to me that ugly ifdeffery. So, I leave it to the respective maintainers to decide.
diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c index fbc59b3f69f2..63bb56819b37 100644 --- a/arch/riscv/mm/cacheflush.c +++ b/arch/riscv/mm/cacheflush.c @@ -4,6 +4,8 @@ */ #include <linux/of.h> +#include <linux/acpi.h> +#include <asm/acpi.h> #include <asm/cacheflush.h> #ifdef CONFIG_SMP @@ -131,15 +133,38 @@ void __init riscv_init_cbo_blocksizes(void) unsigned long cbom_hartid, cboz_hartid; u32 cbom_block_size = 0, cboz_block_size = 0; struct device_node *node; + struct acpi_table_header *rhct; + acpi_status status; + unsigned int cpu; + + if (!acpi_disabled) { + status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct); + if (ACPI_FAILURE(status)) + return; + } - for_each_of_cpu_node(node) { - /* set block-size for cbom and/or cboz extension if available */ - cbo_get_block_size(node, "riscv,cbom-block-size", - &cbom_block_size, &cbom_hartid); - cbo_get_block_size(node, "riscv,cboz-block-size", - &cboz_block_size, &cboz_hartid); + for_each_possible_cpu(cpu) { + if (acpi_disabled) { + node = of_cpu_device_node_get(cpu); + if (!node) { + pr_warn("Unable to find cpu node\n"); + continue; + } + + /* set block-size for cbom and/or cboz extension if available */ + cbo_get_block_size(node, "riscv,cbom-block-size", + &cbom_block_size, &cbom_hartid); + cbo_get_block_size(node, "riscv,cboz-block-size", + &cboz_block_size, &cboz_hartid); + } else { + acpi_get_cbo_block_size(rhct, cpu, &cbom_block_size, + &cboz_block_size, NULL); + } } + if (!acpi_disabled && rhct) + acpi_put_table((struct acpi_table_header *)rhct); + if (cbom_block_size) riscv_cbom_block_size = cbom_block_size;
Using new interface to get the CBO block size information in RHCT, initialize the variables on ACPI platforms. Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> --- arch/riscv/mm/cacheflush.c | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-)