diff mbox

[2/3] acpi: add a utility function for evaluating _FIT

Message ID 1444254577-23744-3-git-send-email-vishal.l.verma@intel.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Verma, Vishal L Oct. 7, 2015, 9:49 p.m. UTC
_FIT is an ACPI method to request an updated NFIT (Nvdimm Firmware
Interface Table) after a hotplug event. Add a function to evaluate the
_FIT method and return a buffer with the updated NFIT.

Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: <linux-acpi@vger.kernel.org>
Cc: <linux-nvdimm@lists.01.org>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 drivers/acpi/utils.c    | 23 +++++++++++++++++++++++
 include/acpi/acpi_bus.h |  1 +
 2 files changed, 24 insertions(+)

Comments

Jeff Moyer Oct. 9, 2015, 5:28 p.m. UTC | #1
Vishal Verma <vishal.l.verma@intel.com> writes:

>  /**
> + * acpi_evaluate_fit: Evaluate _FIT method to get an updated NFIT
> + * @handle: ACPI device handle
> + * @buf: buffer for the updated NFIT
> + *
> + * Evaluate device's _FIT method if present to get an updated NFIT
> + */
> +acpi_status acpi_evaluate_fit(acpi_handle handle, struct acpi_buffer **buf)
> +{
> +	acpi_status status;
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +
> +	status = acpi_evaluate_object(handle, "_FIT", NULL, &buffer);
> +
> +	if (ACPI_FAILURE(status))
> +		return status;
> +
> +	*buf = &buffer;

Umm, unless I'm missing something, you're returning a stack address.

-Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Verma, Vishal L Oct. 9, 2015, 5:54 p.m. UTC | #2
On Fri, 2015-10-09 at 13:28 -0400, Jeff Moyer wrote:
> Vishal Verma <vishal.l.verma@intel.com> writes:

> 

> >  /**

> > + * acpi_evaluate_fit: Evaluate _FIT method to get an updated NFIT

> > + * @handle: ACPI device handle

> > + * @buf: buffer for the updated NFIT

> > + *

> > + * Evaluate device's _FIT method if present to get an updated NFIT

> > + */

> > +acpi_status acpi_evaluate_fit(acpi_handle handle, struct

> > acpi_buffer **buf)

> > +{

> > +	acpi_status status;

> > +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL

> > };

> > +

> > +	status = acpi_evaluate_object(handle, "_FIT", NULL,

> > &buffer);

> > +

> > +	if (ACPI_FAILURE(status))

> > +		return status;

> > +

> > +	*buf = &buffer;

> 

> Umm, unless I'm missing something, you're returning a stack address.


Good point, you're right. Dan/Rafael, is it OK to just remove this
patch entirely, and call acpi_evaluate_object directly from nfit.c? The
current way did feel a bit kludgey to me any way because I was
allocating a buffer here (above), but trying to free it in the caller,
which seems very ugly..
Dan Williams Oct. 9, 2015, 5:56 p.m. UTC | #3
On Fri, Oct 9, 2015 at 10:54 AM, Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
> On Fri, 2015-10-09 at 13:28 -0400, Jeff Moyer wrote:
>> Vishal Verma <vishal.l.verma@intel.com> writes:
>>
>> >  /**
>> > + * acpi_evaluate_fit: Evaluate _FIT method to get an updated NFIT
>> > + * @handle: ACPI device handle
>> > + * @buf: buffer for the updated NFIT
>> > + *
>> > + * Evaluate device's _FIT method if present to get an updated NFIT
>> > + */
>> > +acpi_status acpi_evaluate_fit(acpi_handle handle, struct
>> > acpi_buffer **buf)
>> > +{
>> > +   acpi_status status;
>> > +   struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL
>> > };
>> > +
>> > +   status = acpi_evaluate_object(handle, "_FIT", NULL,
>> > &buffer);
>> > +
>> > +   if (ACPI_FAILURE(status))
>> > +           return status;
>> > +
>> > +   *buf = &buffer;
>>
>> Umm, unless I'm missing something, you're returning a stack address.
>
> Good point, you're right. Dan/Rafael, is it OK to just remove this
> patch entirely, and call acpi_evaluate_object directly from nfit.c? The
> current way did feel a bit kludgey to me any way because I was
> allocating a buffer here (above), but trying to free it in the caller,
> which seems very ugly..

Open coding a call to acpi_evaluate_object() sounds good to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki Oct. 9, 2015, 9:14 p.m. UTC | #4
On Friday, October 09, 2015 10:56:26 AM Dan Williams wrote:
> On Fri, Oct 9, 2015 at 10:54 AM, Verma, Vishal L
> <vishal.l.verma@intel.com> wrote:
> > On Fri, 2015-10-09 at 13:28 -0400, Jeff Moyer wrote:
> >> Vishal Verma <vishal.l.verma@intel.com> writes:
> >>
> >> >  /**
> >> > + * acpi_evaluate_fit: Evaluate _FIT method to get an updated NFIT
> >> > + * @handle: ACPI device handle
> >> > + * @buf: buffer for the updated NFIT
> >> > + *
> >> > + * Evaluate device's _FIT method if present to get an updated NFIT
> >> > + */
> >> > +acpi_status acpi_evaluate_fit(acpi_handle handle, struct
> >> > acpi_buffer **buf)
> >> > +{
> >> > +   acpi_status status;
> >> > +   struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL
> >> > };
> >> > +
> >> > +   status = acpi_evaluate_object(handle, "_FIT", NULL,
> >> > &buffer);
> >> > +
> >> > +   if (ACPI_FAILURE(status))
> >> > +           return status;
> >> > +
> >> > +   *buf = &buffer;
> >>
> >> Umm, unless I'm missing something, you're returning a stack address.
> >
> > Good point, you're right. Dan/Rafael, is it OK to just remove this
> > patch entirely, and call acpi_evaluate_object directly from nfit.c? The
> > current way did feel a bit kludgey to me any way because I was
> > allocating a buffer here (above), but trying to free it in the caller,
> > which seems very ugly..
> 
> Open coding a call to acpi_evaluate_object() sounds good to me.

Agreed.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 475c907..78f7c69 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -614,6 +614,29 @@  acpi_status acpi_evaluate_lck(acpi_handle handle, int lock)
 }
 
 /**
+ * acpi_evaluate_fit: Evaluate _FIT method to get an updated NFIT
+ * @handle: ACPI device handle
+ * @buf: buffer for the updated NFIT
+ *
+ * Evaluate device's _FIT method if present to get an updated NFIT
+ */
+acpi_status acpi_evaluate_fit(acpi_handle handle, struct acpi_buffer **buf)
+{
+	acpi_status status;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+
+	status = acpi_evaluate_object(handle, "_FIT", NULL, &buffer);
+
+	if (ACPI_FAILURE(status))
+		return status;
+
+	*buf = &buffer;
+
+	return status;
+}
+EXPORT_SYMBOL(acpi_evaluate_fit);
+
+/**
  * acpi_evaluate_dsm - evaluate device's _DSM method
  * @handle: ACPI device handle
  * @uuid: UUID of requested functions, should be 16 bytes
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 5ba8fb6..7483399 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -61,6 +61,7 @@  bool acpi_ata_match(acpi_handle handle);
 bool acpi_bay_match(acpi_handle handle);
 bool acpi_dock_match(acpi_handle handle);
 
+acpi_status acpi_evaluate_fit(acpi_handle handle, struct acpi_buffer **buf);
 bool acpi_check_dsm(acpi_handle handle, const u8 *uuid, int rev, u64 funcs);
 union acpi_object *acpi_evaluate_dsm(acpi_handle handle, const u8 *uuid,
 			int rev, int func, union acpi_object *argv4);