| Message ID | 1444254577-23744-3-git-send-email-vishal.l.verma@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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
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..
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.
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
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);
_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(+)