Re: [PATCH v2 0/7] updates to amd-pmc driver

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

Hi Hans,

On 6/24/2021 2:14 AM, Hans de Goede wrote:
> Hi Shyam,
> 
> 
> On 6/23/21 10:01 PM, Shyam Sundar S K wrote:
>> Couple of existing issues on the completion codes to SMU
>> and a newer way to get the s0ix statistics are introduced.
>>
>> Also, add acpi ids for current and future revisions of the
>> pmc controller.
>>
>> Shyam Sundar S K (7):
>>   platform/x86: amd-pmc: Fix command completion code
>>   platform/x86: amd-pmc: Fix SMU firmware reporting mechanism
>>   platform/x86: amd-pmc: call dump registers only once
>>   platform/x86: amd-pmc: Add support for logging SMU metrics
>>   platform/x86: amd-pmc: Add support for logging s0ix counters
>>   platform/x86: amd-pmc: Add support for ACPI ID AMDI0006
>>   platform/x86: amd-pmc: Add new acpi id for future PMC controllers
> 
> Thank you for the new version.
> 
> Can you please respond to Raul's reply to patch 1/6 of v1 of
> the series?  Raul's remark seems very relevant to me.

Unfortunately, I could not find Raul's email in my inbox and hence I
missed to reply.

Hi Raul,

The break condition for readx_poll_timeout() should be 'val ==
AMD_PMC_RESULT_OK'. Reason being:

The smu firmware spec says, we have to wait until the response register
says 1, meaning it is ready to take the job requests. If it is anything
apart from 1, it means it's not in a state to take any requests.

And yes, response register always returns 'val > 0'. Refer to
AMD_PMC_RESULT_* macros.

Maybe instead of doing 'return rc', should I just do 'return -EBUSY' ?

+	if (rc) {
+		dev_err(dev->dev, "SMU response timed out\n");
+		return rc;
+	}
+  	return 0;

Am I missing anything obvious frmo your comments?

Thanks,
Shyam