From: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
To: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Mauro Carvalho Chehab
<mchehab-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org,
"Rafael J. Wysocki" <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Qiuxu Zhuo <qiuxu.zhuo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>,
Aristeu Rozanski <aris-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>,
Len Brown <lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Subject: Re: [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs
Date: Tue, 13 Mar 2018 10:59:01 +0100 [thread overview]
Message-ID: <20180313105901.0f18d027@endymion> (raw)
In-Reply-To: <20180312182430.10335-6-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Hi Tony,
On Mon, 12 Mar 2018 11:24:30 -0700, Tony Luck wrote:
> This just covers the topology function of the EDAC driver.
> We locate which DIMM slots are populated with NVDIMMs and
> query the NFIT and SMBIOS tables to get the size.
>
> Signed-off-by: Tony Luck <tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/edac/Kconfig | 5 +++-
> drivers/edac/skx_edac.c | 66 ++++++++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 64 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 3c4017007647..c12e34564557 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -232,9 +232,12 @@ config EDAC_SBRIDGE
> config EDAC_SKX
> tristate "Intel Skylake server Integrated MC"
> depends on PCI && X86_64 && X86_MCE_INTEL && PCI_MMCONFIG
> + select DMI
> help
> Support for error detection and correction the Intel
> - Skylake server Integrated Memory Controllers.
> + Skylake server Integrated Memory Controllers. If your
> + system has non-volatile DIMMs you should also manually
> + select CONFIG_ACPI_NFIT
Nitpicking: trailing dot is missing.
>
> config EDAC_PND2
> tristate "Intel Pondicherry2"
> diff --git a/drivers/edac/skx_edac.c b/drivers/edac/skx_edac.c
> index 912c4930c9ef..84c18bb1e0cd 100644
> --- a/drivers/edac/skx_edac.c
> +++ b/drivers/edac/skx_edac.c
> (...)
> +static int get_nvdimm_info(struct dimm_info *dimm, struct skx_imc *imc,
> + int chan, int dimmno)
> +{
> + int smbios_handle;
> + u32 dev_handle;
> + u16 flags;
> + u64 size = 0;
> +
> + dev_handle = ACPI_NFIT_BUILD_DEVICE_HANDLE(dimmno, chan, imc->lmc,
> + imc->src_id, 0);
> +
> + smbios_handle = nfit_get_smbios_id(dev_handle, &flags);
> + if (smbios_handle == -EOPNOTSUPP) {
> + pr_warn_once("skx_edac: can't find size of NVDIMM. Try enabling CONFIG_ACPI_NFIT\n");
> + goto unknown_size;
> + }
> + if (smbios_handle < 0) {
> + skx_printk(KERN_ERR, "Can't find handle for NVDIMM ADR=%x\n", dev_handle);
> + goto unknown_size;
> + }
> +
> + if (flags & ACPI_NFIT_MEM_MAP_FAILED) {
> + skx_printk(KERN_ERR, "NVDIMM ADR=%x is not mapped\n", dev_handle);
> + goto unknown_size;
> + }
> +
> + size = dmi_memdev_size(smbios_handle);
> + if (size == ~0ull)
> + skx_printk(KERN_ERR, "Can't find size for NVDIMM ADR=%x/SMBIOS=%x\n",
> + dev_handle, smbios_handle);
> +
> +unknown_size:
> + dimm->nr_pages = size >> PAGE_SHIFT;
> + dimm->grain = 32;
> + dimm->dtype = DEV_UNKNOWN;
> + dimm->mtype = MEM_NVDIMM;
> + dimm->edac_mode = EDAC_SECDED; /* likely better than this */
> +
> + edac_dbg(0, "mc#%d: channel %d, dimm %d, %llu Mb (%u pages)\n",
I did not notice on previous review, but I think "b" in general means
bit not byte, so "MB" would be better.
> + imc->mc, chan, dimmno, size >> 20, dimm->nr_pages);
> +
> + snprintf(dimm->label, sizeof(dimm->label), "CPU_SrcID#%u_MC#%u_Chan#%u_DIMM#%u",
> + imc->src_id, imc->lmc, chan, dimmno);
> +
> + return 1;
> +}
Now this function always return 1, that doesn't make a lot of sense?
Other than these details, the dmi-related code looks good to me now.
Reviewed-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org> # for DMI
--
Jean Delvare
SUSE L3 Support
next prev parent reply other threads:[~2018-03-13 9:59 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-22 19:58 [PATCH 0/5] Teach EDAC about non-volatile DIMMs and add partial support to skx_edac Tony Luck
[not found] ` <20180222195811.27237-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2018-02-22 19:58 ` [PATCH 1/5] EDAC: Drop duplicated array of strings for memory type names Tony Luck
2018-02-22 19:58 ` [PATCH 2/5] edac: Add new memory type for non-volatile DIMMs Tony Luck
2018-02-22 19:58 ` [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle Tony Luck
[not found] ` <20180222195811.27237-4-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2018-02-28 17:36 ` Ross Zwisler
[not found] ` <20180228173621.GA12883-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-02-28 18:48 ` Borislav Petkov
2018-02-22 19:58 ` [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size Tony Luck
[not found] ` <20180222195811.27237-5-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2018-03-09 10:20 ` Jean Delvare
2018-03-09 23:03 ` Luck, Tony
2018-03-10 13:22 ` Jean Delvare
2018-03-12 16:46 ` Luck, Tony
2018-02-22 19:58 ` [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs Tony Luck
[not found] ` <20180222195811.27237-6-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2018-03-09 10:38 ` Jean Delvare
2018-02-28 13:04 ` [PATCH 0/5] Teach EDAC about non-volatile DIMMs and add partial support to skx_edac Borislav Petkov
[not found] ` <20180228130410.GC3769-fF5Pk5pvG8Y@public.gmane.org>
2018-02-28 13:59 ` Dan Williams
[not found] ` <CAPcyv4iHr9nUAS0SiFeqqDkU8NXZ_x=rkD3YS010Dy-nbHJXmA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-03-12 18:24 ` [PATCH 0/5 V3] " Tony Luck
[not found] ` <20180312182430.10335-1-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2018-03-12 18:24 ` [PATCH 1/5] EDAC: Drop duplicated array of strings for memory type names Tony Luck
2018-03-12 18:24 ` [PATCH 2/5] edac: Add new memory type for non-volatile DIMMs Tony Luck
2018-03-12 18:24 ` [PATCH 3/5] acpi, nfit: Add function to look up nvdimm device and provide SMBIOS handle Tony Luck
2018-03-12 18:24 ` [PATCH 4/5] firmware: dmi: Add function to look up a handle and return DIMM size Tony Luck
[not found] ` <20180312182430.10335-5-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2018-03-13 9:43 ` Jean Delvare
2018-03-12 18:24 ` [PATCH 5/5] EDAC, skx_edac: Detect non-volatile DIMMs Tony Luck
[not found] ` <20180312182430.10335-6-tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2018-03-13 9:59 ` Jean Delvare [this message]
2018-03-13 15:59 ` Luck, Tony
2018-03-13 16:16 ` Jean Delvare
2018-03-14 12:00 ` Borislav Petkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180313105901.0f18d027@endymion \
--to=jdelvare-l3a5bk7wagm@public.gmane.org \
--cc=aris-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
--cc=bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org \
--cc=bp-l3A5Bk7waGM@public.gmane.org \
--cc=lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org \
--cc=mchehab-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=qiuxu.zhuo-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
--cc=rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org \
--cc=tony.luck-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).