Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752441AbdFQAKG (ORCPT ); Fri, 16 Jun 2017 20:10:06 -0400 Received: from mail-ot0-f178.google.com ([74.125.82.178]:34330 "EHLO mail-ot0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751144AbdFQAKE (ORCPT ); Fri, 16 Jun 2017 20:10:04 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170608183658.24731-1-toshi.kani@hpe.com> <20170608183658.24731-3-toshi.kani@hpe.com> From: Dan Williams Date: Fri, 16 Jun 2017 17:10:03 -0700 Message-ID: Subject: Re: [PATCH v2 2/2] acpi/nfit: Issue Start ARS to retrieve existing records To: "Kani, Toshimitsu" Cc: "Rafael J. Wysocki" , "Verma, Vishal L" , "Knippers, Linda" , "linux-nvdimm@lists.01.org" , Linux ACPI , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1606 Lines: 35 On Fri, Jun 16, 2017 at 5:01 PM, Kani, Toshimitsu wrote: >> > --- a/drivers/acpi/nfit/core.c >> > +++ b/drivers/acpi/nfit/core.c >> > @@ -1031,7 +1031,7 @@ static ssize_t scrub_store(struct device *dev, >> > if (nd_desc) { >> > struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc); >> > >> > - rc = acpi_nfit_ars_rescan(acpi_desc); >> > + rc = acpi_nfit_ars_rescan(acpi_desc, 0); >> > } >> > device_unlock(dev); >> > if (rc) >> > @@ -2057,6 +2057,10 @@ static int ars_start(struct acpi_nfit_desc >> *acpi_desc, struct nfit_spa *nfit_spa >> > ars_start.type = ND_ARS_VOLATILE; >> > else >> > return -ENOTTY; >> > + if (nfit_spa->ars_prev_data) { >> > + ars_start.flags |= ND_ARS_RETURN_PREV_DATA; >> > + nfit_spa->ars_prev_data = 0; >> > + } >> >> I'd rather you plumb a new 'flags' parameter all the way through from >> acpi_nfit_ars_rescan() to ars_start() rather than carrying this as a >> property of nfit_spa. > > Yes, I wanted to carry 'flags' all the way, but since acpi_nfit_ars_rescan() > calls acpi_nfit_scrub() via acpi_desc->work, all info needs to be marshalled > into struct acpi_nfit_desc. Using nfit_spa allows a request to be carried as > per-spa basis... Ah ok, but I still think it does not belong to a spa. This is control / context information for the workqueue and that belongs with acpi_nfit_desc. It think it's fine if all spas get re-scrubbed with the "prev_data" flag in the the notification case.