All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/2] Updates to amd-pmc driver
@ 2021-11-30 11:23 Sanket Goswami
  2021-11-30 11:23 ` [PATCH v6 1/2] platform/x86: amd-pmc: Simplify error handling and store the pci_dev in amd_pmc_dev structure Sanket Goswami
  2021-11-30 11:23 ` [PATCH v6 2/2] platform/x86: amd-pmc: Add support for AMD Smart Trace Buffer Sanket Goswami
  0 siblings, 2 replies; 4+ messages in thread
From: Sanket Goswami @ 2021-11-30 11:23 UTC (permalink / raw)
  To: Shyam-sundar.S-k, hdegoede, markgross; +Cc: platform-driver-x86, Sanket Goswami

This patch series includes:
- Improvements to error-exits in driver probe and store root port information
- Introduce support for AMD Smart Trace Buffer

Sanket Goswami (2):
  platform/x86: amd-pmc: Simplify error handling and store the pci_dev
    in amd_pmc_dev structure
  platform/x86: amd-pmc: Add support for AMD Smart Trace Buffer

 drivers/platform/x86/amd-pmc.c | 172 ++++++++++++++++++++++++++++++---
 1 file changed, 157 insertions(+), 15 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v6 1/2] platform/x86: amd-pmc: Simplify error handling and store the pci_dev in amd_pmc_dev structure
  2021-11-30 11:23 [PATCH v6 0/2] Updates to amd-pmc driver Sanket Goswami
@ 2021-11-30 11:23 ` Sanket Goswami
  2021-11-30 11:23 ` [PATCH v6 2/2] platform/x86: amd-pmc: Add support for AMD Smart Trace Buffer Sanket Goswami
  1 sibling, 0 replies; 4+ messages in thread
From: Sanket Goswami @ 2021-11-30 11:23 UTC (permalink / raw)
  To: Shyam-sundar.S-k, hdegoede, markgross; +Cc: platform-driver-x86, Sanket Goswami

Handle error-exits in the amd_pmc_probe() to avoid duplication and store
the root port information in amd_pmc_probe() so that the information
can be used across multiple routines.

Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
---
Changes in v6:
- Add goto label in place of return -ENOMEM of error-exit path in
amd_pmc_probe().
- Add pci_dev_put() in amd_pmc_remove().
- Merge patch 1/3 and 2/3 of v5 into a single patch, which was earlier
meant to store the root port information.

Changes in v5:
- Use goto label incase of error-exit path as suggested by Hans.

Changes in v4:
- No change.

Changes in v3:
- No change.

Changes in v2:
- No change.

 drivers/platform/x86/amd-pmc.c | 40 +++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
index b7e50ed050a8..b3b8a5daa02e 100644
--- a/drivers/platform/x86/amd-pmc.c
+++ b/drivers/platform/x86/amd-pmc.c
@@ -121,6 +121,7 @@ struct amd_pmc_dev {
 	u16 minor;
 	u16 rev;
 	struct device *dev;
+	struct pci_dev *rdev;
 	struct mutex lock; /* generic mutex lock */
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 	struct dentry *dbgfs_dir;
@@ -533,22 +534,23 @@ static int amd_pmc_probe(struct platform_device *pdev)
 
 	rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
 	if (!rdev || !pci_match_id(pmc_pci_ids, rdev)) {
-		pci_dev_put(rdev);
-		return -ENODEV;
+		err = -ENODEV;
+		goto err_pci_dev_put;
 	}
 
 	dev->cpu_id = rdev->device;
+	dev->rdev = rdev;
 	err = pci_write_config_dword(rdev, AMD_PMC_SMU_INDEX_ADDRESS, AMD_PMC_BASE_ADDR_LO);
 	if (err) {
 		dev_err(dev->dev, "error writing to 0x%x\n", AMD_PMC_SMU_INDEX_ADDRESS);
-		pci_dev_put(rdev);
-		return pcibios_err_to_errno(err);
+		err = pcibios_err_to_errno(err);
+		goto err_pci_dev_put;
 	}
 
 	err = pci_read_config_dword(rdev, AMD_PMC_SMU_INDEX_DATA, &val);
 	if (err) {
-		pci_dev_put(rdev);
-		return pcibios_err_to_errno(err);
+		err = pcibios_err_to_errno(err);
+		goto err_pci_dev_put;
 	}
 
 	base_addr_lo = val & AMD_PMC_BASE_ADDR_HI_MASK;
@@ -556,24 +558,25 @@ static int amd_pmc_probe(struct platform_device *pdev)
 	err = pci_write_config_dword(rdev, AMD_PMC_SMU_INDEX_ADDRESS, AMD_PMC_BASE_ADDR_HI);
 	if (err) {
 		dev_err(dev->dev, "error writing to 0x%x\n", AMD_PMC_SMU_INDEX_ADDRESS);
-		pci_dev_put(rdev);
-		return pcibios_err_to_errno(err);
+		err = pcibios_err_to_errno(err);
+		goto err_pci_dev_put;
 	}
 
 	err = pci_read_config_dword(rdev, AMD_PMC_SMU_INDEX_DATA, &val);
 	if (err) {
-		pci_dev_put(rdev);
-		return pcibios_err_to_errno(err);
+		err = pcibios_err_to_errno(err);
+		goto err_pci_dev_put;
 	}
 
 	base_addr_hi = val & AMD_PMC_BASE_ADDR_LO_MASK;
-	pci_dev_put(rdev);
 	base_addr = ((u64)base_addr_hi << 32 | base_addr_lo);
 
 	dev->regbase = devm_ioremap(dev->dev, base_addr + AMD_PMC_BASE_ADDR_OFFSET,
 				    AMD_PMC_MAPPING_SIZE);
-	if (!dev->regbase)
-		return -ENOMEM;
+	if (!dev->regbase) {
+		err = -ENOMEM;
+		goto err_pci_dev_put;
+	}
 
 	mutex_init(&dev->lock);
 
@@ -582,8 +585,10 @@ static int amd_pmc_probe(struct platform_device *pdev)
 	base_addr_hi = FCH_BASE_PHY_ADDR_HIGH;
 	fch_phys_addr = ((u64)base_addr_hi << 32 | base_addr_lo);
 	dev->fch_virt_addr = devm_ioremap(dev->dev, fch_phys_addr, FCH_SSC_MAPPING_SIZE);
-	if (!dev->fch_virt_addr)
-		return -ENOMEM;
+	if (!dev->fch_virt_addr) {
+		err = -ENOMEM;
+		goto err_pci_dev_put;
+	}
 
 	/* Use SMU to get the s0i3 debug stats */
 	err = amd_pmc_setup_smu_logging(dev);
@@ -594,6 +599,10 @@ static int amd_pmc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, dev);
 	amd_pmc_dbgfs_register(dev);
 	return 0;
+
+err_pci_dev_put:
+	pci_dev_put(rdev);
+	return err;
 }
 
 static int amd_pmc_remove(struct platform_device *pdev)
@@ -601,6 +610,7 @@ static int amd_pmc_remove(struct platform_device *pdev)
 	struct amd_pmc_dev *dev = platform_get_drvdata(pdev);
 
 	amd_pmc_dbgfs_unregister(dev);
+	pci_dev_put(dev->rdev);
 	mutex_destroy(&dev->lock);
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v6 2/2] platform/x86: amd-pmc: Add support for AMD Smart Trace Buffer
  2021-11-30 11:23 [PATCH v6 0/2] Updates to amd-pmc driver Sanket Goswami
  2021-11-30 11:23 ` [PATCH v6 1/2] platform/x86: amd-pmc: Simplify error handling and store the pci_dev in amd_pmc_dev structure Sanket Goswami
@ 2021-11-30 11:23 ` Sanket Goswami
  2021-12-21 17:48   ` Hans de Goede
  1 sibling, 1 reply; 4+ messages in thread
From: Sanket Goswami @ 2021-11-30 11:23 UTC (permalink / raw)
  To: Shyam-sundar.S-k, hdegoede, markgross; +Cc: platform-driver-x86, Sanket Goswami

STB (Smart Trace Buffer), is a debug trace buffer that isolates the
failures by analyzing the last running feature of a system. This
non-intrusive way always runs in the background and stores the trace
into the SoC.

This patch enables the STB feature by passing module param
"enable_stb=1" while loading the driver and provides mechanism to
access the STB buffer using the read and write routines.

Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
---
Changes in v6:
- Capture the read data directly into buf in amd_pmc_read_stb().

Changes in v5:
- Use kfree() only once in .open as suggested by Hans.

Changes in v4:
- Use kzalloc() for memory allocation.

Changes in v3:
- Use sizeof(u32) with multiplier as suggested by Mark Gross.

Changes in v2:
- Create amd_pmc_stb_debugfs_fops structure to get STB data.
- Address review comments from Hans.

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

diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
index b3b8a5daa02e..581cfd702082 100644
--- a/drivers/platform/x86/amd-pmc.c
+++ b/drivers/platform/x86/amd-pmc.c
@@ -35,6 +35,12 @@
 #define AMD_PMC_SCRATCH_REG_CZN		0x94
 #define AMD_PMC_SCRATCH_REG_YC		0xD14
 
+/* STB Registers */
+#define AMD_PMC_STB_INDEX_ADDRESS	0xF8
+#define AMD_PMC_STB_INDEX_DATA		0xFC
+#define AMD_PMC_STB_PMI_0		0x03E30600
+#define AMD_PMC_STB_PREDEF		0xC6000001
+
 /* Base address of SMU for mapping physical address to virtual address */
 #define AMD_PMC_SMU_INDEX_ADDRESS	0xB8
 #define AMD_PMC_SMU_INDEX_DATA		0xBC
@@ -82,6 +88,7 @@
 #define SOC_SUBSYSTEM_IP_MAX	12
 #define DELAY_MIN_US		2000
 #define DELAY_MAX_US		3000
+#define FIFO_SIZE		4096
 enum amd_pmc_def {
 	MSG_TEST = 0x01,
 	MSG_OS_HINT_PCO,
@@ -128,8 +135,14 @@ struct amd_pmc_dev {
 #endif /* CONFIG_DEBUG_FS */
 };
 
+static bool enable_stb;
+module_param(enable_stb, bool, 0644);
+MODULE_PARM_DESC(enable_stb, "Enable the STB debug mechanism");
+
 static struct amd_pmc_dev pmc;
 static int amd_pmc_send_cmd(struct amd_pmc_dev *dev, u32 arg, u32 *data, u8 msg, bool ret);
+static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data);
+static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf);
 
 static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int reg_offset)
 {
@@ -176,6 +189,50 @@ static int amd_pmc_get_smu_version(struct amd_pmc_dev *dev)
 	return 0;
 }
 
+static int amd_pmc_stb_debugfs_open(struct inode *inode, struct file *filp)
+{
+	struct amd_pmc_dev *dev = filp->f_inode->i_private;
+	u32 size = FIFO_SIZE * sizeof(u32);
+	u32 *buf;
+	int rc;
+
+	buf = kzalloc(size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	rc = amd_pmc_read_stb(dev, buf);
+	if (rc) {
+		kfree(buf);
+		return rc;
+	}
+
+	filp->private_data = buf;
+	return rc;
+}
+
+static ssize_t amd_pmc_stb_debugfs_read(struct file *filp, char __user *buf, size_t size,
+					loff_t *pos)
+{
+	if (!filp->private_data)
+		return -EINVAL;
+
+	return simple_read_from_buffer(buf, size, pos, filp->private_data,
+				       FIFO_SIZE * sizeof(u32));
+}
+
+static int amd_pmc_stb_debugfs_release(struct inode *inode, struct file *filp)
+{
+	kfree(filp->private_data);
+	return 0;
+}
+
+const struct file_operations amd_pmc_stb_debugfs_fops = {
+	.owner = THIS_MODULE,
+	.open = amd_pmc_stb_debugfs_open,
+	.read = amd_pmc_stb_debugfs_read,
+	.release = amd_pmc_stb_debugfs_release,
+};
+
 static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev,
 				 struct seq_file *s)
 {
@@ -289,6 +346,10 @@ static void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
 			    &s0ix_stats_fops);
 	debugfs_create_file("amd_pmc_idlemask", 0644, dev->dbgfs_dir, dev,
 			    &amd_pmc_idlemask_fops);
+	/* Enable STB only when the module_param is set */
+	if (enable_stb)
+		debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev,
+				    &amd_pmc_stb_debugfs_fops);
 }
 #else
 static inline void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
@@ -485,6 +546,13 @@ static int __maybe_unused amd_pmc_suspend(struct device *dev)
 	if (rc)
 		dev_err(pdev->dev, "suspend failed\n");
 
+	if (enable_stb)
+		rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF);
+	if (rc)	{
+		dev_err(pdev->dev, "error writing to STB\n");
+		return rc;
+	}
+
 	return rc;
 }
 
@@ -505,6 +573,14 @@ static int __maybe_unused amd_pmc_resume(struct device *dev)
 	/* Dump the IdleMask to see the blockers */
 	amd_pmc_idlemask_read(pdev, dev, NULL);
 
+	/* Write data incremented by 1 to distinguish in stb_read */
+	if (enable_stb)
+		rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF + 1);
+	if (rc)	{
+		dev_err(pdev->dev, "error writing to STB\n");
+		return rc;
+	}
+
 	return 0;
 }
 
@@ -521,6 +597,62 @@ static const struct pci_device_id pmc_pci_ids[] = {
 	{ }
 };
 
+static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data)
+{
+	int err;
+
+	err = pci_write_config_dword(dev->rdev, AMD_PMC_STB_INDEX_ADDRESS, AMD_PMC_STB_PMI_0);
+	if (err) {
+		dev_err(dev->dev, "failed to write addr in stb: 0x%X\n",
+			AMD_PMC_STB_INDEX_ADDRESS);
+		err = pcibios_err_to_errno(err);
+		goto err_pci_dev_put;
+	}
+
+	err = pci_write_config_dword(dev->rdev, AMD_PMC_STB_INDEX_DATA, data);
+	if (err) {
+		dev_err(dev->dev, "failed to write data in stb: 0x%X\n",
+			AMD_PMC_STB_INDEX_DATA);
+		err = pcibios_err_to_errno(err);
+		goto err_pci_dev_put;
+	}
+
+	return 0;
+
+err_pci_dev_put:
+	pci_dev_put(dev->rdev);
+	return err;
+}
+
+static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf)
+{
+	int i, err;
+
+	err = pci_write_config_dword(dev->rdev, AMD_PMC_STB_INDEX_ADDRESS, AMD_PMC_STB_PMI_0);
+	if (err) {
+		dev_err(dev->dev, "error writing addr to stb: 0x%X\n",
+			AMD_PMC_STB_INDEX_ADDRESS);
+		err = pcibios_err_to_errno(err);
+		goto err_pci_dev_put;
+	}
+
+	for (i = 0; i < FIFO_SIZE; i++) {
+		err = pci_read_config_dword(dev->rdev, AMD_PMC_STB_INDEX_DATA, buf++);
+		if (err) {
+			dev_err(dev->dev, "error reading data from stb: 0x%X\n",
+				AMD_PMC_STB_INDEX_DATA);
+			err = pcibios_err_to_errno(err);
+			goto err_pci_dev_put;
+		}
+	}
+
+	return 0;
+
+err_pci_dev_put:
+	pci_dev_put(dev->rdev);
+	return err;
+}
+
 static int amd_pmc_probe(struct platform_device *pdev)
 {
 	struct amd_pmc_dev *dev = &pmc;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v6 2/2] platform/x86: amd-pmc: Add support for AMD Smart Trace Buffer
  2021-11-30 11:23 ` [PATCH v6 2/2] platform/x86: amd-pmc: Add support for AMD Smart Trace Buffer Sanket Goswami
@ 2021-12-21 17:48   ` Hans de Goede
  0 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2021-12-21 17:48 UTC (permalink / raw)
  To: Sanket Goswami, Shyam-sundar.S-k, markgross; +Cc: platform-driver-x86

Hi,

On 11/30/21 12:23, Sanket Goswami wrote:
> STB (Smart Trace Buffer), is a debug trace buffer that isolates the
> failures by analyzing the last running feature of a system. This
> non-intrusive way always runs in the background and stores the trace
> into the SoC.
> 
> This patch enables the STB feature by passing module param
> "enable_stb=1" while loading the driver and provides mechanism to
> access the STB buffer using the read and write routines.
> 
> Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
> ---
> Changes in v6:
> - Capture the read data directly into buf in amd_pmc_read_stb().
> 
> Changes in v5:
> - Use kfree() only once in .open as suggested by Hans.
> 
> Changes in v4:
> - Use kzalloc() for memory allocation.
> 
> Changes in v3:
> - Use sizeof(u32) with multiplier as suggested by Mark Gross.
> 
> Changes in v2:
> - Create amd_pmc_stb_debugfs_fops structure to get STB data.
> - Address review comments from Hans.
> 
>  drivers/platform/x86/amd-pmc.c | 132 +++++++++++++++++++++++++++++++++
>  1 file changed, 132 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
> index b3b8a5daa02e..581cfd702082 100644
> --- a/drivers/platform/x86/amd-pmc.c
> +++ b/drivers/platform/x86/amd-pmc.c
> @@ -35,6 +35,12 @@
>  #define AMD_PMC_SCRATCH_REG_CZN		0x94
>  #define AMD_PMC_SCRATCH_REG_YC		0xD14
>  
> +/* STB Registers */
> +#define AMD_PMC_STB_INDEX_ADDRESS	0xF8
> +#define AMD_PMC_STB_INDEX_DATA		0xFC
> +#define AMD_PMC_STB_PMI_0		0x03E30600
> +#define AMD_PMC_STB_PREDEF		0xC6000001
> +
>  /* Base address of SMU for mapping physical address to virtual address */
>  #define AMD_PMC_SMU_INDEX_ADDRESS	0xB8
>  #define AMD_PMC_SMU_INDEX_DATA		0xBC
> @@ -82,6 +88,7 @@
>  #define SOC_SUBSYSTEM_IP_MAX	12
>  #define DELAY_MIN_US		2000
>  #define DELAY_MAX_US		3000
> +#define FIFO_SIZE		4096
>  enum amd_pmc_def {
>  	MSG_TEST = 0x01,
>  	MSG_OS_HINT_PCO,
> @@ -128,8 +135,14 @@ struct amd_pmc_dev {
>  #endif /* CONFIG_DEBUG_FS */
>  };
>  
> +static bool enable_stb;
> +module_param(enable_stb, bool, 0644);
> +MODULE_PARM_DESC(enable_stb, "Enable the STB debug mechanism");
> +
>  static struct amd_pmc_dev pmc;
>  static int amd_pmc_send_cmd(struct amd_pmc_dev *dev, u32 arg, u32 *data, u8 msg, bool ret);
> +static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data);
> +static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf);
>  
>  static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int reg_offset)
>  {
> @@ -176,6 +189,50 @@ static int amd_pmc_get_smu_version(struct amd_pmc_dev *dev)
>  	return 0;
>  }
>  
> +static int amd_pmc_stb_debugfs_open(struct inode *inode, struct file *filp)
> +{
> +	struct amd_pmc_dev *dev = filp->f_inode->i_private;
> +	u32 size = FIFO_SIZE * sizeof(u32);
> +	u32 *buf;
> +	int rc;
> +
> +	buf = kzalloc(size, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	rc = amd_pmc_read_stb(dev, buf);
> +	if (rc) {
> +		kfree(buf);
> +		return rc;
> +	}
> +
> +	filp->private_data = buf;
> +	return rc;
> +}
> +
> +static ssize_t amd_pmc_stb_debugfs_read(struct file *filp, char __user *buf, size_t size,
> +					loff_t *pos)
> +{
> +	if (!filp->private_data)
> +		return -EINVAL;
> +
> +	return simple_read_from_buffer(buf, size, pos, filp->private_data,
> +				       FIFO_SIZE * sizeof(u32));
> +}
> +
> +static int amd_pmc_stb_debugfs_release(struct inode *inode, struct file *filp)
> +{
> +	kfree(filp->private_data);
> +	return 0;
> +}
> +
> +const struct file_operations amd_pmc_stb_debugfs_fops = {
> +	.owner = THIS_MODULE,
> +	.open = amd_pmc_stb_debugfs_open,
> +	.read = amd_pmc_stb_debugfs_read,
> +	.release = amd_pmc_stb_debugfs_release,
> +};
> +
>  static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev,
>  				 struct seq_file *s)
>  {
> @@ -289,6 +346,10 @@ static void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
>  			    &s0ix_stats_fops);
>  	debugfs_create_file("amd_pmc_idlemask", 0644, dev->dbgfs_dir, dev,
>  			    &amd_pmc_idlemask_fops);
> +	/* Enable STB only when the module_param is set */
> +	if (enable_stb)
> +		debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev,
> +				    &amd_pmc_stb_debugfs_fops);
>  }
>  #else
>  static inline void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
> @@ -485,6 +546,13 @@ static int __maybe_unused amd_pmc_suspend(struct device *dev)
>  	if (rc)
>  		dev_err(pdev->dev, "suspend failed\n");
>  
> +	if (enable_stb)
> +		rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF);
> +	if (rc)	{
> +		dev_err(pdev->dev, "error writing to STB\n");
> +		return rc;
> +	}
> +
>  	return rc;
>  }
>  
> @@ -505,6 +573,14 @@ static int __maybe_unused amd_pmc_resume(struct device *dev)
>  	/* Dump the IdleMask to see the blockers */
>  	amd_pmc_idlemask_read(pdev, dev, NULL);
>  
> +	/* Write data incremented by 1 to distinguish in stb_read */
> +	if (enable_stb)
> +		rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF + 1);
> +	if (rc)	{
> +		dev_err(pdev->dev, "error writing to STB\n");
> +		return rc;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -521,6 +597,62 @@ static const struct pci_device_id pmc_pci_ids[] = {
>  	{ }
>  };
>  
> +static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data)
> +{
> +	int err;
> +
> +	err = pci_write_config_dword(dev->rdev, AMD_PMC_STB_INDEX_ADDRESS, AMD_PMC_STB_PMI_0);
> +	if (err) {
> +		dev_err(dev->dev, "failed to write addr in stb: 0x%X\n",
> +			AMD_PMC_STB_INDEX_ADDRESS);
> +		err = pcibios_err_to_errno(err);
> +		goto err_pci_dev_put;

Since the code now no longer gets the rdev on-demand here, but instead uses
the rdev reference storied in dev->rdev, it should not call pci_put_dev here,
instead it should do:

		return pcibios_err_to_errno(err)

I've fixed this; and the 3 similar cases below (2 in amd_pmc_read_stb(); while
merging this series:


Thank you for your patch-series, I've applied the series 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



> +	}
> +
> +	err = pci_write_config_dword(dev->rdev, AMD_PMC_STB_INDEX_DATA, data);
> +	if (err) {
> +		dev_err(dev->dev, "failed to write data in stb: 0x%X\n",
> +			AMD_PMC_STB_INDEX_DATA);
> +		err = pcibios_err_to_errno(err);
> +		goto err_pci_dev_put;
> +	}
> +
> +	return 0;
> +
> +err_pci_dev_put:
> +	pci_dev_put(dev->rdev);
> +	return err;
> +}
> +
> +static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf)
> +{
> +	int i, err;
> +
> +	err = pci_write_config_dword(dev->rdev, AMD_PMC_STB_INDEX_ADDRESS, AMD_PMC_STB_PMI_0);
> +	if (err) {
> +		dev_err(dev->dev, "error writing addr to stb: 0x%X\n",
> +			AMD_PMC_STB_INDEX_ADDRESS);
> +		err = pcibios_err_to_errno(err);
> +		goto err_pci_dev_put;
> +	}
> +
> +	for (i = 0; i < FIFO_SIZE; i++) {
> +		err = pci_read_config_dword(dev->rdev, AMD_PMC_STB_INDEX_DATA, buf++);
> +		if (err) {
> +			dev_err(dev->dev, "error reading data from stb: 0x%X\n",
> +				AMD_PMC_STB_INDEX_DATA);
> +			err = pcibios_err_to_errno(err);
> +			goto err_pci_dev_put;
> +		}
> +	}
> +
> +	return 0;
> +
> +err_pci_dev_put:
> +	pci_dev_put(dev->rdev);
> +	return err;
> +}
> +
>  static int amd_pmc_probe(struct platform_device *pdev)
>  {
>  	struct amd_pmc_dev *dev = &pmc;
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-12-21 17:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-30 11:23 [PATCH v6 0/2] Updates to amd-pmc driver Sanket Goswami
2021-11-30 11:23 ` [PATCH v6 1/2] platform/x86: amd-pmc: Simplify error handling and store the pci_dev in amd_pmc_dev structure Sanket Goswami
2021-11-30 11:23 ` [PATCH v6 2/2] platform/x86: amd-pmc: Add support for AMD Smart Trace Buffer Sanket Goswami
2021-12-21 17:48   ` Hans de Goede

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.