On Tue, 15 Oct 2024, Shyam Sundar S K wrote:
> Use platform_get_resource() to fetch the memory resource instead of
> acpi_walk_resources() and devm_ioremap_resource() for mapping the
> resources.
>
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@xxxxxxx>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@xxxxxxx>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> ---
> drivers/platform/x86/amd/pmf/Kconfig | 1 +
> drivers/platform/x86/amd/pmf/acpi.c | 37 ++++++++-------------------
> drivers/platform/x86/amd/pmf/pmf.h | 6 +++--
> drivers/platform/x86/amd/pmf/tee-if.c | 8 +++---
> 4 files changed, 20 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
> index f4fa8bd8bda8..99d67cdbd91e 100644
> --- a/drivers/platform/x86/amd/pmf/Kconfig
> +++ b/drivers/platform/x86/amd/pmf/Kconfig
> @@ -11,6 +11,7 @@ config AMD_PMF
> select ACPI_PLATFORM_PROFILE
> depends on TEE && AMDTEE
> depends on AMD_SFH_HID
> + depends on HAS_IOMEM
> help
> This driver provides support for the AMD Platform Management Framework.
> The goal is to enhance end user experience by making AMD PCs smarter,
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index d5b496433d69..2d871ff74fa7 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -433,37 +433,22 @@ int apmf_install_handler(struct amd_pmf_dev *pmf_dev)
> return 0;
> }
>
> -static acpi_status apmf_walk_resources(struct acpi_resource *res, void *data)
> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev)
> {
> - struct amd_pmf_dev *dev = data;
> + struct platform_device *pdev = to_platform_device(pmf_dev->dev);
>
> - switch (res->type) {
> - case ACPI_RESOURCE_TYPE_ADDRESS64:
> - dev->policy_addr = res->data.address64.address.minimum;
> - dev->policy_sz = res->data.address64.address.address_length;
> - break;
> - case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> - dev->policy_addr = res->data.fixed_memory32.address;
> - dev->policy_sz = res->data.fixed_memory32.address_length;
> - break;
> - }
> -
> - if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ || dev->policy_sz == 0) {
> - pr_err("Incorrect Policy params, possibly a SBIOS bug\n");
> - return AE_ERROR;
> + pmf_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!pmf_dev->res) {
> + dev_err(pmf_dev->dev, "Failed to get I/O memory resource\n");
> + return -EINVAL;
> }
>
> - return AE_OK;
> -}
> + pmf_dev->policy_addr = pmf_dev->res->start;
> + pmf_dev->policy_sz = pmf_dev->res->end - pmf_dev->res->start;
A small thing still, I realized this should really have a comment because
it has a big risk of getting "fixed".
Also please describe what's going on here in the changelog (answer "why?")
since this is such a thing that people who look this code from history
have zero chance on figuring out the reasoning on their own.
--
i.
> -int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev)
> -{
> - acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev);
> - acpi_status status;
> -
> - status = acpi_walk_resources(ahandle, METHOD_NAME__CRS, apmf_walk_resources, pmf_dev);
> - if (ACPI_FAILURE(status)) {
> - dev_dbg(pmf_dev->dev, "acpi_walk_resources failed :%d\n", status);
> + if (!pmf_dev->policy_addr || pmf_dev->policy_sz > POLICY_BUF_MAX_SZ ||
> + pmf_dev->policy_sz == 0) {
> + dev_err(pmf_dev->dev, "Incorrect policy params, possibly a SBIOS bug\n");
> return -EINVAL;
> }
>
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 8ce8816da9c1..a79808fda1d8 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -13,6 +13,7 @@
>
> #include <linux/acpi.h>
> #include <linux/input.h>
> +#include <linux/platform_device.h>
> #include <linux/platform_profile.h>
>
> #define POLICY_BUF_MAX_SZ 0x4b000
> @@ -355,19 +356,20 @@ struct amd_pmf_dev {
> /* Smart PC solution builder */
> struct dentry *esbin;
> unsigned char *policy_buf;
> - u32 policy_sz;
> + resource_size_t policy_sz;
> struct tee_context *tee_ctx;
> struct tee_shm *fw_shm_pool;
> u32 session_id;
> void *shbuf;
> struct delayed_work pb_work;
> struct pmf_action_table *prev_data;
> - u64 policy_addr;
> + resource_size_t policy_addr;
> void __iomem *policy_base;
> bool smart_pc_enabled;
> u16 pmf_if_version;
> struct input_dev *pmf_idev;
> size_t mtable_size;
> + struct resource *res;
> };
>
> struct apmf_sps_prop_granular_v2 {
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 19c27b6e4666..555b8d6314e0 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -257,7 +257,7 @@ static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev)
> return -ENODEV;
> }
>
> - dev_dbg(dev->dev, "Policy Binary size: %u bytes\n", dev->policy_sz);
> + dev_dbg(dev->dev, "Policy Binary size: %llu bytes\n", dev->policy_sz);
> memset(dev->shbuf, 0, dev->policy_sz);
> ta_sm = dev->shbuf;
> in = &ta_sm->pmf_input.init_table;
> @@ -512,9 +512,9 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
> if (ret)
> goto error;
>
> - dev->policy_base = devm_ioremap(dev->dev, dev->policy_addr, dev->policy_sz);
> - if (!dev->policy_base) {
> - ret = -ENOMEM;
> + dev->policy_base = devm_ioremap_resource(dev->dev, dev->res);
> + if (IS_ERR(dev->policy_base)) {
> + ret = PTR_ERR(dev->policy_base);
> goto error;
> }
>
>
