Re: [PATCH v4 01/11] platform/x86/amd/pmc: Move STB block into amd_pmc_s2d_init()

[Shyam Sundar S K <Shyam-sundar.S-k@amd.com>]

On 11/7/2024 16:10, Ilpo Järvinen wrote:
> On Thu, 7 Nov 2024, Shyam Sundar S K wrote:
> 
>> Transfer the support for STB-related file operations to the
>> amd_pmc_s2d_init() function, thereby consolidating the STB and S2D
>> (Spill to DRAM) functionality in one location.
>>
>> For older platforms that supported S2D, exit immediately after creating
>> debugfs. These platforms may not support the PMFW messages available on
>> newer platforms. This adjustment is necessary due to the relocation of
>> debugfs creation into amd_pmc_s2d_init().
>>
>> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>> Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
>> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>>  drivers/platform/x86/amd/pmc/pmc.c | 18 +++++++++---------
>>  1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
>> index bbb8edb62e00..54ceb2f9bf56 100644
>> --- a/drivers/platform/x86/amd/pmc/pmc.c
>> +++ b/drivers/platform/x86/amd/pmc/pmc.c
>> @@ -648,15 +648,6 @@ static void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
>>  			    &s0ix_stats_fops);
>>  	debugfs_create_file("amd_pmc_idlemask", 0644, dev->dbgfs_dir, dev,
>>  			    &amd_pmc_idlemask_fops);
>> -	/* Enable STB only when the module_param is set */
>> -	if (enable_stb) {
>> -		if (amd_pmc_is_stb_supported(dev))
>> -			debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev,
>> -					    &amd_pmc_stb_debugfs_fops_v2);
>> -		else
>> -			debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev,
>> -					    &amd_pmc_stb_debugfs_fops);
>> -	}
>>  }
>>  
>>  static void amd_pmc_dump_registers(struct amd_pmc_dev *dev)
>> @@ -982,6 +973,15 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev)
>>  	u32 size = 0;
>>  	int ret;
>>  
>> +	if (amd_pmc_is_stb_supported(dev)) {
>> +		debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev,
>> +				    &amd_pmc_stb_debugfs_fops_v2);
>> +	} else {
>> +		debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev,
>> +				    &amd_pmc_stb_debugfs_fops);
>> +		return 0;
>> +	}
>> +
>>  	/* Spill to DRAM feature uses separate SMU message port */
>>  	dev->msg_port = 1;
> 
> This now runs afoul the other issue you even mentioned yourself (IIRC), 
> that is, dev->dbgfs_dir is initialized inside amd_pmc_dbgfs_register() 
> which is only called after amd_pmc_s2d_init() until it is moved in patch 
> 2.
> 
> Thus, you need to combine patches 1 & 2 so you don't get a broken kernel
> after this patch.
> 
> Please also move the enable_stb check inside amd_pmc_s2d_init() in this 
> patch since that's another thing you've now broken in between patches 1 & 
> 3.
> 
> So to reiterate, in the first patch combine: Patch 1 + 2 + the if () logic 
> move from amd_pmc_probe() into amd_pmc_s2d_init().
> 

OK. Will do it.

Thanks,
Shyam