From: Vaibhav Jain <vaibhav@linux.ibm.com>
To: "Verma\, Vishal L" <vishal.l.verma@intel.com>,
"linux-nvdimm\@lists.01.org" <linux-nvdimm@lists.01.org>
Cc: "aneesh.kumar@linux.ibm.com" <aneesh.kumar@linux.ibm.com>
Subject: Re: [ndctl PATCH v5 2/6] libncdtl: Add initial support for NVDIMM_FAMILY_PAPR nvdimm family
Date: Wed, 03 Jun 2020 22:19:18 +0530 [thread overview]
Message-ID: <87v9k8gkmp.fsf@linux.ibm.com> (raw)
In-Reply-To: <67eb2e50aa65509f16e63dc33d1ff5e88b4b5496.camel@intel.com>
"Verma, Vishal L" <vishal.l.verma@intel.com> writes:
> On Wed, 2020-06-03 at 15:27 +0530, Vaibhav Jain wrote:
>> >
>> > Two things here:
>> > 1. Why not use the new ndctl_bus_has_of_node helper here? and
>> > 2. This looks redundant. add_papr_dimm() is only called if
>> > ndctl_bus_has_of_node() during add_dimm.
>> Presently we have two different nvdimm implementations:
>>
>> * papr-scm: handled by arch/powerpc/platforms/pseries/papr_scm kernel module.
>> * nvdimm-n: handled by drivers/nvdimm/of_pmem kernel module.
>>
>> Both nvdimms are exposed to the kernel via device tree nodes but different
>> 'compatible' properties. This patchset only adds support for 'papr-scm'
>> compatible nvdimms.
>>
>> 'ndctl_bus_has_of_node()' simply indicates if the nvdimm has an
>> open-firmware compatible devicetree node associated with it and doesnt
>> necessarily indicate if its papr-scm compliant.
>>
>> Hence validating the 'compatible' attribute value is necessary here.
>> Please see a more detailed info below regarding the 'compatible' sysfs
>> attribute.
>>
> Understood - one more question:
>
> Would it be useful to wrap the 'compatible' check into an API similar to
> _has_of_node - say ndctl_bus_is_papr_compatible()? I'm not too strongly
> attached this, there is only one user so far after all, but it seemed
> like an easy thing that might get copy-pasted around in the future.
Yes, sounds good to me. Should simplify the add_papr_dimm() function a
bit as I can just call ndctl_bus_is_papr_compatible() and if true then
setup the cmd_family.
Will roll out this change in v6 iteration.
--
Cheers
~ Vaibhav
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
next prev parent reply other threads:[~2020-06-03 16:49 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-29 22:05 [ndctl PATCH v5 0/6] Add support for reporting papr nvdimm health Vaibhav Jain
2020-05-29 22:05 ` [ndctl PATCH v5 1/6] libndctl: Refactor out add_dimm() to handle NFIT specific init Vaibhav Jain
2020-06-03 1:05 ` Verma, Vishal L
2020-06-03 7:10 ` Vaibhav Jain
2020-05-29 22:05 ` [ndctl PATCH v5 2/6] libncdtl: Add initial support for NVDIMM_FAMILY_PAPR nvdimm family Vaibhav Jain
2020-06-03 5:47 ` Verma, Vishal L
2020-06-03 9:57 ` Vaibhav Jain
2020-06-03 15:20 ` Verma, Vishal L
2020-06-03 16:49 ` Vaibhav Jain [this message]
2020-05-29 22:05 ` [ndctl PATCH v5 3/6] libndctl: Introduce new dimm-ops dimm_init() & dimm_uninit() Vaibhav Jain
2020-06-04 1:28 ` Verma, Vishal L
2020-06-04 21:42 ` Vaibhav Jain
2020-05-29 22:05 ` [ndctl PATCH v5 4/6] libndctl,papr_scm: Add definitions for PAPR nvdimm specific methods Vaibhav Jain
2020-05-29 22:05 ` [ndctl PATCH v5 5/6] papr: Add scaffolding to issue and handle PDSM requests Vaibhav Jain
2020-05-29 22:06 ` [ndctl PATCH v5 6/6] libndctl,papr_scm: Implement support for PAPR_PDSM_HEALTH Vaibhav Jain
2020-06-04 1:26 ` Williams, Dan J
2020-06-04 21:55 ` Vaibhav Jain
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=87v9k8gkmp.fsf@linux.ibm.com \
--to=vaibhav@linux.ibm.com \
--cc=aneesh.kumar@linux.ibm.com \
--cc=linux-nvdimm@lists.01.org \
--cc=vishal.l.verma@intel.com \
/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