diff mbox series

[v5,3/4] platform/x86/amd/pmc: move some variables to struct amd_pmc_dev

Message ID 20231009094539.6746-3-Shyam-sundar.S-k@amd.com (mailing list archive)
State Changes Requested, archived
Headers show
Series [v5,1/4] platform/x86/amd/pmc: Use flex array when calling amd_pmc_stb_debugfs_open_v2() | expand

Commit Message

Shyam Sundar S K Oct. 9, 2023, 9:45 a.m. UTC 
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@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
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(-)

Comments

Hans de Goede Oct. 9, 2023, 10:09 a.m. UTC | #1 
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@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>

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;
diff mbox series

Patch

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;