Re: [PATCH 2/3] Allow specifying a default DSM family

On Mon, Mar 6, 2017 at 4:25 PM, Linda Knippers <linda.knippers(a)hpe.com&gt; wrote: > Provide the ability to request a default DSM family. If it is not > supported, then fall back to the normal discovery order. > > This is helpful for testing platforms that support multiple DSM families. > It will also allow administrators to request the DSM family that their > management tools support, which may not be the first one found using > the current discovery order. > > Signed-off-by: Linda Knippers <linda.knippers(a)hpe.com&gt; > --- > drivers/acpi/nfit/core.c | 26 ++++++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > index 97d42ff..78c46a7 100644 > --- a/drivers/acpi/nfit/core.c > +++ b/drivers/acpi/nfit/core.c > @@ -55,6 +55,11 @@ > module_param(override_dsm_mask, ulong, S_IRUGO); > MODULE_PARM_DESC(override_dsm_mask, "Bitmask of allowed NVDIMM DSM functions"); > > +static int default_dsm_family = -1; > +module_param(default_dsm_family, int, S_IRUGO); > +MODULE_PARM_DESC(default_dsm_family, > + "Try this DSM type first when identifying NVDIMM family"); > + > LIST_HEAD(acpi_descs); > DEFINE_MUTEX(acpi_desc_lock); > > @@ -1371,7 +1376,7 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, > struct device *dev = acpi_desc->dev; > unsigned long dsm_mask; > const u8 *uuid; > - int i; > + int i = -1; > > /* nfit test assumes 1:1 relationship between commands and dsms */ > nfit_mem->dsm_mask = acpi_desc->dimm_cmd_force_en; > @@ -1399,10 +1404,23 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, > * Until standardization materializes we need to consider 4 > * different command sets. Note, that checking for function0 (bit0) > * tells us if any commands are reachable through this uuid. > + * First check for a requested default. > */ > - for (i = NVDIMM_FAMILY_INTEL; i <= NVDIMM_FAMILY_MSFT; i++) > - if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), 1, 1)) > - break; > + if (default_dsm_family >= NVDIMM_FAMILY_INTEL && > + default_dsm_family <= NVDIMM_FAMILY_MSFT) { > + if (acpi_check_dsm(adev_dimm->handle, > + to_nfit_uuid(default_dsm_family), 1, 1)) > + i = default_dsm_family; > + else > + dev_dbg(dev, "default_dsm_family %d not supported\n", > + default_dsm_family); > + } > + if (i == -1) { > + for (i = NVDIMM_FAMILY_INTEL; i <= NVDIMM_FAMILY_MSFT; i++) > + if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), > + 1, 1)) > + break; > + } I think we can make this simpler and more deterministic with a "force" option? Something like: @@ -1397,8 +1397,12 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, * tells us if any commands are reachable through this uuid. */ for (i = NVDIMM_FAMILY_INTEL; i <= NVDIMM_FAMILY_MSFT; i++) - if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), 1, 1)) - break; + if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), 1, 1)) { + if (force_dsm_family < 0) + break; + else if (i == force_dsm_family) + break; + } /* limit the supported commands to those that are publicly documented */ nfit_mem->family = i; ...because the user specifying the override should know ahead of time if that family is available, and we should fail the detection otherwise.