> From: Dan Williams <dan.j.williams@xxxxxxxxx> > Sent: Monday, January 28, 2019 1:01 PM > > Hi Dexuan, > Looks good. Just one update request and a note below... > > On Wed, Jan 23, 2019 at 12:51 PM Dexuan Cui <decui@xxxxxxxxxxxxx> wrote: > > ... > > --- a/drivers/acpi/nfit/core.c > > +++ b/drivers/acpi/nfit/core.c > > @@ -1840,7 +1840,7 @@ static int acpi_nfit_add_dimm(struct > acpi_nfit_desc *acpi_desc, > > dev_set_drvdata(&adev_dimm->dev, nfit_mem); > > > > /* > > - * Until standardization materializes we need to consider 4 > > + * Until standardization materializes we need to consider 5 > > * different command sets. Note, that checking for function0 > (bit0) > > * tells us if any commands are reachable through this GUID. > > */ > > This comment is stale. This "HYPERV" family is the first example of > the "right" way to define a new NVDIMM command set. Lets update it to > mention the process and considerations for submitting new command sets > to UEFI (http://www.uefi.org/RFIC_LIST). The fact that there's now a > defined process is a step forward from when this comment was initially > written. Also, the fact that the process encourages "adopt" vs > "supersede" addresses the main concern about vendor-specific > command-set proliferation. I made the below simple change. Is this enough? Please suggest the proper wording/sentence, as I'm relatively new to NVDIMM, and I don't really know the history of the standardization process. --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -1732,8 +1732,10 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, dev_set_drvdata(&adev_dimm->dev, nfit_mem); /* - * Until standardization materializes we need to consider 4 - * different command sets. Note, that checking for function0 (bit0) + * New command sets should be submitted to UEFI + * http://www.uefi.org/RFIC_LIST. + * + * Note, that checking for function0 (bit0) * tells us if any commands are reachable through this GUID. */ for (i = 0; i <= NVDIMM_FAMILY_MAX; i++) > > @@ -1865,6 +1865,8 @@ static int acpi_nfit_add_dimm(struct > acpi_nfit_desc *acpi_desc, > > dsm_mask &= ~(1 << 8); > > } else if (nfit_mem->family == NVDIMM_FAMILY_MSFT) { > > dsm_mask = 0xffffffff; > > + } else if (nfit_mem->family == NVDIMM_FAMILY_HYPERV) { > > + dsm_mask = 0x1f; > > Just a note that starting with commit 5e9e38d0db1d "acpi/nfit: Block > function zero DSMs", bit0 in this mask will be cleared to ensure that > only the kernel has the ability to probe for supported DSM functions. Thanks for the note! Thanks, -- Dexuan
