Hi Shyam,
On 10/9/23 11:45, Shyam Sundar S K wrote:
> Move fsize, stb_rdptr_offset, num_samples to struct amd_pmc_dev so
> that these variables are accessible across functions.
>
> Signed-off-by: Sanket Goswami <Sanket.Goswami@xxxxxxx>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
These variables are only valid temporarily, they can change over
possibly multiple read calls racing. Therefor these MUST NOT
be stored in a global data struct like this.
If you need access to some of these in the new
amd_pmc_stb_handle_efr() introduced in patch 4/4 then please
just pass them as function parameters.
As for accessing the buffer size in amd_pmc_stb_debugfs_read_v2()
I have explained how to do that properly in my review of patch 1/4.
Regards,
Hans
> ---
> v5:
> - new patch based on comments in v4 from Hans.
> - based on review-ilpo branch
>
> drivers/platform/x86/amd/pmc/pmc.c | 24 +++++++++++++-----------
> drivers/platform/x86/amd/pmc/pmc.h | 3 +++
> 2 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index 67daa655cc6a..071d92b7fbde 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -244,8 +244,8 @@ static const struct file_operations amd_pmc_stb_debugfs_fops = {
> static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
> {
> struct amd_pmc_dev *dev = filp->f_inode->i_private;
> - u32 fsize, num_samples, val, stb_rdptr_offset = 0;
> struct amd_pmc_stb_v2_data *flex_arr;
> + u32 val;
> int ret;
>
> /* Write dummy postcode while reading the STB buffer */
> @@ -261,7 +261,7 @@ static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
> dev_warn_once(dev->dev, "S2D force flush not supported\n");
>
> /* Get the num_samples to calculate the last push location */
> - ret = amd_pmc_send_cmd(dev, S2D_NUM_SAMPLES, &num_samples, dev->s2d_msg_id, true);
> + ret = amd_pmc_send_cmd(dev, S2D_NUM_SAMPLES, &dev->num_samples, dev->s2d_msg_id, true);
> /* Clear msg_port for other SMU operation */
> dev->msg_port = 0;
> if (ret) {
> @@ -270,29 +270,31 @@ static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
> }
>
> /* Start capturing data from the last push location */
> - if (num_samples > S2D_TELEMETRY_BYTES_MAX) {
> + if (dev->num_samples > S2D_TELEMETRY_BYTES_MAX) {
> /* First read oldest data starting 1 behind last write till end of ringbuffer */
> - stb_rdptr_offset = num_samples % S2D_TELEMETRY_BYTES_MAX;
> - fsize = S2D_TELEMETRY_BYTES_MAX - stb_rdptr_offset;
> + dev->stb_rdptr_offset = dev->num_samples % S2D_TELEMETRY_BYTES_MAX;
> + dev->fsize = S2D_TELEMETRY_BYTES_MAX - dev->stb_rdptr_offset;
>
> flex_arr = kzalloc(struct_size(flex_arr, data, S2D_TELEMETRY_BYTES_MAX),
> GFP_KERNEL);
> if (!flex_arr)
> return -ENOMEM;
>
> - memcpy_fromio(flex_arr->data, dev->stb_virt_addr + stb_rdptr_offset, fsize);
> + memcpy_fromio(flex_arr->data, dev->stb_virt_addr + dev->stb_rdptr_offset,
> + dev->fsize);
> /* Second copy the newer samples from offset 0 - last write */
> - memcpy_fromio(flex_arr->data + fsize, dev->stb_virt_addr, stb_rdptr_offset);
> + memcpy_fromio(flex_arr->data + dev->fsize, dev->stb_virt_addr,
> + dev->stb_rdptr_offset);
> } else {
> - fsize = num_samples;
> - flex_arr = kzalloc(struct_size(flex_arr, data, num_samples), GFP_KERNEL);
> + dev->fsize = dev->num_samples;
> + flex_arr = kzalloc(struct_size(flex_arr, data, dev->num_samples), GFP_KERNEL);
> if (!flex_arr)
> return -ENOMEM;
>
> - memcpy_fromio(flex_arr->data, dev->stb_virt_addr, num_samples);
> + memcpy_fromio(flex_arr->data, dev->stb_virt_addr, dev->num_samples);
> }
>
> - flex_arr->size = fsize;
> + flex_arr->size = dev->fsize;
> filp->private_data = flex_arr->data;
>
> return 0;
> diff --git a/drivers/platform/x86/amd/pmc/pmc.h b/drivers/platform/x86/amd/pmc/pmc.h
> index c27bd6a5642f..12728eedecda 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.h
> +++ b/drivers/platform/x86/amd/pmc/pmc.h
> @@ -26,6 +26,9 @@ struct amd_pmc_dev {
> u32 dram_size;
> u32 num_ips;
> u32 s2d_msg_id;
> + u32 fsize;
> + u32 num_samples;
> + u32 stb_rdptr_offset;
> /* SMU version information */
> u8 smu_program;
> u8 major;
