On Sat, 30 Sep 2023, Shyam Sundar S K wrote:
> In the current code, the metrics table information was required only
> for auto-mode or CnQF at a given time. Hence keeping the return type
> of amd_pmf_set_dram_addr() as static made sense.
>
> But with the addition of Smart PC builder feature, the metrics table
> information has to be shared by the Smart PC also and this feature
> resides outside of core.c.
>
> To make amd_pmf_set_dram_addr() visible outside of core.c make it
> as a non-static function and move the allocation of memory for
> metrics table from amd_pmf_init_metrics_table() to amd_pmf_set_dram_addr()
> as amd_pmf_set_dram_addr() is the common function to set the DRAM
> address.
>
> Reviewed-by: Mario Limonciello <mario.limoncie...@amd.com>
> Signed-off-by: Shyam Sundar S K <shyam-sundar....@amd.com>
> ---
> drivers/platform/x86/amd/pmf/core.c | 26 ++++++++++++++++++--------
> drivers/platform/x86/amd/pmf/pmf.h | 1 +
> 2 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/core.c
> b/drivers/platform/x86/amd/pmf/core.c
> index 68f1389dda3e..678dce4fea08 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -251,29 +251,35 @@ static const struct pci_device_id pmf_pci_ids[] = {
> { }
> };
>
> -static void amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
> +int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev)
> {
> u64 phys_addr;
> u32 hi, low;
>
> + /* Get Metrics Table Address */
> + dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
> + if (!dev->buf)
> + return -ENOMEM;
> +
> phys_addr = virt_to_phys(dev->buf);
> hi = phys_addr >> 32;
> low = phys_addr & GENMASK(31, 0);
>
> amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
> amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
> +
> + return 0;
> }
>
> int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
> {
> - /* Get Metrics Table Address */
> - dev->buf = kzalloc(sizeof(dev->m_table), GFP_KERNEL);
> - if (!dev->buf)
> - return -ENOMEM;
> + int ret;
>
> INIT_DELAYED_WORK(&dev->work_buffer, amd_pmf_get_metrics);
>
> - amd_pmf_set_dram_addr(dev);
> + ret = amd_pmf_set_dram_addr(dev);
> + if (ret)
> + return ret;
>
> /*
> * Start collecting the metrics data after a small delay
> @@ -287,9 +293,13 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
> static int amd_pmf_resume_handler(struct device *dev)
> {
> struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
> + int ret;
>
> - if (pdev->buf)
> - amd_pmf_set_dram_addr(pdev);
> + if (pdev->buf) {
> + ret = amd_pmf_set_dram_addr(pdev);
Won't this now leak the previous ->buf?
> + if (ret)
> + return ret;
> + }
>
> return 0;
> }
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
> b/drivers/platform/x86/amd/pmf/pmf.h
> index e0837799f521..3930b8ed8333 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -421,6 +421,7 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev);
> int amd_pmf_get_power_source(void);
> int apmf_install_handler(struct amd_pmf_dev *pmf_dev);
> int apmf_os_power_slider_update(struct amd_pmf_dev *dev, u8 flag);
> +int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev);
>
> /* SPS Layer */
> int amd_pmf_get_pprof_modes(struct amd_pmf_dev *pmf);
>
--
i.
