"Verma, Vishal L" <vishal.l.ve...@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-le...@lists.01.org
