[PATCH 1/4] platform/x86/amd/pmc: Notify user when platform does not support s0ix transition

[PATCH 1/4] platform/x86/amd/pmc: Notify user when platform does not support s0ix transition

[Shyam Sundar S K]

Some of the AMD platforms do not support modern standby, so when such
CPU ID is detected, a warning message will be displayed to the user.

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 | 3 ++-
 drivers/platform/x86/amd/pmc/pmc.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
index e6124498b195..c7c7afb8a431 100644
--- a/drivers/platform/x86/amd/pmc/pmc.c
+++ b/drivers/platform/x86/amd/pmc/pmc.c
@@ -808,6 +808,7 @@ static const struct pci_device_id pmc_pci_ids[] = {
 	{ 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) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_SHP) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_ROOT) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_ROOT) },
 	{ }
@@ -832,7 +833,7 @@ static int amd_pmc_probe(struct platform_device *pdev)
 
 	dev->cpu_id = rdev->device;
 
-	if (dev->cpu_id == AMD_CPU_ID_SP) {
+	if (dev->cpu_id == AMD_CPU_ID_SP || dev->cpu_id == AMD_CPU_ID_SHP) {
 		dev_warn_once(dev->dev, "S0i3 is not supported on this hardware\n");
 		err = -ENODEV;
 		goto err_pci_dev_put;
diff --git a/drivers/platform/x86/amd/pmc/pmc.h b/drivers/platform/x86/amd/pmc/pmc.h
index f43f0253b0f5..8f39988ce7a3 100644
--- a/drivers/platform/x86/amd/pmc/pmc.h
+++ b/drivers/platform/x86/amd/pmc/pmc.h
@@ -79,6 +79,7 @@ void amd_mp2_stb_deinit(struct amd_pmc_dev *dev);
 #define AMD_CPU_ID_CB			0x14D8
 #define AMD_CPU_ID_PS			0x14E8
 #define AMD_CPU_ID_SP			0x14A4
+#define AMD_CPU_ID_SHP			0x153A
 #define PCI_DEVICE_ID_AMD_1AH_M20H_ROOT 0x1507
 #define PCI_DEVICE_ID_AMD_1AH_M60H_ROOT 0x1122
 #define PCI_DEVICE_ID_AMD_MP2_STB	0x172c
-- 
2.34.1

[PATCH 2/4] platform/x86/amd/pmc: Move macros and structures to the PMC header file

[Shyam Sundar S K]

To improve the code organization and readability, move the macros and
structures from the AMD PMC driver to the PMC header file.

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 | 81 ------------------------------
 drivers/platform/x86/amd/pmc/pmc.h | 81 ++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 81 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
index c7c7afb8a431..742920530987 100644
--- a/drivers/platform/x86/amd/pmc/pmc.c
+++ b/drivers/platform/x86/amd/pmc/pmc.c
@@ -32,70 +32,6 @@
 
 #include "pmc.h"
 
-/* SMU communication registers */
-#define AMD_PMC_REGISTER_RESPONSE	0x980
-#define AMD_PMC_REGISTER_ARGUMENT	0x9BC
-
-/* PMC Scratch Registers */
-#define AMD_PMC_SCRATCH_REG_CZN		0x94
-#define AMD_PMC_SCRATCH_REG_YC		0xD14
-#define AMD_PMC_SCRATCH_REG_1AH		0xF14
-
-/* STB Registers */
-#define AMD_PMC_STB_S2IDLE_PREPARE	0xC6000001
-#define AMD_PMC_STB_S2IDLE_RESTORE	0xC6000002
-#define AMD_PMC_STB_S2IDLE_CHECK	0xC6000003
-
-/* Base address of SMU for mapping physical address to virtual address */
-#define AMD_PMC_MAPPING_SIZE		0x01000
-#define AMD_PMC_BASE_ADDR_OFFSET	0x10000
-#define AMD_PMC_BASE_ADDR_LO		0x13B102E8
-#define AMD_PMC_BASE_ADDR_HI		0x13B102EC
-#define AMD_PMC_BASE_ADDR_LO_MASK	GENMASK(15, 0)
-#define AMD_PMC_BASE_ADDR_HI_MASK	GENMASK(31, 20)
-
-/* SMU Response Codes */
-#define AMD_PMC_RESULT_OK                    0x01
-#define AMD_PMC_RESULT_CMD_REJECT_BUSY       0xFC
-#define AMD_PMC_RESULT_CMD_REJECT_PREREQ     0xFD
-#define AMD_PMC_RESULT_CMD_UNKNOWN           0xFE
-#define AMD_PMC_RESULT_FAILED                0xFF
-
-/* FCH SSC Registers */
-#define FCH_S0I3_ENTRY_TIME_L_OFFSET	0x30
-#define FCH_S0I3_ENTRY_TIME_H_OFFSET	0x34
-#define FCH_S0I3_EXIT_TIME_L_OFFSET	0x38
-#define FCH_S0I3_EXIT_TIME_H_OFFSET	0x3C
-#define FCH_SSC_MAPPING_SIZE		0x800
-#define FCH_BASE_PHY_ADDR_LOW		0xFED81100
-#define FCH_BASE_PHY_ADDR_HIGH		0x00000000
-
-/* SMU Message Definations */
-#define SMU_MSG_GETSMUVERSION		0x02
-#define SMU_MSG_LOG_GETDRAM_ADDR_HI	0x04
-#define SMU_MSG_LOG_GETDRAM_ADDR_LO	0x05
-#define SMU_MSG_LOG_START		0x06
-#define SMU_MSG_LOG_RESET		0x07
-#define SMU_MSG_LOG_DUMP_DATA		0x08
-#define SMU_MSG_GET_SUP_CONSTRAINTS	0x09
-
-#define PMC_MSG_DELAY_MIN_US		50
-#define RESPONSE_REGISTER_LOOP_MAX	20000
-
-#define DELAY_MIN_US		2000
-#define DELAY_MAX_US		3000
-
-enum amd_pmc_def {
-	MSG_TEST = 0x01,
-	MSG_OS_HINT_PCO,
-	MSG_OS_HINT_RN,
-};
-
-struct amd_pmc_bit_map {
-	const char *name;
-	u32 bit_mask;
-};
-
 static const struct amd_pmc_bit_map soc15_ip_blk_v2[] = {
 	{"DISPLAY",     BIT(0)},
 	{"CPU",         BIT(1)},
@@ -165,23 +101,6 @@ static inline void amd_pmc_reg_write(struct amd_pmc_dev *dev, int reg_offset, u3
 	iowrite32(val, dev->regbase + reg_offset);
 }
 
-struct smu_metrics {
-	u32 table_version;
-	u32 hint_count;
-	u32 s0i3_last_entry_status;
-	u32 timein_s0i2;
-	u64 timeentering_s0i3_lastcapture;
-	u64 timeentering_s0i3_totaltime;
-	u64 timeto_resume_to_os_lastcapture;
-	u64 timeto_resume_to_os_totaltime;
-	u64 timein_s0i3_lastcapture;
-	u64 timein_s0i3_totaltime;
-	u64 timein_swdrips_lastcapture;
-	u64 timein_swdrips_totaltime;
-	u64 timecondition_notmet_lastcapture[32];
-	u64 timecondition_notmet_totaltime[32];
-} __packed;
-
 static void amd_pmc_get_ip_info(struct amd_pmc_dev *dev)
 {
 	switch (dev->cpu_id) {
diff --git a/drivers/platform/x86/amd/pmc/pmc.h b/drivers/platform/x86/amd/pmc/pmc.h
index 8f39988ce7a3..62f3e51020fd 100644
--- a/drivers/platform/x86/amd/pmc/pmc.h
+++ b/drivers/platform/x86/amd/pmc/pmc.h
@@ -14,6 +14,59 @@
 #include <linux/types.h>
 #include <linux/mutex.h>
 
+/* SMU communication registers */
+#define AMD_PMC_REGISTER_RESPONSE	0x980
+#define AMD_PMC_REGISTER_ARGUMENT	0x9BC
+
+/* PMC Scratch Registers */
+#define AMD_PMC_SCRATCH_REG_CZN		0x94
+#define AMD_PMC_SCRATCH_REG_YC		0xD14
+#define AMD_PMC_SCRATCH_REG_1AH		0xF14
+
+/* STB Registers */
+#define AMD_PMC_STB_S2IDLE_PREPARE	0xC6000001
+#define AMD_PMC_STB_S2IDLE_RESTORE	0xC6000002
+#define AMD_PMC_STB_S2IDLE_CHECK	0xC6000003
+
+/* Base address of SMU for mapping physical address to virtual address */
+#define AMD_PMC_MAPPING_SIZE		0x01000
+#define AMD_PMC_BASE_ADDR_OFFSET	0x10000
+#define AMD_PMC_BASE_ADDR_LO		0x13B102E8
+#define AMD_PMC_BASE_ADDR_HI		0x13B102EC
+#define AMD_PMC_BASE_ADDR_LO_MASK	GENMASK(15, 0)
+#define AMD_PMC_BASE_ADDR_HI_MASK	GENMASK(31, 20)
+
+/* SMU Response Codes */
+#define AMD_PMC_RESULT_OK                    0x01
+#define AMD_PMC_RESULT_CMD_REJECT_BUSY       0xFC
+#define AMD_PMC_RESULT_CMD_REJECT_PREREQ     0xFD
+#define AMD_PMC_RESULT_CMD_UNKNOWN           0xFE
+#define AMD_PMC_RESULT_FAILED                0xFF
+
+/* FCH SSC Registers */
+#define FCH_S0I3_ENTRY_TIME_L_OFFSET	0x30
+#define FCH_S0I3_ENTRY_TIME_H_OFFSET	0x34
+#define FCH_S0I3_EXIT_TIME_L_OFFSET	0x38
+#define FCH_S0I3_EXIT_TIME_H_OFFSET	0x3C
+#define FCH_SSC_MAPPING_SIZE		0x800
+#define FCH_BASE_PHY_ADDR_LOW		0xFED81100
+#define FCH_BASE_PHY_ADDR_HIGH		0x00000000
+
+/* SMU Message Definations */
+#define SMU_MSG_GETSMUVERSION		0x02
+#define SMU_MSG_LOG_GETDRAM_ADDR_HI	0x04
+#define SMU_MSG_LOG_GETDRAM_ADDR_LO	0x05
+#define SMU_MSG_LOG_START		0x06
+#define SMU_MSG_LOG_RESET		0x07
+#define SMU_MSG_LOG_DUMP_DATA		0x08
+#define SMU_MSG_GET_SUP_CONSTRAINTS	0x09
+
+#define PMC_MSG_DELAY_MIN_US		50
+#define RESPONSE_REGISTER_LOOP_MAX	20000
+
+#define DELAY_MIN_US		2000
+#define DELAY_MAX_US		3000
+
 enum s2d_msg_port {
 	MSG_PORT_PMC,
 	MSG_PORT_S2D,
@@ -65,6 +118,34 @@ struct amd_pmc_dev {
 	struct stb_arg stb_arg;
 };
 
+struct amd_pmc_bit_map {
+	const char *name;
+	u32 bit_mask;
+};
+
+struct smu_metrics {
+	u32 table_version;
+	u32 hint_count;
+	u32 s0i3_last_entry_status;
+	u32 timein_s0i2;
+	u64 timeentering_s0i3_lastcapture;
+	u64 timeentering_s0i3_totaltime;
+	u64 timeto_resume_to_os_lastcapture;
+	u64 timeto_resume_to_os_totaltime;
+	u64 timein_s0i3_lastcapture;
+	u64 timein_s0i3_totaltime;
+	u64 timein_swdrips_lastcapture;
+	u64 timein_swdrips_totaltime;
+	u64 timecondition_notmet_lastcapture[32];
+	u64 timecondition_notmet_totaltime[32];
+} __packed;
+
+enum amd_pmc_def {
+	MSG_TEST = 0x01,
+	MSG_OS_HINT_PCO,
+	MSG_OS_HINT_RN,
+};
+
 void amd_pmc_process_restore_quirks(struct amd_pmc_dev *dev);
 void amd_pmc_quirks_init(struct amd_pmc_dev *dev);
 void amd_mp2_stb_init(struct amd_pmc_dev *dev);
-- 
2.34.1

[PATCH 3/4] platform/x86/amd/pmc: Remove unnecessary line breaks

[Shyam Sundar S K]

Enhance code readability by fixing line break and blank line
inconsistencies.

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 | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
index 742920530987..d80a5b899a1d 100644
--- a/drivers/platform/x86/amd/pmc/pmc.c
+++ b/drivers/platform/x86/amd/pmc/pmc.c
@@ -168,7 +168,6 @@ static int get_metrics_table(struct amd_pmc_dev *pdev, struct smu_metrics *table
 {
 	if (!pdev->smu_virt_addr) {
 		int ret = amd_pmc_setup_smu_logging(pdev);
-
 		if (ret)
 			return ret;
 	}
@@ -222,7 +221,6 @@ static ssize_t smu_fw_version_show(struct device *d, struct device_attribute *at
 
 	if (!dev->major) {
 		int rc = amd_pmc_get_smu_version(dev);
-
 		if (rc)
 			return rc;
 	}
@@ -236,7 +234,6 @@ static ssize_t smu_program_show(struct device *d, struct device_attribute *attr,
 
 	if (!dev->major) {
 		int rc = amd_pmc_get_smu_version(dev);
-
 		if (rc)
 			return rc;
 	}
@@ -704,7 +701,6 @@ static int amd_pmc_suspend_handler(struct device *dev)
 	 */
 	if (pdev->disable_8042_wakeup && !disable_workarounds) {
 		int rc = amd_pmc_wa_irq1(pdev);
-
 		if (rc) {
 			dev_err(pdev->dev, "failed to adjust keyboard wakeup: %d\n", rc);
 			return rc;
@@ -743,7 +739,6 @@ static int amd_pmc_probe(struct platform_device *pdev)
 	u32 val;
 
 	dev->dev = &pdev->dev;
-
 	rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
 	if (!rdev || !pci_match_id(pmc_pci_ids, rdev)) {
 		err = -ENODEV;
@@ -751,7 +746,6 @@ static int amd_pmc_probe(struct platform_device *pdev)
 	}
 
 	dev->cpu_id = rdev->device;
-
 	if (dev->cpu_id == AMD_CPU_ID_SP || dev->cpu_id == AMD_CPU_ID_SHP) {
 		dev_warn_once(dev->dev, "S0i3 is not supported on this hardware\n");
 		err = -ENODEV;
@@ -767,7 +761,6 @@ static int amd_pmc_probe(struct platform_device *pdev)
 	}
 
 	base_addr_lo = val & AMD_PMC_BASE_ADDR_HI_MASK;
-
 	err = amd_smn_read(0, AMD_PMC_BASE_ADDR_HI, &val);
 	if (err) {
 		dev_err(dev->dev, "error reading 0x%x\n", AMD_PMC_BASE_ADDR_HI);
-- 
2.34.1

Re: [PATCH 3/4] platform/x86/amd/pmc: Remove unnecessary line breaks

[Mario Limonciello]

On 2/17/2025 02:17, Shyam Sundar S K wrote:
> Enhance code readability by fixing line break and blank line
> inconsistencies.
> 
> 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 | 7 -------
>   1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index 742920530987..d80a5b899a1d 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -168,7 +168,6 @@ static int get_metrics_table(struct amd_pmc_dev *pdev, struct smu_metrics *table
>   {
>   	if (!pdev->smu_virt_addr) {
>   		int ret = amd_pmc_setup_smu_logging(pdev);
> -
>   		if (ret)

Does checkpatch like this?

I thought it has checks explicitly for
"Missing a blank line after declarations"

>   			return ret;
>   	}
> @@ -222,7 +221,6 @@ static ssize_t smu_fw_version_show(struct device *d, struct device_attribute *at
>   
>   	if (!dev->major) {
>   		int rc = amd_pmc_get_smu_version(dev);
> -
>   		if (rc)
>   			return rc;
>   	}
> @@ -236,7 +234,6 @@ static ssize_t smu_program_show(struct device *d, struct device_attribute *attr,
>   
>   	if (!dev->major) {
>   		int rc = amd_pmc_get_smu_version(dev);
> -
>   		if (rc)
>   			return rc;
>   	}
> @@ -704,7 +701,6 @@ static int amd_pmc_suspend_handler(struct device *dev)
>   	 */
>   	if (pdev->disable_8042_wakeup && !disable_workarounds) {
>   		int rc = amd_pmc_wa_irq1(pdev);
> -
>   		if (rc) {
>   			dev_err(pdev->dev, "failed to adjust keyboard wakeup: %d\n", rc);
>   			return rc;
> @@ -743,7 +739,6 @@ static int amd_pmc_probe(struct platform_device *pdev)
>   	u32 val;
>   
>   	dev->dev = &pdev->dev;
> -
>   	rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>   	if (!rdev || !pci_match_id(pmc_pci_ids, rdev)) {
>   		err = -ENODEV;
> @@ -751,7 +746,6 @@ static int amd_pmc_probe(struct platform_device *pdev)
>   	}
>   
>   	dev->cpu_id = rdev->device;
> -
>   	if (dev->cpu_id == AMD_CPU_ID_SP || dev->cpu_id == AMD_CPU_ID_SHP) {
>   		dev_warn_once(dev->dev, "S0i3 is not supported on this hardware\n");
>   		err = -ENODEV;
> @@ -767,7 +761,6 @@ static int amd_pmc_probe(struct platform_device *pdev)
>   	}
>   
>   	base_addr_lo = val & AMD_PMC_BASE_ADDR_HI_MASK;
> -
>   	err = amd_smn_read(0, AMD_PMC_BASE_ADDR_HI, &val);
>   	if (err) {
>   		dev_err(dev->dev, "error reading 0x%x\n", AMD_PMC_BASE_ADDR_HI);

Re: [PATCH 3/4] platform/x86/amd/pmc: Remove unnecessary line breaks

[Shyam Sundar S K]

On 2/17/2025 21:08, Mario Limonciello wrote:
> On 2/17/2025 02:17, Shyam Sundar S K wrote:
>> Enhance code readability by fixing line break and blank line
>> inconsistencies.
>>
>> 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 | 7 -------
>>   1 file changed, 7 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/
>> x86/amd/pmc/pmc.c
>> index 742920530987..d80a5b899a1d 100644
>> --- a/drivers/platform/x86/amd/pmc/pmc.c
>> +++ b/drivers/platform/x86/amd/pmc/pmc.c
>> @@ -168,7 +168,6 @@ static int get_metrics_table(struct amd_pmc_dev
>> *pdev, struct smu_metrics *table
>>   {
>>       if (!pdev->smu_virt_addr) {
>>           int ret = amd_pmc_setup_smu_logging(pdev);
>> -
>>           if (ret)
> 
> Does checkpatch like this?
> 
> I thought it has checks explicitly for
> "Missing a blank line after declarations"
> 

Not sure what is meant by blank line after declaration in this context.

This change is mid-way of the function.

I did run checkpatch again and did not see any issue.

total: 0 errors, 0 warnings, 0 checks, 49 lines checked

pmc_v1/0003-platform-x86-amd-pmc-Remove-unnecessary-line-breaks.patch
has no obvious style problems and is ready for submission.

Thanks,
Shyam

>>               return ret;
>>       }
>> @@ -222,7 +221,6 @@ static ssize_t smu_fw_version_show(struct device
>> *d, struct device_attribute *at
>>         if (!dev->major) {
>>           int rc = amd_pmc_get_smu_version(dev);
>> -
>>           if (rc)
>>               return rc;
>>       }
>> @@ -236,7 +234,6 @@ static ssize_t smu_program_show(struct device
>> *d, struct device_attribute *attr,
>>         if (!dev->major) {
>>           int rc = amd_pmc_get_smu_version(dev);
>> -
>>           if (rc)
>>               return rc;
>>       }
>> @@ -704,7 +701,6 @@ static int amd_pmc_suspend_handler(struct device
>> *dev)
>>        */
>>       if (pdev->disable_8042_wakeup && !disable_workarounds) {
>>           int rc = amd_pmc_wa_irq1(pdev);
>> -
>>           if (rc) {
>>               dev_err(pdev->dev, "failed to adjust keyboard wakeup:
>> %d\n", rc);
>>               return rc;
>> @@ -743,7 +739,6 @@ static int amd_pmc_probe(struct platform_device
>> *pdev)
>>       u32 val;
>>         dev->dev = &pdev->dev;
>> -
>>       rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>>       if (!rdev || !pci_match_id(pmc_pci_ids, rdev)) {
>>           err = -ENODEV;
>> @@ -751,7 +746,6 @@ static int amd_pmc_probe(struct platform_device
>> *pdev)
>>       }
>>         dev->cpu_id = rdev->device;
>> -
>>       if (dev->cpu_id == AMD_CPU_ID_SP || dev->cpu_id ==
>> AMD_CPU_ID_SHP) {
>>           dev_warn_once(dev->dev, "S0i3 is not supported on this
>> hardware\n");
>>           err = -ENODEV;
>> @@ -767,7 +761,6 @@ static int amd_pmc_probe(struct platform_device
>> *pdev)
>>       }
>>         base_addr_lo = val & AMD_PMC_BASE_ADDR_HI_MASK;
>> -
>>       err = amd_smn_read(0, AMD_PMC_BASE_ADDR_HI, &val);
>>       if (err) {
>>           dev_err(dev->dev, "error reading 0x%x\n",
>> AMD_PMC_BASE_ADDR_HI);
> 

Re: [PATCH 3/4] platform/x86/amd/pmc: Remove unnecessary line breaks

[Mario Limonciello]

On 2/18/2025 01:08, Shyam Sundar S K wrote:
> 
> 
> On 2/17/2025 21:08, Mario Limonciello wrote:
>> On 2/17/2025 02:17, Shyam Sundar S K wrote:
>>> Enhance code readability by fixing line break and blank line
>>> inconsistencies.
>>>
>>> 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 | 7 -------
>>>    1 file changed, 7 deletions(-)
>>>
>>> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/
>>> x86/amd/pmc/pmc.c
>>> index 742920530987..d80a5b899a1d 100644
>>> --- a/drivers/platform/x86/amd/pmc/pmc.c
>>> +++ b/drivers/platform/x86/amd/pmc/pmc.c
>>> @@ -168,7 +168,6 @@ static int get_metrics_table(struct amd_pmc_dev
>>> *pdev, struct smu_metrics *table
>>>    {
>>>        if (!pdev->smu_virt_addr) {
>>>            int ret = amd_pmc_setup_smu_logging(pdev);
>>> -
>>>            if (ret)
>>
>> Does checkpatch like this?
>>
>> I thought it has checks explicitly for
>> "Missing a blank line after declarations"
>>
> 
> Not sure what is meant by blank line after declaration in this context.
> 
> This change is mid-way of the function.
> 
> I did run checkpatch again and did not see any issue.
> 
> total: 0 errors, 0 warnings, 0 checks, 49 lines checked
> 
> pmc_v1/0003-platform-x86-amd-pmc-Remove-unnecessary-line-breaks.patch
> has no obvious style problems and is ready for submission.

Here is the specific check I was expecting checkpatch to throw:

https://github.com/torvalds/linux/blob/v6.14-rc3/scripts/checkpatch.pl#L4118

I had "thought" the intent was there was a blank line after declaration 
no matter the scope not just the start of function.

> 
> Thanks,
> Shyam
> 
>>>                return ret;
>>>        }
>>> @@ -222,7 +221,6 @@ static ssize_t smu_fw_version_show(struct device
>>> *d, struct device_attribute *at
>>>          if (!dev->major) {
>>>            int rc = amd_pmc_get_smu_version(dev);
>>> -
>>>            if (rc)
>>>                return rc;
>>>        }
>>> @@ -236,7 +234,6 @@ static ssize_t smu_program_show(struct device
>>> *d, struct device_attribute *attr,
>>>          if (!dev->major) {
>>>            int rc = amd_pmc_get_smu_version(dev);
>>> -
>>>            if (rc)
>>>                return rc;
>>>        }
>>> @@ -704,7 +701,6 @@ static int amd_pmc_suspend_handler(struct device
>>> *dev)
>>>         */
>>>        if (pdev->disable_8042_wakeup && !disable_workarounds) {
>>>            int rc = amd_pmc_wa_irq1(pdev);
>>> -
>>>            if (rc) {
>>>                dev_err(pdev->dev, "failed to adjust keyboard wakeup:
>>> %d\n", rc);
>>>                return rc;
>>> @@ -743,7 +739,6 @@ static int amd_pmc_probe(struct platform_device
>>> *pdev)
>>>        u32 val;
>>>          dev->dev = &pdev->dev;
>>> -
>>>        rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>>>        if (!rdev || !pci_match_id(pmc_pci_ids, rdev)) {
>>>            err = -ENODEV;
>>> @@ -751,7 +746,6 @@ static int amd_pmc_probe(struct platform_device
>>> *pdev)
>>>        }
>>>          dev->cpu_id = rdev->device;
>>> -
>>>        if (dev->cpu_id == AMD_CPU_ID_SP || dev->cpu_id ==
>>> AMD_CPU_ID_SHP) {
>>>            dev_warn_once(dev->dev, "S0i3 is not supported on this
>>> hardware\n");
>>>            err = -ENODEV;
>>> @@ -767,7 +761,6 @@ static int amd_pmc_probe(struct platform_device
>>> *pdev)
>>>        }
>>>          base_addr_lo = val & AMD_PMC_BASE_ADDR_HI_MASK;
>>> -
>>>        err = amd_smn_read(0, AMD_PMC_BASE_ADDR_HI, &val);
>>>        if (err) {
>>>            dev_err(dev->dev, "error reading 0x%x\n",
>>> AMD_PMC_BASE_ADDR_HI);
>>
> 

Re: [PATCH 3/4] platform/x86/amd/pmc: Remove unnecessary line breaks

[Ilpo Järvinen]

On Mon, 17 Feb 2025, Shyam Sundar S K wrote:

> Enhance code readability by fixing line break and blank line
> inconsistencies.
> 
> 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 | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index 742920530987..d80a5b899a1d 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -168,7 +168,6 @@ static int get_metrics_table(struct amd_pmc_dev *pdev, struct smu_metrics *table
>  {
>  	if (!pdev->smu_virt_addr) {
>  		int ret = amd_pmc_setup_smu_logging(pdev);
> -
>  		if (ret)

This should be:

		int ret;

		ret = amd_pmc_setup_smu_logging(pdev);
		if (ret)

However, I'd make ret function level local since these functions are not 
that complicated so block local variables aren't that useful.

While at it, I'd also rename the variable to rc to be consistent with the 
rest of the file.

>  			return ret;
>  	}
> @@ -222,7 +221,6 @@ static ssize_t smu_fw_version_show(struct device *d, struct device_attribute *at
>  
>  	if (!dev->major) {
>  		int rc = amd_pmc_get_smu_version(dev);
> -
>  		if (rc)
>  			return rc;
>  	}
> @@ -236,7 +234,6 @@ static ssize_t smu_program_show(struct device *d, struct device_attribute *attr,
>  
>  	if (!dev->major) {
>  		int rc = amd_pmc_get_smu_version(dev);
> -
>  		if (rc)
>  			return rc;
>  	}
> @@ -704,7 +701,6 @@ static int amd_pmc_suspend_handler(struct device *dev)
>  	 */
>  	if (pdev->disable_8042_wakeup && !disable_workarounds) {
>  		int rc = amd_pmc_wa_irq1(pdev);
> -
>  		if (rc) {
>  			dev_err(pdev->dev, "failed to adjust keyboard wakeup: %d\n", rc);
>  			return rc;
> @@ -743,7 +739,6 @@ static int amd_pmc_probe(struct platform_device *pdev)
>  	u32 val;
>  
>  	dev->dev = &pdev->dev;
> -
>  	rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>  	if (!rdev || !pci_match_id(pmc_pci_ids, rdev)) {
>  		err = -ENODEV;
> @@ -751,7 +746,6 @@ static int amd_pmc_probe(struct platform_device *pdev)
>  	}
>  
>  	dev->cpu_id = rdev->device;
> -
>  	if (dev->cpu_id == AMD_CPU_ID_SP || dev->cpu_id == AMD_CPU_ID_SHP) {
>  		dev_warn_once(dev->dev, "S0i3 is not supported on this hardware\n");
>  		err = -ENODEV;
> @@ -767,7 +761,6 @@ static int amd_pmc_probe(struct platform_device *pdev)
>  	}
>  
>  	base_addr_lo = val & AMD_PMC_BASE_ADDR_HI_MASK;
> -
>  	err = amd_smn_read(0, AMD_PMC_BASE_ADDR_HI, &val);
>  	if (err) {
>  		dev_err(dev->dev, "error reading 0x%x\n", AMD_PMC_BASE_ADDR_HI);
> 

-- 
 i.

[PATCH 4/4] platform/x86/amd/pmc: Use managed APIs for mutex

[Shyam Sundar S K]

Adopt managed devm_* APIs for handling mutex creation and deletion,
facilitating automatic resource cleanup.

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 | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
index d80a5b899a1d..f4768bd6b424 100644
--- a/drivers/platform/x86/amd/pmc/pmc.c
+++ b/drivers/platform/x86/amd/pmc/pmc.c
@@ -778,7 +778,7 @@ static int amd_pmc_probe(struct platform_device *pdev)
 		goto err_pci_dev_put;
 	}
 
-	mutex_init(&dev->lock);
+	devm_mutex_init(dev->dev, &dev->lock);
 
 	/* Get num of IP blocks within the SoC */
 	amd_pmc_get_ip_info(dev);
@@ -817,7 +817,6 @@ static void amd_pmc_remove(struct platform_device *pdev)
 	pci_dev_put(dev->rdev);
 	if (IS_ENABLED(CONFIG_AMD_MP2_STB))
 		amd_mp2_stb_deinit(dev);
-	mutex_destroy(&dev->lock);
 }
 
 static const struct acpi_device_id amd_pmc_acpi_ids[] = {
-- 
2.34.1

Re: [PATCH 4/4] platform/x86/amd/pmc: Use managed APIs for mutex

[Ilpo Järvinen]

On Mon, 17 Feb 2025, Shyam Sundar S K wrote:

> Adopt managed devm_* APIs for handling mutex creation and deletion,
> facilitating automatic resource cleanup.
> 
> 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 | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index d80a5b899a1d..f4768bd6b424 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -778,7 +778,7 @@ static int amd_pmc_probe(struct platform_device *pdev)
>  		goto err_pci_dev_put;
>  	}
>  
> -	mutex_init(&dev->lock);
> +	devm_mutex_init(dev->dev, &dev->lock);

Please add error handling.

>  
>  	/* Get num of IP blocks within the SoC */
>  	amd_pmc_get_ip_info(dev);
> @@ -817,7 +817,6 @@ static void amd_pmc_remove(struct platform_device *pdev)
>  	pci_dev_put(dev->rdev);
>  	if (IS_ENABLED(CONFIG_AMD_MP2_STB))
>  		amd_mp2_stb_deinit(dev);
> -	mutex_destroy(&dev->lock);
>  }
>  
>  static const struct acpi_device_id amd_pmc_acpi_ids[] = {
> 

-- 
 i.

Re: [PATCH 1/4] platform/x86/amd/pmc: Notify user when platform does not support s0ix transition

[Mario Limonciello]

On 2/17/2025 02:17, Shyam Sundar S K wrote:
> Some of the AMD platforms do not support modern standby, so when such
> CPU ID is detected, a warning message will be displayed to the user.
> 
> 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>

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

This patch specifically I think should also be

Cc: stable@vger.kernel.org

> ---
>   drivers/platform/x86/amd/pmc/pmc.c | 3 ++-
>   drivers/platform/x86/amd/pmc/pmc.h | 1 +
>   2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c
> index e6124498b195..c7c7afb8a431 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.c
> +++ b/drivers/platform/x86/amd/pmc/pmc.c
> @@ -808,6 +808,7 @@ static const struct pci_device_id pmc_pci_ids[] = {
>   	{ 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) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_SHP) },
>   	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M20H_ROOT) },
>   	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_1AH_M60H_ROOT) },
>   	{ }
> @@ -832,7 +833,7 @@ static int amd_pmc_probe(struct platform_device *pdev)
>   
>   	dev->cpu_id = rdev->device;
>   
> -	if (dev->cpu_id == AMD_CPU_ID_SP) {
> +	if (dev->cpu_id == AMD_CPU_ID_SP || dev->cpu_id == AMD_CPU_ID_SHP) {
>   		dev_warn_once(dev->dev, "S0i3 is not supported on this hardware\n");
>   		err = -ENODEV;
>   		goto err_pci_dev_put;
> diff --git a/drivers/platform/x86/amd/pmc/pmc.h b/drivers/platform/x86/amd/pmc/pmc.h
> index f43f0253b0f5..8f39988ce7a3 100644
> --- a/drivers/platform/x86/amd/pmc/pmc.h
> +++ b/drivers/platform/x86/amd/pmc/pmc.h
> @@ -79,6 +79,7 @@ void amd_mp2_stb_deinit(struct amd_pmc_dev *dev);
>   #define AMD_CPU_ID_CB			0x14D8
>   #define AMD_CPU_ID_PS			0x14E8
>   #define AMD_CPU_ID_SP			0x14A4
> +#define AMD_CPU_ID_SHP			0x153A
>   #define PCI_DEVICE_ID_AMD_1AH_M20H_ROOT 0x1507
>   #define PCI_DEVICE_ID_AMD_1AH_M60H_ROOT 0x1122
>   #define PCI_DEVICE_ID_AMD_MP2_STB	0x172c