Re: [PATCH 3/3] acpi: nfit: Add support for hotplug

[Toshi Kani <toshi.kani@hpe.com>]

On Fri, 2015-10-09 at 12:44 -0700, Dan Williams wrote:
> [ adding Robert and Toshi ]
> 
> On Wed, Oct 7, 2015 at 2:49 PM, Vishal Verma <vishal.l.verma@intel.com>
> wrote:
 :
> > @@ -242,9 +246,13 @@ static bool add_memdev(struct acpi_nfit_desc
> > *acpi_desc,
> >                 struct acpi_nfit_memory_map *memdev)
> >  {
> >         struct device *dev = acpi_desc->dev;
> > -       struct nfit_memdev *nfit_memdev = devm_kzalloc(dev,
> > -                       sizeof(*nfit_memdev), GFP_KERNEL);
> > +       struct nfit_memdev *nfit_memdev;
> > +
> > +       list_for_each_entry(nfit_memdev, &acpi_desc->memdevs, list)
> > +               if (memcmp(nfit_memdev->memdev, memdev,
> > sizeof(*memdev)) == 0)
> > +                       return true;
> 
> I'm wondering if we need to be cognizant of flag changes here.
> Robert, Toshi are you expecting that the flags of an existing memory
> device entry will be updated by the this notification mechanism?

This patch-set can focus on the simple hot-add case for now.  We think bit
[4] of the flags, smart and health event bit, could be updated on an
existing memory device when such an event has occurred during runtime, but
it may require clarification to the spec since the bit is currently defined
as "SMART and health events prior to OSPM handoff" (which does not make
sense for _FIT).  We will need to support other updates in the flags, but
we will first need to understand what the OS should do in such scenarios.

> > @@ -811,6 +835,8 @@ static int acpi_nfit_register_dimms(struct
> > acpi_nfit_desc *acpi_desc)
> >                          */
> >                         dev_err(acpi_desc->dev, "duplicate DCR
> > detected: %s\n",
> >                                         nvdimm_name(nvdimm));
> > +                       /* TODO Do we need the warning? */
> > +                       dimm_count++;
> 
> Robert, comments?

Yes, this warning message should be removed.

Thanks,
-Toshi