Re: [PATCH v8 3/3] platform/x86/amd/pmc: Add dump_custom_stb module parameter

From: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	markgross@kernel.org, Sanket.Goswami@amd.com,
	mario.limonciello@amd.com, platform-driver-x86@vger.kernel.org,
	Harsh Jain <Harsh.Jain@amd.com>
Subject: Re: [PATCH v8 3/3] platform/x86/amd/pmc: Add dump_custom_stb module parameter
Date: Fri, 13 Oct 2023 12:52:49 +0530	[thread overview]
Message-ID: <97ee4e13-5c96-4857-8e08-b4559feed3eb@amd.com> (raw)
In-Reply-To: <5f3241ba-a9a0-10c5-f139-c6bb42c75ac9@linux.intel.com>

On 10/12/2023 7:38 PM, Ilpo Järvinen wrote:
> On Tue, 10 Oct 2023, Shyam Sundar S K wrote:
> 
>> There have been instances when the default size (1M) of the STB is not
>> sufficient to get the complete traces of the failure. In such scenarios
>> we can use a module_param to enable full trace that shall contain more
>> debugging data. This is not a regular case and hence not enabling this
>> capability by default.
>>
>> With this change, there will be two cases on how the driver fetches the
>> stb data:
>> 1) A special case (proposed now) - which is required only for certain
>> platforms. Here, a new module param will be supplied to the driver that
>> will have a special PMFW supporting enhanced dram sizes for getting
>> the stb data. Without the special PMFW support, just setting the module
>> param will not help to get the enhanced stb data.
>>
>> To adapt to this change, we will have a new amd_pmc_stb_handle_efr() to
>> handle enhanced firmware reporting mechanism. Note that, since num_samples
>> based r/w pointer offset calculation is not required for enhanced firmware
>> reporting we will have this mailbox command sent only in case of regular
>> STB cases.
>>
>> 2) Current code branch which fetches the stb data based on the parameters
>> like the num_samples, fsize and the r/w pointer.
>>
>> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>> Co-developed-by: Harsh Jain <Harsh.Jain@amd.com>
>> Signed-off-by: Harsh Jain <Harsh.Jain@amd.com>
>> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> 
> Applied to review-ilpo with following changes and notes:
> 
> - I renamed flex_arr -> stb_data_arr (in patches 2 and 3)
> 
> - I had to resolve a conflict in patch 3 with another addition, which
>   also came from you. Please try to proactively construct your series
>   such that this kind of problems are avoided (preferrable) or, if
>   unavoidable, at least note the existance of the conflict.
> 
> Please check I didn't break anything with my conflict resolution & renames.
> 

Thanks Ilpo, everything seems to be working even after your edits.

Thanks,
Shyam