Re: [PATCH 2/4] platform/x86/amd/pmc: Read SMU version at the time of probe

["Limonciello, Mario" <mario.limonciello@amd.com>]

On 8/11/2023 6:21 AM, Shyam Sundar S K wrote:
> Currently the SMU version is being read at multiple places, unify all
> of them and get the SMU version at the time of probe.
> 
> 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 | 39 +++++-------------------------
>   1 file changed, 6 insertions(+), 33 deletions(-)
> 

It's actually quite intentional that the SMU version isn't read at probe 
but rather at first use.  The reason is that it increased boot time in a 
measurable way.

> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index c1e788b67a74..043451fabbbe 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -405,12 +405,6 @@ static ssize_t smu_fw_version_show(struct device *d, struct device_attribute *at
>   {
>   	struct amd_pmc_dev *dev = dev_get_drvdata(d);
>   
> -	if (!dev->major) {
> -		int rc = amd_pmc_get_smu_version(dev);
> -
> -		if (rc)
> -			return rc;
> -	}
>   	return sysfs_emit(buf, "%u.%u.%u\n", dev->major, dev->minor, dev->rev);
>   }
>   
> @@ -419,12 +413,6 @@ static ssize_t smu_program_show(struct device *d, struct device_attribute *attr,
>   {
>   	struct amd_pmc_dev *dev = dev_get_drvdata(d);
>   
> -	if (!dev->major) {
> -		int rc = amd_pmc_get_smu_version(dev);
> -
> -		if (rc)
> -			return rc;
> -	}
>   	return sysfs_emit(buf, "%u\n", dev->smu_program);
>   }
>   
> @@ -526,16 +514,9 @@ static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev,
>   				 struct seq_file *s)
>   {
>   	u32 val;
> -	int rc;
>   
>   	switch (pdev->cpu_id) {
>   	case AMD_CPU_ID_CZN:
> -		/* we haven't yet read SMU version */
> -		if (!pdev->major) {
> -			rc = amd_pmc_get_smu_version(pdev);
> -			if (rc)
> -				return rc;
> -		}
>   		if (pdev->major > 56 || (pdev->major >= 55 && pdev->minor >= 37))
>   			val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_CZN);
>   		else
> @@ -717,13 +698,6 @@ static int amd_pmc_get_os_hint(struct amd_pmc_dev *dev)
>   static int amd_pmc_czn_wa_irq1(struct amd_pmc_dev *pdev)
>   {
>   	struct device *d;
> -	int rc;
> -
> -	if (!pdev->major) {
> -		rc = amd_pmc_get_smu_version(pdev);
> -		if (rc)
> -			return rc;
> -	}
>   
>   	if (pdev->major > 64 || (pdev->major == 64 && pdev->minor > 65))
>   		return 0;
> @@ -749,13 +723,6 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
>   	struct rtc_time tm;
>   	int rc;
>   
> -	/* we haven't yet read SMU version */
> -	if (!pdev->major) {
> -		rc = amd_pmc_get_smu_version(pdev);
> -		if (rc)
> -			return rc;
> -	}
> -
>   	if (pdev->major < 64 || (pdev->major == 64 && pdev->minor < 53))
>   		return 0;
>   
> @@ -1059,6 +1026,12 @@ static int amd_pmc_probe(struct platform_device *pdev)
>   
>   	mutex_init(&dev->lock);
>   
> +	err = amd_pmc_get_smu_version(dev);
> +	if (err) {
> +		dev_err(dev->dev, "error reading SMU version\n");
> +		goto err_pci_dev_put;
> +	}
> +
>   	if (enable_stb && amd_pmc_is_stb_supported(dev)) {
>   		err = amd_pmc_s2d_init(dev);
>   		if (err)