On 03/06/2017 07:39 PM, Dan Williams wrote: > On Mon, Mar 6, 2017 at 4:25 PM, Linda Knippers <[email protected]> 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 <[email protected]> >> --- >> 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.
It's more complicated when you have a mixture of NVDIMM types, where not all NVDIMMs support the same families. For example, you could have an Intel NVDIMM in the same system as an HPE NVDIMM-N. The reason I called it "default" is because it should be the default for all devices that support that family but I didn't want other types of NVDIMMs to end up with no family at all. -- ljk _______________________________________________ Linux-nvdimm mailing list [email protected] https://lists.01.org/mailman/listinfo/linux-nvdimm
