"Verma, Vishal L" <vishal.l.verma(a)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
> 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
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.