Currently if STB is disabled but an earlier function reported an
error an incorrect error will be emitted about failing to write to
STB.

Correct this logic error by only showing errors when STB is enabled.

Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/amd-pmc.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
index 7317993cd91b..e9d0dbbb2887 100644
--- a/drivers/platform/x86/amd-pmc.c
+++ b/drivers/platform/x86/amd-pmc.c
@@ -655,10 +655,11 @@ static void amd_pmc_s2idle_prepare(void)
 		return;
 	}
 
-	if (enable_stb)
+	if (enable_stb) {
 		rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF);
-	if (rc)
-		dev_err(pdev->dev, "error writing to STB: %d\n", rc);
+		if (rc)
+			dev_err(pdev->dev, "error writing to STB: %d\n", rc);
+	}
 }
 
 static void amd_pmc_s2idle_restore(void)
@@ -679,10 +680,11 @@ static void amd_pmc_s2idle_restore(void)
 	amd_pmc_idlemask_read(pdev, pdev->dev, NULL);
 
 	/* Write data incremented by 1 to distinguish in stb_read */
-	if (enable_stb)
+	if (enable_stb) {
 		rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF + 1);
-	if (rc)
-		dev_err(pdev->dev, "error writing to STB: %d\n", rc);
+		if (rc)
+			dev_err(pdev->dev, "error writing to STB: %d\n", rc);
+	}
 
 	/* Notify on failed entry */
 	amd_pmc_validate_deepest(pdev);
-- 
2.34.1

Hi,

On 3/17/22 20:03, Mario Limonciello wrote:
> Currently if STB is disabled but an earlier function reported an
> error an incorrect error will be emitted about failing to write to
> STB.
> 
> Correct this logic error by only showing errors when STB is enabled.
> 
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans


> ---
>  drivers/platform/x86/amd-pmc.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
> index 7317993cd91b..e9d0dbbb2887 100644
> --- a/drivers/platform/x86/amd-pmc.c
> +++ b/drivers/platform/x86/amd-pmc.c
> @@ -655,10 +655,11 @@ static void amd_pmc_s2idle_prepare(void)
>  		return;
>  	}
>  
> -	if (enable_stb)
> +	if (enable_stb) {
>  		rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF);
> -	if (rc)
> -		dev_err(pdev->dev, "error writing to STB: %d\n", rc);
> +		if (rc)
> +			dev_err(pdev->dev, "error writing to STB: %d\n", rc);
> +	}
>  }
>  
>  static void amd_pmc_s2idle_restore(void)
> @@ -679,10 +680,11 @@ static void amd_pmc_s2idle_restore(void)
>  	amd_pmc_idlemask_read(pdev, pdev->dev, NULL);
>  
>  	/* Write data incremented by 1 to distinguish in stb_read */
> -	if (enable_stb)
> +	if (enable_stb) {
>  		rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF + 1);
> -	if (rc)
> -		dev_err(pdev->dev, "error writing to STB: %d\n", rc);
> +		if (rc)
> +			dev_err(pdev->dev, "error writing to STB: %d\n", rc);
> +	}
>  
>  	/* Notify on failed entry */
>  	amd_pmc_validate_deepest(pdev);