[PATCH] platform/x86: amd: pmc: provide user message where s0ix is not supported

[PATCH] platform/x86: amd: pmc: provide user message where s0ix is not supported

[Shyam Sundar S K]

Some platforms do not support hardware backed s0i3 transitions. When such
CPUs are detected, provide a warning message to the user.

Suggested-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
Based on review-hans. Apologies, I missed this change in the earlier
series.

 drivers/platform/x86/amd/pmc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index bb7597ca334f..3d2a377f4424 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -93,6 +93,7 @@
 #define AMD_CPU_ID_YC			0x14B5
 #define AMD_CPU_ID_CB			0x14D8
 #define AMD_CPU_ID_PS			0x14E8
+#define AMD_CPU_ID_SP			0x14A4
 
 #define PMC_MSG_DELAY_MIN_US		50
 #define RESPONSE_REGISTER_LOOP_MAX	20000
@@ -913,6 +914,7 @@ static const struct pci_device_id pmc_pci_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RN) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_PCO) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RV) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_SP) },
 	{ }
 };
 
@@ -1022,6 +1024,13 @@ static int amd_pmc_probe(struct platform_device *pdev)
 	}
 
 	dev->cpu_id = rdev->device;
+
+	if (dev->cpu_id == AMD_CPU_ID_SP) {
+		dev_warn_once(dev->dev, "S0i3 is not supported on this hardware\n");
+		err = -ENODEV;
+		goto err_pci_dev_put;
+	}
+
 	dev->rdev = rdev;
 	err = amd_smn_read(0, AMD_PMC_BASE_ADDR_LO, &val);
 	if (err) {
-- 
2.25.1

RE: [PATCH] platform/x86: amd: pmc: provide user message where s0ix is not supported

[Limonciello, Mario]

[Public]

> Some platforms do not support hardware backed s0i3 transitions. When such
> CPUs are detected, provide a warning message to the user.
> 
> Suggested-by: Sanket Goswami <Sanket.Goswami@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> Based on review-hans. Apologies, I missed this change in the earlier
> series.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

> 
>  drivers/platform/x86/amd/pmc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/pmc.c
> b/drivers/platform/x86/amd/pmc.c
> index bb7597ca334f..3d2a377f4424 100644
> --- a/drivers/platform/x86/amd/pmc.c
> +++ b/drivers/platform/x86/amd/pmc.c
> @@ -93,6 +93,7 @@
>  #define AMD_CPU_ID_YC			0x14B5
>  #define AMD_CPU_ID_CB			0x14D8
>  #define AMD_CPU_ID_PS			0x14E8
> +#define AMD_CPU_ID_SP			0x14A4
> 
>  #define PMC_MSG_DELAY_MIN_US		50
>  #define RESPONSE_REGISTER_LOOP_MAX	20000
> @@ -913,6 +914,7 @@ static const struct pci_device_id pmc_pci_ids[] = {
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RN) },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_PCO) },
>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RV) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_SP) },
>  	{ }
>  };
> 
> @@ -1022,6 +1024,13 @@ static int amd_pmc_probe(struct platform_device
> *pdev)
>  	}
> 
>  	dev->cpu_id = rdev->device;
> +
> +	if (dev->cpu_id == AMD_CPU_ID_SP) {
> +		dev_warn_once(dev->dev, "S0i3 is not supported on this
> hardware\n");
> +		err = -ENODEV;
> +		goto err_pci_dev_put;
> +	}
> +
>  	dev->rdev = rdev;
>  	err = amd_smn_read(0, AMD_PMC_BASE_ADDR_LO, &val);
>  	if (err) {
> --
> 2.25.1

Re: [PATCH] platform/x86: amd: pmc: provide user message where s0ix is not supported

[Hans de Goede]

Hi,

On 4/12/23 16:23, Limonciello, Mario wrote:
> [Public]
> 
>> Some platforms do not support hardware backed s0i3 transitions. When such
>> CPUs are detected, provide a warning message to the user.
>>
>> Suggested-by: Sanket Goswami <Sanket.Goswami@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>> Based on review-hans. Apologies, I missed this change in the earlier
>> series.
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

Hmm. I find it a bit weird to add a cpu-id, causing the module
to get loaded, just to print a warning and then abort the probe().

But I guess this will help you with debugging AMD suspend/resume
issues so I'll take this:

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 | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/platform/x86/amd/pmc.c
>> b/drivers/platform/x86/amd/pmc.c
>> index bb7597ca334f..3d2a377f4424 100644
>> --- a/drivers/platform/x86/amd/pmc.c
>> +++ b/drivers/platform/x86/amd/pmc.c
>> @@ -93,6 +93,7 @@
>>  #define AMD_CPU_ID_YC			0x14B5
>>  #define AMD_CPU_ID_CB			0x14D8
>>  #define AMD_CPU_ID_PS			0x14E8
>> +#define AMD_CPU_ID_SP			0x14A4
>>
>>  #define PMC_MSG_DELAY_MIN_US		50
>>  #define RESPONSE_REGISTER_LOOP_MAX	20000
>> @@ -913,6 +914,7 @@ static const struct pci_device_id pmc_pci_ids[] = {
>>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RN) },
>>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_PCO) },
>>  	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_RV) },
>> +	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_SP) },
>>  	{ }
>>  };
>>
>> @@ -1022,6 +1024,13 @@ static int amd_pmc_probe(struct platform_device
>> *pdev)
>>  	}
>>
>>  	dev->cpu_id = rdev->device;
>> +
>> +	if (dev->cpu_id == AMD_CPU_ID_SP) {
>> +		dev_warn_once(dev->dev, "S0i3 is not supported on this
>> hardware\n");
>> +		err = -ENODEV;
>> +		goto err_pci_dev_put;
>> +	}
>> +
>>  	dev->rdev = rdev;
>>  	err = amd_smn_read(0, AMD_PMC_BASE_ADDR_LO, &val);
>>  	if (err) {
>> --
>> 2.25.1
>