On Mon, Jan 28, 2019 at 1:40 PM Dexuan Cui <decui@xxxxxxxxxxxxx> wrote: > > > 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. > + * How about something a bit more relevant for the code in question: --- There are 4 "legacy" NVDIMM command sets (NVDIMM_FAMILY_{INTEL,MSFT,HPE1,HPE2}) that were created before an EFI working group was established to constrain this proliferation. The nfit driver probes for the supported command set by GUID. Note, If you're a platform developer looking to add a new command set to this probe consider using an existing set, or otherwise seek approval to publish the command set at http://www.uefi.org/RFIC_LIST.