On Fri, 22 Sep 2023, Shyam Sundar S K wrote:
Add () to the function name in the shortlog. "Change signature" is quite
vague, perhaps you could come up something more descriptive.
> Make amd_pmf_set_dram_addr() as non-static so that same function
> can be used across files.
This says nothing about the move of allocation.
> 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 | 14 ++++++++------
> drivers/platform/x86/amd/pmf/pmf.h | 1 +
> 2 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/core.c
> b/drivers/platform/x86/amd/pmf/core.c
> index 68f1389dda3e..5fb03ed614ff 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -251,26 +251,28 @@ 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;
> -
> INIT_DELAYED_WORK(&dev->work_buffer, amd_pmf_get_metrics);
>
> amd_pmf_set_dram_addr(dev);
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
> b/drivers/platform/x86/amd/pmf/pmf.h
> index a9333ff6c0a7..ea15ce547d24 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);
>
Why are not amd_pmf_set_dram_addr() callers made to handle/pass on
errors???
--
i.
