On 11/5/2024 15:14, Ilpo Järvinen wrote:
> On Tue, 5 Nov 2024, Shyam Sundar S K wrote:
>> On 11/1/2024 15:58, Ilpo Järvinen wrote:
>>> On Tue, 29 Oct 2024, Shyam Sundar S K wrote:
>>>
>>>> To distinguish between the PMC message port and the S2D (Spill to DRAM)
>>>> message port, replace the use of 0 and 1 with an enum.
>>>>
>>>> Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx>
>>>> Co-developed-by: Sanket Goswami <Sanket.Goswami@xxxxxxx>
>>>> Signed-off-by: Sanket Goswami <Sanket.Goswami@xxxxxxx>
>>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
>>>> ---
>>>> drivers/platform/x86/amd/pmc/mp1_stb.c | 13 +++++++++----
>>>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/amd/pmc/mp1_stb.c b/drivers/platform/x86/amd/pmc/mp1_stb.c
>>>> index 5efec020ecac..2b06861c479b 100644
>>>> --- a/drivers/platform/x86/amd/pmc/mp1_stb.c
>>>> +++ b/drivers/platform/x86/amd/pmc/mp1_stb.c
>>>> @@ -47,6 +47,11 @@ enum s2d_arg {
>>>> S2D_DRAM_SIZE,
>>>> };
>>>>
>>>> +enum s2d_msg_port {
>>>> + MSG_PORT_PMC,
>>>> + MSG_PORT_S2D,
>>>> +};
>>>> +
>>>> struct amd_stb_v2_data {
>>>> size_t size;
>>>> u8 data[] __counted_by(size);
>>>> @@ -156,7 +161,7 @@ static int amd_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
>>>> dev_err(dev->dev, "error writing to STB: %d\n", ret);
>>>>
>>>> /* Spill to DRAM num_samples uses separate SMU message port */
>>>> - dev->msg_port = 1;
>>>> + dev->msg_port = MSG_PORT_S2D;
>>>>
>>>> ret = amd_pmc_send_cmd(dev, 0, &val, STB_FORCE_FLUSH_DATA, 1);
>>>> if (ret)
>>>> @@ -173,7 +178,7 @@ static int amd_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
>>>> /* 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);
>>>> /* Clear msg_port for other SMU operation */
>>>> - dev->msg_port = 0;
>>>> + dev->msg_port = MSG_PORT_PMC;
>>>> if (ret) {
>>>> dev_err(dev->dev, "error: S2D_NUM_SAMPLES not supported : %d\n", ret);
>>>> return ret;
>>>> @@ -266,7 +271,7 @@ int amd_s2d_init(struct amd_pmc_dev *dev)
>>>> &amd_stb_debugfs_fops);
>>>>
>>>> /* Spill to DRAM feature uses separate SMU message port */
>>>> - dev->msg_port = 1;
>>>> + dev->msg_port = MSG_PORT_S2D;
>>>>
>>>> amd_pmc_send_cmd(dev, S2D_TELEMETRY_SIZE, &size, dev->s2d_msg_id, true);
>>>> if (size != S2D_TELEMETRY_BYTES_MAX)
>>>> @@ -284,7 +289,7 @@ int amd_s2d_init(struct amd_pmc_dev *dev)
>>>> stb_phys_addr = ((u64)phys_addr_hi << 32 | phys_addr_low);
>>>>
>>>> /* Clear msg_port for other SMU operation */
>>>> - dev->msg_port = 0;
>>>> + dev->msg_port = MSG_PORT_PMC;
>>>>
>>>> dev->stb_virt_addr = devm_ioremap(dev->dev, stb_phys_addr, dev->dram_size);
>>>> if (!dev->stb_virt_addr)
>>>
>>> This change is incomplete, you missed all places using it:
>>>
>>> if (dev->msg_port) {
>>>
>>> + add helper for this:
>>>
>>> dev->msg_port ? "S2D" : "PMC"
>>>
>>
>>
>> I am not sure if I understand your comment fully. Can you please
>> elaborate?
>
> There are users of dev->msg_port that should be also touched by this
> change but weren't.
>
> For the printing, I suggested a helper function which returns the correct
> string so you don't need to do the compare within print argument.
>
Got it. I have addressed this helper function in 8/13 of v3 just sent
out now. Please have a look at it.
Thanks,
Shyam
