[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZAhYeuCmdYAnanNv@sunil-laptop>
Date: Wed, 8 Mar 2023 15:12:18 +0530
From: Sunil V L <sunilvl@...tanamicro.com>
To: Conor Dooley <conor@...nel.org>
Cc: linux-riscv@...ts.infradead.org, linux-acpi@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Len Brown <lenb@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <maz@...nel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Jonathan Corbet <corbet@....net>,
Anup Patel <apatel@...tanamicro.com>,
Andrew Jones <ajones@...tanamicro.com>,
Atish Patra <atishp@...osinc.com>,
'Conor Dooley ' <conor.dooley@...rochip.com>,
"Rafael J . Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH V3 18/20] RISC-V: Add ACPI initialization in setup_arch()
On Mon, Mar 06, 2023 at 09:17:34PM +0000, Conor Dooley wrote:
> On Fri, Mar 03, 2023 at 07:06:45PM +0530, Sunil V L wrote:
> > Initialize the ACPI core for RISC-V during boot.
> >
> > ACPI tables and interpreter are initialized based on
> > the information passed from the firmware and the value of
> > the kernel parameter 'acpi'.
> >
> > With ACPI support added for RISC-V, the kernel parameter 'acpi'
> > is also supported on RISC-V. Hence, update the documentation.
> >
> > Signed-off-by: Sunil V L <sunilvl@...tanamicro.com>
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > Reviewed-by: Andrew Jones <ajones@...tanamicro.com>
> > ---
>
> > +static int __init acpi_fadt_sanity_check(void)
> > +{
> > + struct acpi_table_header *table;
> > + struct acpi_table_fadt *fadt;
> > + acpi_status status;
> > + int ret = 0;
> > +
> > + /*
> > + * FADT is required on riscv; retrieve it to check its presence
> > + * and carry out revision and ACPI HW reduced compliancy tests
> > + */
> > + status = acpi_get_table(ACPI_SIG_FADT, 0, &table);
> > + if (ACPI_FAILURE(status)) {
> > + const char *msg = acpi_format_exception(status);
> > +
> > + pr_err("Failed to get FADT table, %s\n", msg);
> > + return -ENODEV;
> > + }
> > +
> > + fadt = (struct acpi_table_fadt *)table;
> > +
> > + /*
> > + * Revision in table header is the FADT Major revision, and there
> > + * is a minor revision of FADT.
>
> What is the point of this part of the comment? Isn't it obvious from the
> below code that you expect a major and minor revision?
> If feel like you're trying to make a point in it, but the point has been
> lost :/
>
It just highlights that major and minor revision fields are in two
different places. Let me remove this comment since it is part of the
spec anyway.
Thanks,
Sunil
Powered by blists - more mailing lists