[4/5] EDAC, dsm_edac: Wrap ACPI DSM methods for address translation

From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Tony Luck <tony.luck@intel.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Borislav Petkov <bp@alien8.de>,
	qiuxu.zhuo@intel.com, aris@redhat.com, mchehab@s-opensource.com,
	"open list:EDAC-CORE" <linux-edac@vger.kernel.org>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>
Subject: [4/5] EDAC, dsm_edac: Wrap ACPI DSM methods for address translation
Date: Tue, 9 Oct 2018 17:29:39 +0200	[thread overview]
Message-ID: <CAJZ5v0i_t_BZO_-sW+TkamSjv6j=ftLd7unfG1-fwqLrOm+yUg@mail.gmail.com> (raw)

On Tue, Oct 9, 2018 at 5:26 PM Luck, Tony <tony.luck@intel.com> wrote:
>
> On Tue, Oct 09, 2018 at 12:28:54PM +0200, Rafael J. Wysocki wrote:
> > Well, it looks reasonable, one question though (below).
> >
> > > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > > index dd1eea90f67f..327c93b51cb7 100644
> > > --- a/drivers/acpi/Kconfig
> > > +++ b/drivers/acpi/Kconfig
> > > @@ -498,6 +498,16 @@ config ACPI_EXTLOG
> > >       driver adds support for that functionality with corresponding
> > >       tracepoint which carries that information to userspace.
> > >
> > > +config ACPI_ADXL
> > > +   bool "Physical address to DIMM address translation"
> > > +   def_bool n
> > > +   help
> > > +     Enable interface that calls into BIOS using a DSM (device
> > > +     specific method) to convert system physical addresses
> > > +     to DIMM (socket, channel, rank, dimm, etc.).
> > > +     Only available on some servers.
> > > +     Used by newer EDAC drivers.
> > > +
> >
> > I wonder if the extra config option is really needed.  I guess it is useful
> > for the "tinification" people, but then can it simply depend on something
> > else we already have?
> >
> > Distros will set it to "Y" anyway.
>
> I was planning on having the EDAC driver(s) that need this do a
>
>         select ACPI_ADXL
>
> That way the tinification people are happy, and distros that include
> EDAC will set it without any other changes.
>
> > > +#define ACPI_ADXL_PATH                     "\\_SB.ADXL"
> >
> > It would be good to add a comment to say where the ADXL object is
> > defined.
>
> Can you expand on what you want here? My naive ACPI knowledge
> thought that "\\_SB.ADXL" was a "pathname" saying that the ADXL
> device is part of the "SB" (is that System Board?).
>
> Are you asking which platforms include this?

No, I'm asking about where the contents of that object are documented.
Presumably, the patch is based on some documentation, so it would be
good to give a pointer to it here, if available.

> Qiuxu has tested this. So I can re-post to linux-acpi for any
> other comments from the community.
>
> Procedure question. If you like this, can you "Ack" it and let
> Boris take it through his tree.

No, I can't ACK it at this time yet, sorry.

> The changes to the Skylake EDAC driver are dependent on this, so we need
> to have the commits apply in the right order.

No problem with that when the patch is ready.

Thanks,
Rafael