diff mbox series

[v2,RESEND,02/11] platform/x86/amd/pmf: Add support for PMF APCI layer

Message ID 20220728182028.2082096-3-Shyam-sundar.S-k@amd.com (mailing list archive)
State Changes Requested, archived
Headers show
Series platform/x86/amd/pmf: Introduce AMD PMF Driver | expand

Commit Message

Shyam Sundar S K July 28, 2022, 6:20 p.m. UTC
PMF driver implements the ACPI methods as defined by AMD for PMF Support.
The ACPI layer acts as a glue that helps in providing the infrastructure
for OEMs customization.

OEMs can refer to PMF support documentation to decide on the list of
functions to be supported on their specific platform model.

AMD mandates that PMF ACPI fn0 and fn1 to be implemented which
provides the set of functions, params and the notifications that
would be sent to PMF driver so that PMF driver can adapt and
react.

Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/Makefile |   2 +-
 drivers/platform/x86/amd/pmf/acpi.c   | 182 ++++++++++++++++++++++++++
 drivers/platform/x86/amd/pmf/core.c   |   1 +
 drivers/platform/x86/amd/pmf/pmf.h    |  32 +++++
 4 files changed, 216 insertions(+), 1 deletion(-)
 create mode 100644 drivers/platform/x86/amd/pmf/acpi.c

Comments

Hans de Goede Aug. 1, 2022, 11:24 a.m. UTC | #1
Hi,

On 7/28/22 20:20, Shyam Sundar S K wrote:
> PMF driver implements the ACPI methods as defined by AMD for PMF Support.
> The ACPI layer acts as a glue that helps in providing the infrastructure
> for OEMs customization.
> 
> OEMs can refer to PMF support documentation to decide on the list of
> functions to be supported on their specific platform model.
> 
> AMD mandates that PMF ACPI fn0 and fn1 to be implemented which
> provides the set of functions, params and the notifications that
> would be sent to PMF driver so that PMF driver can adapt and
> react.
> 
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>  drivers/platform/x86/amd/pmf/Makefile |   2 +-
>  drivers/platform/x86/amd/pmf/acpi.c   | 182 ++++++++++++++++++++++++++
>  drivers/platform/x86/amd/pmf/core.c   |   1 +
>  drivers/platform/x86/amd/pmf/pmf.h    |  32 +++++
>  4 files changed, 216 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/platform/x86/amd/pmf/acpi.c
> 
> diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
> index 459005f659e5..2617eba773ce 100644
> --- a/drivers/platform/x86/amd/pmf/Makefile
> +++ b/drivers/platform/x86/amd/pmf/Makefile
> @@ -5,4 +5,4 @@
>  #
>  
>  obj-$(CONFIG_AMD_PMF) += amd-pmf.o
> -amd-pmf-objs := core.o
> +amd-pmf-objs := core.o acpi.o
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> new file mode 100644
> index 000000000000..bd796d7e1d96
> --- /dev/null
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD Platform Management Framework Driver
> + *
> + * Copyright (c) 2022, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> + */
> +
> +#include <linux/acpi.h>
> +#include "pmf.h"
> +
> +#define APMF_METHOD "\\_SB_.PMF_.APMF"
> +
> +static unsigned long supported_func;
> +
> +static void apmf_if_parse_functions(struct apmf_if_functions *func, u32 mask);
> +
> +static union acpi_object *apmf_if_call(struct apmf_if *apmf_if, int func, struct acpi_buffer *param)
> +{
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_object_list apmf_if_arg_list;
> +	union acpi_object apmf_if_args[2];
> +	acpi_status status;
> +
> +	apmf_if_arg_list.count = 2;
> +	apmf_if_arg_list.pointer = &apmf_if_args[0];
> +
> +	apmf_if_args[0].type = ACPI_TYPE_INTEGER;
> +	apmf_if_args[0].integer.value = func;
> +
> +	if (param) {
> +		apmf_if_args[1].type = ACPI_TYPE_BUFFER;
> +		apmf_if_args[1].buffer.length = param->length;
> +		apmf_if_args[1].buffer.pointer = param->pointer;
> +	} else {
> +		apmf_if_args[1].type = ACPI_TYPE_INTEGER;
> +		apmf_if_args[1].integer.value = 0;
> +	}
> +
> +	status = acpi_evaluate_object(apmf_if->handle, NULL, &apmf_if_arg_list, &buffer);
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("PMF: APMF method call failed\n");
> +		if (status != AE_NOT_FOUND)
> +			kfree(buffer.pointer);
> +
> +		return NULL;
> +	}
> +
> +	return buffer.pointer;
> +}
> +

The below helper is much better then the open coding in the v1 series, thank you.

> +static int apmf_if_call_store_buffer(struct apmf_if *apmf_if, int func, void *dest, size_t out_sz)
> +{
> +	union acpi_object *info;
> +	size_t size;
> +	int err = 0;
> +
> +	info = apmf_if_call(apmf_if, func, NULL);
> +	if (!info)
> +		return -EIO;
> +
> +	if (info->type != ACPI_TYPE_BUFFER) {
> +		pr_err("object is not a buffer\n");

Since you use pr_err, you should put the following line:

#define pr_fmt(fmt) "AMD-PMF: " fmt

*above* the first #include in this .c file, so that the errors
get pre-fixed with "AMD-PMF: " when printed.

> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (info->buffer.length < 2) {
> +		pr_err("buffer too small\n");
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	size = *(u16 *)info->buffer.pointer;
> +	if (info->buffer.length < size) {
> +		pr_err("buffer smaller then headersize %u < %zu\n",
> +		       info->buffer.length, size);
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (size < out_sz) {
> +		pr_err("buffer too small %zu\n", size);
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	size = out_sz;

Please drop this line

> +	memcpy(dest, info->buffer.pointer, size);

and just use out_sz directly here instead of size.

> +
> +out:
> +	kfree(info);
> +	return err;
> +}
> +
> +static void apmf_if_parse_functions(struct apmf_if_functions *func, u32 mask)
> +{
> +	func->system_params = mask & APMF_FUNC_GET_SYS_PARAMS;
> +}
> +
> +int is_apmf_func_supported(unsigned long index)
> +{
> +	/* If bit-n is set, that indicates function n+1 is supported */
> +	return ((supported_func & (1 << (index - 1))) != 0);
> +}
> +
> +static int apmf_if_verify_interface(struct amd_pmf_dev *pdev, struct apmf_if *apmf_if)
> +{
> +	struct apmf_verify_interface output;
> +	int err;
> +
> +	err = apmf_if_call_store_buffer(apmf_if, APMF_FUNC_VERIFY_INTERFACE,
> +					&output, sizeof(output));
> +	if (err)
> +		return err;
> +
> +	apmf_if_parse_functions(&apmf_if->func, output.supported_functions);
> +	supported_func = output.supported_functions;
> +	dev_dbg(pdev->dev, "supported functions:0x%x notifications:0x%x\n",
> +		output.supported_functions, output.notification_mask);
> +
> +	return 0;
> +}
> +
> +static int apmf_get_system_params(struct apmf_if *apmf_if)
> +{
> +	struct apmf_system_params params;
> +	int err;
> +
> +	if (apmf_if->func.system_params) {
> +		err = apmf_if_call_store_buffer(apmf_if, APMF_FUNC_GET_SYS_PARAMS,
> +						&params, sizeof(params));
> +		if (err)
> +			return err;
> +	}
> +
> +	pr_debug("PMF: system params mask:0x%x flags:0x%x cmd_code:0x%x\n",
> +		 params.valid_mask,
> +		 params.flags,
> +		 params.command_code);
> +	params.flags = params.flags & params.valid_mask;
> +
> +	return 0;
> +}
> +
> +int apmf_acpi_init(struct amd_pmf_dev *pmf_dev)
> +{
> +	acpi_handle apmf_if_handle;
> +	struct apmf_if *apmf_if;
> +	acpi_status status;
> +	int ret;
> +
> +	status = acpi_get_handle(NULL, (acpi_string) APMF_METHOD, &apmf_if_handle);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	apmf_if = kzalloc(sizeof(*apmf_if), GFP_KERNEL);
> +	if (!apmf_if)
> +		goto out;
> +
> +	apmf_if->handle = apmf_if_handle;
> +
> +	ret = apmf_if_verify_interface(pmf_dev, apmf_if);
> +	if (ret) {
> +		dev_err(pmf_dev->dev, "APMF verify interface failed :%d\n", ret);
> +		kfree(apmf_if);
> +		goto out;
> +	}
> +	pmf_dev->apmf_if = apmf_if;
> +
> +	ret = apmf_get_system_params(apmf_if);
> +	if (ret) {
> +		dev_err(pmf_dev->dev, "APMF apmf_get_system_params failed :%d\n", ret);
> +		kfree(apmf_if);
> +		goto out;

If you hit this error path you now have pmf_dev->ampf_of pointing to free-ed
memory. Please move the:

	pmf_dev->apmf_if = apmf_if;

statement to below this "if (ret) {}" block.

> +	}
> +
> +out:
> +	return ret;
> +}
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index aef97965c181..c5002b7bb904 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -204,6 +204,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
>  	if (!dev->regbase)
>  		return -ENOMEM;
>  
> +	apmf_acpi_init(dev);
>  	platform_set_drvdata(pdev, dev);
>  
>  	mutex_init(&dev->lock);
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 1c2e942e5096..08c6bc0e2b42 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -11,6 +11,12 @@
>  #ifndef PMF_H
>  #define PMF_H
>  
> +#include <linux/acpi.h>
> +
> +/* APMF Functions */
> +#define APMF_FUNC_VERIFY_INTERFACE			0
> +#define APMF_FUNC_GET_SYS_PARAMS			1
> +
>  /* Message Definitions */
>  #define SET_SPL				0x03 /* SPL: Sustained Power Limit */
>  #define SET_SPPT			0x05 /* SPPT: Slow Package Power Tracking */
> @@ -30,6 +36,30 @@
>  #define GET_STT_LIMIT_APU	0x20
>  #define GET_STT_LIMIT_HS2	0x21
>  
> +/* AMD PMF BIOS interfaces */
> +struct apmf_if_functions {
> +	bool system_params;
> +};
> +
> +struct apmf_if {
> +	acpi_handle handle;
> +	struct apmf_if_functions func;
> +};
> +
> +struct apmf_verify_interface {
> +	u16 size;
> +	u16 version;
> +	u32 notification_mask;
> +	u32 supported_functions;
> +} __packed;
> +
> +struct apmf_system_params {
> +	u16 size;
> +	u32 valid_mask;
> +	u32 flags;
> +	u8 command_code;
> +} __packed;
> +
>  struct amd_pmf_dev {
>  	void __iomem *regbase;
>  	void __iomem *smu_virt_addr;
> @@ -37,10 +67,12 @@ struct amd_pmf_dev {
>  	u32 base_addr;
>  	u32 cpu_id;
>  	struct device *dev;
> +	struct apmf_if *apmf_if;
>  	struct mutex lock; /* protects the PMF interface */
>  };
>  
>  /* Core Layer */
> +int apmf_acpi_init(struct amd_pmf_dev *pmf_dev);
>  int amd_pmf_send_cmd(struct amd_pmf_dev *dev, u8 message, bool get, u32 arg, u32 *data);
>  
>  #endif /* PMF_H */

Except for the few small remarks this looks good to me now,
so with the few remarks fixed, yoy can add my:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

To the next version.

Regards,

Hans
Hans de Goede Aug. 1, 2022, 12:48 p.m. UTC | #2
Hi,

On 7/28/22 20:20, Shyam Sundar S K wrote:
> PMF driver implements the ACPI methods as defined by AMD for PMF Support.
> The ACPI layer acts as a glue that helps in providing the infrastructure
> for OEMs customization.
> 
> OEMs can refer to PMF support documentation to decide on the list of
> functions to be supported on their specific platform model.
> 
> AMD mandates that PMF ACPI fn0 and fn1 to be implemented which
> provides the set of functions, params and the notifications that
> would be sent to PMF driver so that PMF driver can adapt and
> react.
> 
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>  drivers/platform/x86/amd/pmf/Makefile |   2 +-
>  drivers/platform/x86/amd/pmf/acpi.c   | 182 ++++++++++++++++++++++++++
>  drivers/platform/x86/amd/pmf/core.c   |   1 +
>  drivers/platform/x86/amd/pmf/pmf.h    |  32 +++++
>  4 files changed, 216 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/platform/x86/amd/pmf/acpi.c
> 
> diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
> index 459005f659e5..2617eba773ce 100644
> --- a/drivers/platform/x86/amd/pmf/Makefile
> +++ b/drivers/platform/x86/amd/pmf/Makefile
> @@ -5,4 +5,4 @@
>  #
>  
>  obj-$(CONFIG_AMD_PMF) += amd-pmf.o
> -amd-pmf-objs := core.o
> +amd-pmf-objs := core.o acpi.o
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> new file mode 100644
> index 000000000000..bd796d7e1d96
> --- /dev/null
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD Platform Management Framework Driver
> + *
> + * Copyright (c) 2022, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> + */
> +
> +#include <linux/acpi.h>
> +#include "pmf.h"
> +
> +#define APMF_METHOD "\\_SB_.PMF_.APMF"
> +
> +static unsigned long supported_func;
> +
> +static void apmf_if_parse_functions(struct apmf_if_functions *func, u32 mask);
> +
> +static union acpi_object *apmf_if_call(struct apmf_if *apmf_if, int func, struct acpi_buffer *param)
> +{
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	struct acpi_object_list apmf_if_arg_list;
> +	union acpi_object apmf_if_args[2];
> +	acpi_status status;
> +
> +	apmf_if_arg_list.count = 2;
> +	apmf_if_arg_list.pointer = &apmf_if_args[0];
> +
> +	apmf_if_args[0].type = ACPI_TYPE_INTEGER;
> +	apmf_if_args[0].integer.value = func;
> +
> +	if (param) {
> +		apmf_if_args[1].type = ACPI_TYPE_BUFFER;
> +		apmf_if_args[1].buffer.length = param->length;
> +		apmf_if_args[1].buffer.pointer = param->pointer;
> +	} else {
> +		apmf_if_args[1].type = ACPI_TYPE_INTEGER;
> +		apmf_if_args[1].integer.value = 0;
> +	}
> +
> +	status = acpi_evaluate_object(apmf_if->handle, NULL, &apmf_if_arg_list, &buffer);
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("PMF: APMF method call failed\n");
> +		if (status != AE_NOT_FOUND)
> +			kfree(buffer.pointer);
> +
> +		return NULL;
> +	}
> +
> +	return buffer.pointer;
> +}
> +
> +static int apmf_if_call_store_buffer(struct apmf_if *apmf_if, int func, void *dest, size_t out_sz)
> +{
> +	union acpi_object *info;
> +	size_t size;
> +	int err = 0;
> +
> +	info = apmf_if_call(apmf_if, func, NULL);
> +	if (!info)
> +		return -EIO;
> +
> +	if (info->type != ACPI_TYPE_BUFFER) {
> +		pr_err("object is not a buffer\n");
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (info->buffer.length < 2) {
> +		pr_err("buffer too small\n");
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	size = *(u16 *)info->buffer.pointer;
> +	if (info->buffer.length < size) {
> +		pr_err("buffer smaller then headersize %u < %zu\n",
> +		       info->buffer.length, size);
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (size < out_sz) {
> +		pr_err("buffer too small %zu\n", size);
> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	size = out_sz;
> +	memcpy(dest, info->buffer.pointer, size);
> +
> +out:
> +	kfree(info);
> +	return err;
> +}
> +
> +static void apmf_if_parse_functions(struct apmf_if_functions *func, u32 mask)
> +{
> +	func->system_params = mask & APMF_FUNC_GET_SYS_PARAMS;

I just noticed that this is not correct, this should be:

	func->system_params = mask & BIT(APMF_FUNC_GET_SYS_PARAMS - 1);

Which for APMF_FUNC_GET_SYS_PARAMS happens to be the same, but for
other AMPF_FUNC_* values the difference does matter!

Also this information is duplicated by:

is_apmf_func_supported(APMF_FUNC_GET_SYS_PARAMS) please drop the
apmf_if_functions struct and use is_apmf_func_supported() everywhere.

Note this means that you also will need to drop func->sbios_heartbeat
which does not come from the mask and which is initialized in
a different place rather then inside apmf_if_parse_functions() which
already shows that sbios_hearbeat does not really belong inside
this struct.

Instead of moving the sbios_heartbeat flag, please just directly
use notify_cfg.hb_interval inside apmf_acpi_deinit(). This will also
make the cancel_delayed_work_sync(&pmf_dev->heart_beat) condition
match the queue condition 1:1.


> +}
> +
> +int is_apmf_func_supported(unsigned long index)
> +{
> +	/* If bit-n is set, that indicates function n+1 is supported */
> +	return ((supported_func & (1 << (index - 1))) != 0);

Please write this as:

	return (supported_func & BIT(index - 1)) != 0;

or as:

	return !!(supported_func & BIT(index - 1));


> +}
> +
> +static int apmf_if_verify_interface(struct amd_pmf_dev *pdev, struct apmf_if *apmf_if)
> +{
> +	struct apmf_verify_interface output;
> +	int err;
> +
> +	err = apmf_if_call_store_buffer(apmf_if, APMF_FUNC_VERIFY_INTERFACE,
> +					&output, sizeof(output));
> +	if (err)
> +		return err;
> +
> +	apmf_if_parse_functions(&apmf_if->func, output.supported_functions);
> +	supported_func = output.supported_functions;
> +	dev_dbg(pdev->dev, "supported functions:0x%x notifications:0x%x\n",
> +		output.supported_functions, output.notification_mask);
> +
> +	return 0;
> +}
> +
> +static int apmf_get_system_params(struct apmf_if *apmf_if)
> +{
> +	struct apmf_system_params params;
> +	int err;
> +
> +	if (apmf_if->func.system_params) {
> +		err = apmf_if_call_store_buffer(apmf_if, APMF_FUNC_GET_SYS_PARAMS,
> +						&params, sizeof(params));
> +		if (err)
> +			return err;
> +	}
> +
> +	pr_debug("PMF: system params mask:0x%x flags:0x%x cmd_code:0x%x\n",
> +		 params.valid_mask,
> +		 params.flags,
> +		 params.command_code);
> +	params.flags = params.flags & params.valid_mask;
> +
> +	return 0;
> +}
> +
> +int apmf_acpi_init(struct amd_pmf_dev *pmf_dev)
> +{
> +	acpi_handle apmf_if_handle;
> +	struct apmf_if *apmf_if;
> +	acpi_status status;
> +	int ret;
> +
> +	status = acpi_get_handle(NULL, (acpi_string) APMF_METHOD, &apmf_if_handle);
> +	if (ACPI_FAILURE(status))
> +		return -ENODEV;
> +
> +	apmf_if = kzalloc(sizeof(*apmf_if), GFP_KERNEL);
> +	if (!apmf_if)
> +		goto out;
> +
> +	apmf_if->handle = apmf_if_handle;
> +
> +	ret = apmf_if_verify_interface(pmf_dev, apmf_if);
> +	if (ret) {
> +		dev_err(pmf_dev->dev, "APMF verify interface failed :%d\n", ret);
> +		kfree(apmf_if);
> +		goto out;
> +	}
> +	pmf_dev->apmf_if = apmf_if;

After dropping the apmf_if_functions sub-struct from amp_if amp_if only contains
an acpi_handle. Please just store the handle (and later on also the struct
apmf_notification_cfg) directly inside struct amd_pmf_dev, this also removes
the need for doing the whole alloc + free + associated error handling dance here.

Regsards,

Hans


> +
> +	ret = apmf_get_system_params(apmf_if);
> +	if (ret) {
> +		dev_err(pmf_dev->dev, "APMF apmf_get_system_params failed :%d\n", ret);
> +		kfree(apmf_if);
> +		goto out;
> +	}
> +
> +out:
> +	return ret;
> +}
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index aef97965c181..c5002b7bb904 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -204,6 +204,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
>  	if (!dev->regbase)
>  		return -ENOMEM;
>  
> +	apmf_acpi_init(dev);
>  	platform_set_drvdata(pdev, dev);
>  
>  	mutex_init(&dev->lock);
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 1c2e942e5096..08c6bc0e2b42 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -11,6 +11,12 @@
>  #ifndef PMF_H
>  #define PMF_H
>  
> +#include <linux/acpi.h>
> +
> +/* APMF Functions */
> +#define APMF_FUNC_VERIFY_INTERFACE			0
> +#define APMF_FUNC_GET_SYS_PARAMS			1
> +
>  /* Message Definitions */
>  #define SET_SPL				0x03 /* SPL: Sustained Power Limit */
>  #define SET_SPPT			0x05 /* SPPT: Slow Package Power Tracking */
> @@ -30,6 +36,30 @@
>  #define GET_STT_LIMIT_APU	0x20
>  #define GET_STT_LIMIT_HS2	0x21
>  
> +/* AMD PMF BIOS interfaces */
> +struct apmf_if_functions {
> +	bool system_params;
> +};
> +
> +struct apmf_if {
> +	acpi_handle handle;
> +	struct apmf_if_functions func;
> +};
> +
> +struct apmf_verify_interface {
> +	u16 size;
> +	u16 version;
> +	u32 notification_mask;
> +	u32 supported_functions;
> +} __packed;
> +
> +struct apmf_system_params {
> +	u16 size;
> +	u32 valid_mask;
> +	u32 flags;
> +	u8 command_code;
> +} __packed;
> +
>  struct amd_pmf_dev {
>  	void __iomem *regbase;
>  	void __iomem *smu_virt_addr;
> @@ -37,10 +67,12 @@ struct amd_pmf_dev {
>  	u32 base_addr;
>  	u32 cpu_id;
>  	struct device *dev;
> +	struct apmf_if *apmf_if;
>  	struct mutex lock; /* protects the PMF interface */
>  };
>  
>  /* Core Layer */
> +int apmf_acpi_init(struct amd_pmf_dev *pmf_dev);
>  int amd_pmf_send_cmd(struct amd_pmf_dev *dev, u8 message, bool get, u32 arg, u32 *data);
>  
>  #endif /* PMF_H */
Hans de Goede Aug. 1, 2022, 1:18 p.m. UTC | #3
Hi,

On 8/1/22 14:48, Hans de Goede wrote:
> Hi,
> 
> On 7/28/22 20:20, Shyam Sundar S K wrote:
>> PMF driver implements the ACPI methods as defined by AMD for PMF Support.
>> The ACPI layer acts as a glue that helps in providing the infrastructure
>> for OEMs customization.
>>
>> OEMs can refer to PMF support documentation to decide on the list of
>> functions to be supported on their specific platform model.
>>
>> AMD mandates that PMF ACPI fn0 and fn1 to be implemented which
>> provides the set of functions, params and the notifications that
>> would be sent to PMF driver so that PMF driver can adapt and
>> react.
>>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>>  drivers/platform/x86/amd/pmf/Makefile |   2 +-
>>  drivers/platform/x86/amd/pmf/acpi.c   | 182 ++++++++++++++++++++++++++
>>  drivers/platform/x86/amd/pmf/core.c   |   1 +
>>  drivers/platform/x86/amd/pmf/pmf.h    |  32 +++++
>>  4 files changed, 216 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/platform/x86/amd/pmf/acpi.c
>>
>> diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
>> index 459005f659e5..2617eba773ce 100644
>> --- a/drivers/platform/x86/amd/pmf/Makefile
>> +++ b/drivers/platform/x86/amd/pmf/Makefile
>> @@ -5,4 +5,4 @@
>>  #
>>  
>>  obj-$(CONFIG_AMD_PMF) += amd-pmf.o
>> -amd-pmf-objs := core.o
>> +amd-pmf-objs := core.o acpi.o
>> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
>> new file mode 100644
>> index 000000000000..bd796d7e1d96
>> --- /dev/null
>> +++ b/drivers/platform/x86/amd/pmf/acpi.c
>> @@ -0,0 +1,182 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * AMD Platform Management Framework Driver
>> + *
>> + * Copyright (c) 2022, Advanced Micro Devices, Inc.
>> + * All Rights Reserved.
>> + *
>> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include "pmf.h"
>> +
>> +#define APMF_METHOD "\\_SB_.PMF_.APMF"
>> +
>> +static unsigned long supported_func;
>> +
>> +static void apmf_if_parse_functions(struct apmf_if_functions *func, u32 mask);
>> +
>> +static union acpi_object *apmf_if_call(struct apmf_if *apmf_if, int func, struct acpi_buffer *param)
>> +{
>> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>> +	struct acpi_object_list apmf_if_arg_list;
>> +	union acpi_object apmf_if_args[2];
>> +	acpi_status status;
>> +
>> +	apmf_if_arg_list.count = 2;
>> +	apmf_if_arg_list.pointer = &apmf_if_args[0];
>> +
>> +	apmf_if_args[0].type = ACPI_TYPE_INTEGER;
>> +	apmf_if_args[0].integer.value = func;
>> +
>> +	if (param) {
>> +		apmf_if_args[1].type = ACPI_TYPE_BUFFER;
>> +		apmf_if_args[1].buffer.length = param->length;
>> +		apmf_if_args[1].buffer.pointer = param->pointer;
>> +	} else {
>> +		apmf_if_args[1].type = ACPI_TYPE_INTEGER;
>> +		apmf_if_args[1].integer.value = 0;
>> +	}
>> +
>> +	status = acpi_evaluate_object(apmf_if->handle, NULL, &apmf_if_arg_list, &buffer);
>> +	if (ACPI_FAILURE(status)) {
>> +		pr_err("PMF: APMF method call failed\n");

Since the plan is to drop struct apmf_if and to directly store the
handle in amd_pmf_dev we will now have an amd_pmf_dev here so
please use dev_err() instead of pr_err() here.

>> +		if (status != AE_NOT_FOUND)
>> +			kfree(buffer.pointer);
>> +
>> +		return NULL;
>> +	}
>> +
>> +	return buffer.pointer;
>> +}
>> +
>> +static int apmf_if_call_store_buffer(struct apmf_if *apmf_if, int func, void *dest, size_t out_sz)
>> +{
>> +	union acpi_object *info;
>> +	size_t size;
>> +	int err = 0;
>> +
>> +	info = apmf_if_call(apmf_if, func, NULL);
>> +	if (!info)
>> +		return -EIO;
>> +
>> +	if (info->type != ACPI_TYPE_BUFFER) {
>> +		pr_err("object is not a buffer\n");

Since the plan is to drop struct apmf_if and to directly store the
handle in amd_pmf_dev we will now have an amd_pmf_dev here so
please use dev_err() instead of pr_err() here.

This also means that adding the:

#define pr_fmt(fmt) "AMD-PMF: " fmt

line will no longer be necessary.

>> +		err = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	if (info->buffer.length < 2) {
>> +		pr_err("buffer too small\n");
>> +		err = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	size = *(u16 *)info->buffer.pointer;
>> +	if (info->buffer.length < size) {
>> +		pr_err("buffer smaller then headersize %u < %zu\n",
>> +		       info->buffer.length, size);
>> +		err = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	if (size < out_sz) {
>> +		pr_err("buffer too small %zu\n", size);
>> +		err = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	size = out_sz;
>> +	memcpy(dest, info->buffer.pointer, size);
>> +
>> +out:
>> +	kfree(info);
>> +	return err;
>> +}
>> +
>> +static void apmf_if_parse_functions(struct apmf_if_functions *func, u32 mask)
>> +{
>> +	func->system_params = mask & APMF_FUNC_GET_SYS_PARAMS;
> 
> I just noticed that this is not correct, this should be:
> 
> 	func->system_params = mask & BIT(APMF_FUNC_GET_SYS_PARAMS - 1);
> 
> Which for APMF_FUNC_GET_SYS_PARAMS happens to be the same, but for
> other AMPF_FUNC_* values the difference does matter!
> 
> Also this information is duplicated by:
> 
> is_apmf_func_supported(APMF_FUNC_GET_SYS_PARAMS) please drop the
> apmf_if_functions struct and use is_apmf_func_supported() everywhere.
> 
> Note this means that you also will need to drop func->sbios_heartbeat
> which does not come from the mask and which is initialized in
> a different place rather then inside apmf_if_parse_functions() which
> already shows that sbios_hearbeat does not really belong inside
> this struct.
> 
> Instead of moving the sbios_heartbeat flag, please just directly
> use notify_cfg.hb_interval inside apmf_acpi_deinit(). This will also
> make the cancel_delayed_work_sync(&pmf_dev->heart_beat) condition
> match the queue condition 1:1.
> 
> 
>> +}
>> +
>> +int is_apmf_func_supported(unsigned long index)
>> +{
>> +	/* If bit-n is set, that indicates function n+1 is supported */
>> +	return ((supported_func & (1 << (index - 1))) != 0);
> 
> Please write this as:
> 
> 	return (supported_func & BIT(index - 1)) != 0;
> 
> or as:
> 
> 	return !!(supported_func & BIT(index - 1));
> 
> 

Also please make this function take a "struct amd_pmf_dev *pdev" argument
and add an "u32 supported_func" to amd_pmf_dev instead of making it a global
variable.

Regards,

Hans



>> +}
>> +
>> +static int apmf_if_verify_interface(struct amd_pmf_dev *pdev, struct apmf_if *apmf_if)
>> +{
>> +	struct apmf_verify_interface output;
>> +	int err;
>> +
>> +	err = apmf_if_call_store_buffer(apmf_if, APMF_FUNC_VERIFY_INTERFACE,
>> +					&output, sizeof(output));
>> +	if (err)
>> +		return err;
>> +
>> +	apmf_if_parse_functions(&apmf_if->func, output.supported_functions);
>> +	supported_func = output.supported_functions;
>> +	dev_dbg(pdev->dev, "supported functions:0x%x notifications:0x%x\n",
>> +		output.supported_functions, output.notification_mask);
>> +
>> +	return 0;
>> +}
>> +
>> +static int apmf_get_system_params(struct apmf_if *apmf_if)
>> +{
>> +	struct apmf_system_params params;
>> +	int err;
>> +
>> +	if (apmf_if->func.system_params) {
>> +		err = apmf_if_call_store_buffer(apmf_if, APMF_FUNC_GET_SYS_PARAMS,
>> +						&params, sizeof(params));
>> +		if (err)
>> +			return err;
>> +	}
>> +
>> +	pr_debug("PMF: system params mask:0x%x flags:0x%x cmd_code:0x%x\n",
>> +		 params.valid_mask,
>> +		 params.flags,
>> +		 params.command_code);
>> +	params.flags = params.flags & params.valid_mask;
>> +
>> +	return 0;
>> +}
>> +
>> +int apmf_acpi_init(struct amd_pmf_dev *pmf_dev)
>> +{
>> +	acpi_handle apmf_if_handle;
>> +	struct apmf_if *apmf_if;
>> +	acpi_status status;
>> +	int ret;
>> +
>> +	status = acpi_get_handle(NULL, (acpi_string) APMF_METHOD, &apmf_if_handle);
>> +	if (ACPI_FAILURE(status))
>> +		return -ENODEV;
>> +
>> +	apmf_if = kzalloc(sizeof(*apmf_if), GFP_KERNEL);
>> +	if (!apmf_if)
>> +		goto out;
>> +
>> +	apmf_if->handle = apmf_if_handle;
>> +
>> +	ret = apmf_if_verify_interface(pmf_dev, apmf_if);
>> +	if (ret) {
>> +		dev_err(pmf_dev->dev, "APMF verify interface failed :%d\n", ret);
>> +		kfree(apmf_if);
>> +		goto out;
>> +	}
>> +	pmf_dev->apmf_if = apmf_if;
> 
> After dropping the apmf_if_functions sub-struct from amp_if amp_if only contains
> an acpi_handle. Please just store the handle (and later on also the struct
> apmf_notification_cfg) directly inside struct amd_pmf_dev, this also removes
> the need for doing the whole alloc + free + associated error handling dance here.
> 
> Regsards,
> 
> Hans
> 
> 
>> +
>> +	ret = apmf_get_system_params(apmf_if);
>> +	if (ret) {
>> +		dev_err(pmf_dev->dev, "APMF apmf_get_system_params failed :%d\n", ret);
>> +		kfree(apmf_if);
>> +		goto out;
>> +	}
>> +
>> +out:
>> +	return ret;
>> +}
>> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
>> index aef97965c181..c5002b7bb904 100644
>> --- a/drivers/platform/x86/amd/pmf/core.c
>> +++ b/drivers/platform/x86/amd/pmf/core.c
>> @@ -204,6 +204,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
>>  	if (!dev->regbase)
>>  		return -ENOMEM;
>>  
>> +	apmf_acpi_init(dev);
>>  	platform_set_drvdata(pdev, dev);
>>  
>>  	mutex_init(&dev->lock);
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
>> index 1c2e942e5096..08c6bc0e2b42 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -11,6 +11,12 @@
>>  #ifndef PMF_H
>>  #define PMF_H
>>  
>> +#include <linux/acpi.h>
>> +
>> +/* APMF Functions */
>> +#define APMF_FUNC_VERIFY_INTERFACE			0
>> +#define APMF_FUNC_GET_SYS_PARAMS			1
>> +
>>  /* Message Definitions */
>>  #define SET_SPL				0x03 /* SPL: Sustained Power Limit */
>>  #define SET_SPPT			0x05 /* SPPT: Slow Package Power Tracking */
>> @@ -30,6 +36,30 @@
>>  #define GET_STT_LIMIT_APU	0x20
>>  #define GET_STT_LIMIT_HS2	0x21
>>  
>> +/* AMD PMF BIOS interfaces */
>> +struct apmf_if_functions {
>> +	bool system_params;
>> +};
>> +
>> +struct apmf_if {
>> +	acpi_handle handle;
>> +	struct apmf_if_functions func;
>> +};
>> +
>> +struct apmf_verify_interface {
>> +	u16 size;
>> +	u16 version;
>> +	u32 notification_mask;
>> +	u32 supported_functions;
>> +} __packed;
>> +
>> +struct apmf_system_params {
>> +	u16 size;
>> +	u32 valid_mask;
>> +	u32 flags;
>> +	u8 command_code;
>> +} __packed;
>> +
>>  struct amd_pmf_dev {
>>  	void __iomem *regbase;
>>  	void __iomem *smu_virt_addr;
>> @@ -37,10 +67,12 @@ struct amd_pmf_dev {
>>  	u32 base_addr;
>>  	u32 cpu_id;
>>  	struct device *dev;
>> +	struct apmf_if *apmf_if;
>>  	struct mutex lock; /* protects the PMF interface */
>>  };
>>  
>>  /* Core Layer */
>> +int apmf_acpi_init(struct amd_pmf_dev *pmf_dev);
>>  int amd_pmf_send_cmd(struct amd_pmf_dev *dev, u8 message, bool get, u32 arg, u32 *data);
>>  
>>  #endif /* PMF_H */
diff mbox series

Patch

diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
index 459005f659e5..2617eba773ce 100644
--- a/drivers/platform/x86/amd/pmf/Makefile
+++ b/drivers/platform/x86/amd/pmf/Makefile
@@ -5,4 +5,4 @@ 
 #
 
 obj-$(CONFIG_AMD_PMF) += amd-pmf.o
-amd-pmf-objs := core.o
+amd-pmf-objs := core.o acpi.o
diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
new file mode 100644
index 000000000000..bd796d7e1d96
--- /dev/null
+++ b/drivers/platform/x86/amd/pmf/acpi.c
@@ -0,0 +1,182 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD Platform Management Framework Driver
+ *
+ * Copyright (c) 2022, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ */
+
+#include <linux/acpi.h>
+#include "pmf.h"
+
+#define APMF_METHOD "\\_SB_.PMF_.APMF"
+
+static unsigned long supported_func;
+
+static void apmf_if_parse_functions(struct apmf_if_functions *func, u32 mask);
+
+static union acpi_object *apmf_if_call(struct apmf_if *apmf_if, int func, struct acpi_buffer *param)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_object_list apmf_if_arg_list;
+	union acpi_object apmf_if_args[2];
+	acpi_status status;
+
+	apmf_if_arg_list.count = 2;
+	apmf_if_arg_list.pointer = &apmf_if_args[0];
+
+	apmf_if_args[0].type = ACPI_TYPE_INTEGER;
+	apmf_if_args[0].integer.value = func;
+
+	if (param) {
+		apmf_if_args[1].type = ACPI_TYPE_BUFFER;
+		apmf_if_args[1].buffer.length = param->length;
+		apmf_if_args[1].buffer.pointer = param->pointer;
+	} else {
+		apmf_if_args[1].type = ACPI_TYPE_INTEGER;
+		apmf_if_args[1].integer.value = 0;
+	}
+
+	status = acpi_evaluate_object(apmf_if->handle, NULL, &apmf_if_arg_list, &buffer);
+	if (ACPI_FAILURE(status)) {
+		pr_err("PMF: APMF method call failed\n");
+		if (status != AE_NOT_FOUND)
+			kfree(buffer.pointer);
+
+		return NULL;
+	}
+
+	return buffer.pointer;
+}
+
+static int apmf_if_call_store_buffer(struct apmf_if *apmf_if, int func, void *dest, size_t out_sz)
+{
+	union acpi_object *info;
+	size_t size;
+	int err = 0;
+
+	info = apmf_if_call(apmf_if, func, NULL);
+	if (!info)
+		return -EIO;
+
+	if (info->type != ACPI_TYPE_BUFFER) {
+		pr_err("object is not a buffer\n");
+		err = -EINVAL;
+		goto out;
+	}
+
+	if (info->buffer.length < 2) {
+		pr_err("buffer too small\n");
+		err = -EINVAL;
+		goto out;
+	}
+
+	size = *(u16 *)info->buffer.pointer;
+	if (info->buffer.length < size) {
+		pr_err("buffer smaller then headersize %u < %zu\n",
+		       info->buffer.length, size);
+		err = -EINVAL;
+		goto out;
+	}
+
+	if (size < out_sz) {
+		pr_err("buffer too small %zu\n", size);
+		err = -EINVAL;
+		goto out;
+	}
+
+	size = out_sz;
+	memcpy(dest, info->buffer.pointer, size);
+
+out:
+	kfree(info);
+	return err;
+}
+
+static void apmf_if_parse_functions(struct apmf_if_functions *func, u32 mask)
+{
+	func->system_params = mask & APMF_FUNC_GET_SYS_PARAMS;
+}
+
+int is_apmf_func_supported(unsigned long index)
+{
+	/* If bit-n is set, that indicates function n+1 is supported */
+	return ((supported_func & (1 << (index - 1))) != 0);
+}
+
+static int apmf_if_verify_interface(struct amd_pmf_dev *pdev, struct apmf_if *apmf_if)
+{
+	struct apmf_verify_interface output;
+	int err;
+
+	err = apmf_if_call_store_buffer(apmf_if, APMF_FUNC_VERIFY_INTERFACE,
+					&output, sizeof(output));
+	if (err)
+		return err;
+
+	apmf_if_parse_functions(&apmf_if->func, output.supported_functions);
+	supported_func = output.supported_functions;
+	dev_dbg(pdev->dev, "supported functions:0x%x notifications:0x%x\n",
+		output.supported_functions, output.notification_mask);
+
+	return 0;
+}
+
+static int apmf_get_system_params(struct apmf_if *apmf_if)
+{
+	struct apmf_system_params params;
+	int err;
+
+	if (apmf_if->func.system_params) {
+		err = apmf_if_call_store_buffer(apmf_if, APMF_FUNC_GET_SYS_PARAMS,
+						&params, sizeof(params));
+		if (err)
+			return err;
+	}
+
+	pr_debug("PMF: system params mask:0x%x flags:0x%x cmd_code:0x%x\n",
+		 params.valid_mask,
+		 params.flags,
+		 params.command_code);
+	params.flags = params.flags & params.valid_mask;
+
+	return 0;
+}
+
+int apmf_acpi_init(struct amd_pmf_dev *pmf_dev)
+{
+	acpi_handle apmf_if_handle;
+	struct apmf_if *apmf_if;
+	acpi_status status;
+	int ret;
+
+	status = acpi_get_handle(NULL, (acpi_string) APMF_METHOD, &apmf_if_handle);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	apmf_if = kzalloc(sizeof(*apmf_if), GFP_KERNEL);
+	if (!apmf_if)
+		goto out;
+
+	apmf_if->handle = apmf_if_handle;
+
+	ret = apmf_if_verify_interface(pmf_dev, apmf_if);
+	if (ret) {
+		dev_err(pmf_dev->dev, "APMF verify interface failed :%d\n", ret);
+		kfree(apmf_if);
+		goto out;
+	}
+	pmf_dev->apmf_if = apmf_if;
+
+	ret = apmf_get_system_params(apmf_if);
+	if (ret) {
+		dev_err(pmf_dev->dev, "APMF apmf_get_system_params failed :%d\n", ret);
+		kfree(apmf_if);
+		goto out;
+	}
+
+out:
+	return ret;
+}
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index aef97965c181..c5002b7bb904 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -204,6 +204,7 @@  static int amd_pmf_probe(struct platform_device *pdev)
 	if (!dev->regbase)
 		return -ENOMEM;
 
+	apmf_acpi_init(dev);
 	platform_set_drvdata(pdev, dev);
 
 	mutex_init(&dev->lock);
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 1c2e942e5096..08c6bc0e2b42 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -11,6 +11,12 @@ 
 #ifndef PMF_H
 #define PMF_H
 
+#include <linux/acpi.h>
+
+/* APMF Functions */
+#define APMF_FUNC_VERIFY_INTERFACE			0
+#define APMF_FUNC_GET_SYS_PARAMS			1
+
 /* Message Definitions */
 #define SET_SPL				0x03 /* SPL: Sustained Power Limit */
 #define SET_SPPT			0x05 /* SPPT: Slow Package Power Tracking */
@@ -30,6 +36,30 @@ 
 #define GET_STT_LIMIT_APU	0x20
 #define GET_STT_LIMIT_HS2	0x21
 
+/* AMD PMF BIOS interfaces */
+struct apmf_if_functions {
+	bool system_params;
+};
+
+struct apmf_if {
+	acpi_handle handle;
+	struct apmf_if_functions func;
+};
+
+struct apmf_verify_interface {
+	u16 size;
+	u16 version;
+	u32 notification_mask;
+	u32 supported_functions;
+} __packed;
+
+struct apmf_system_params {
+	u16 size;
+	u32 valid_mask;
+	u32 flags;
+	u8 command_code;
+} __packed;
+
 struct amd_pmf_dev {
 	void __iomem *regbase;
 	void __iomem *smu_virt_addr;
@@ -37,10 +67,12 @@  struct amd_pmf_dev {
 	u32 base_addr;
 	u32 cpu_id;
 	struct device *dev;
+	struct apmf_if *apmf_if;
 	struct mutex lock; /* protects the PMF interface */
 };
 
 /* Core Layer */
+int apmf_acpi_init(struct amd_pmf_dev *pmf_dev);
 int amd_pmf_send_cmd(struct amd_pmf_dev *dev, u8 message, bool get, u32 arg, u32 *data);
 
 #endif /* PMF_H */