Re: [ndctl PATCH v6 1/5] libndctl: Refactor out add_dimm() to handle NFIT specific init

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Tue, 2020-06-16 at 11:00 +0530, Vaibhav Jain wrote:
> Presently add_dimm() only probes dimms that support NFIT/ACPI. Hence

"...probes NVDIMMs for platforms that support the ACPI NFIT"

The NFIT is a platform firmware thing, not directly related to the DIMMs
themselves.

> this patch refactors this functionality into two functions namely

s/Hence this patch refactors/Refactor/

> add_dimm() and add_nfit_dimm(). Function add_dimm() performs
> allocation and common 'struct ndctl_dimm' initialization and depending
> on whether the dimm-bus supports NIFT, calls add_nfit_dimm(). Once
> the probe is completed based on the value of 'ndctl_dimm.cmd_family'
> appropriate dimm-ops are assigned to the dimm.
> 
> In case dimm-bus is of unknown type or doesn't support NFIT the
> initialization still continues, with no dimm-ops assigned to the
> 'struct ndctl_dimm' there-by limiting the functionality available.

No need to hyphenate 'thereby'

> 
> This patch shouldn't introduce any behavioral change.
> 
> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> ---
> Changelog:
> 
> v5..v6:
> * Changed a return code for add_nfit_dimm() in case a allocation
>   failed. [ Vishal ]
> * Updated an error message in error path of add_dimm() [ Vishal ]
> 
> v4..v5:
> * None
> 
> v3..v4:
> * None
> 
> v2..v3:
> * None
> 
> v1..v2:
> * None
> ---
>  ndctl/lib/libndctl.c | 196 +++++++++++++++++++++++++------------------
>  1 file changed, 115 insertions(+), 81 deletions(-)
> 
[..]

> +
> +	/* Assign dimm-ops based on command family */
> +	if (dimm->cmd_family == NVDIMM_FAMILY_INTEL)
> +		dimm->ops = intel_dimm_ops;
> +	if (dimm->cmd_family == NVDIMM_FAMILY_HPE1)
> +		dimm->ops = hpe1_dimm_ops;
> +	if (dimm->cmd_family == NVDIMM_FAMILY_MSFT)
> +		dimm->ops = msft_dimm_ops;
> +	if (dimm->cmd_family == NVDIMM_FAMILY_HYPERV)
> +		dimm->ops = hyperv_dimm_ops;
>   out:
> +	if (rc) {
> +		/* Ensure dimm_uninit() is not called during free_dimm() */
> +		dimm->ops = NULL;

I think the above assignment and comment are now stale..

> +		err(ctx, "%s: probe failed: %s\n", ndctl_dimm_get_devname(dimm),
> +		    strerror(-rc));
> +		goto err_read;
> +	}
> +
>  	list_add(&bus->dimms, &dimm->list);
>  	free(path);
>  
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org