| Message ID | 20211026184045.2201-2-Sanket.Goswami@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Updates to amd-pmc driver | expand |
Hi Sanket, On 10/26/21 20:40, Sanket Goswami wrote: > Store the root port information in amd_pmc_probe() so that the > information can be used across multiple routines. > > Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com> > --- > Changes in v3: > - Address review comments from Hans. > > Changes in v2: > - Store the rdev info in amd_pmc_probe() as suggested by Hans. > > drivers/platform/x86/amd-pmc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c > index 678bf6874c63..5d88e55e1ce7 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; > @@ -541,6 +542,7 @@ static int amd_pmc_probe(struct platform_device *pdev) > } > > 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); You are still introducing a leak of the rdev on error-exits here, there are error-exits here: dev->regbase = devm_ioremap(dev->dev, base_addr + AMD_PMC_BASE_ADDR_OFFSET, AMD_PMC_MAPPING_SIZE); if (!dev->regbase) return -ENOMEM; and here: dev->fch_virt_addr = devm_ioremap(dev->dev, fch_phys_addr, FCH_SSC_MAPPING_SIZE); if (!dev->fch_virt_addr) return -ENOMEM; Which now also need a pci_dev_put() call before the "return -ENOMEM". Please make patch 2/3 the first patch in the series and then use goto err_pci_dev_put here. Note you will also need to deal with setting err, and with these paths not using pcibios_err_to_errno() ... Regards, Hans > @@ -570,7 +572,6 @@ static int amd_pmc_probe(struct platform_device *pdev) > } > > 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, > @@ -604,6 +605,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; > } >
diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c index 678bf6874c63..5d88e55e1ce7 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; @@ -541,6 +542,7 @@ static int amd_pmc_probe(struct platform_device *pdev) } 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); @@ -570,7 +572,6 @@ static int amd_pmc_probe(struct platform_device *pdev) } 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, @@ -604,6 +605,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; }
Store the root port information in amd_pmc_probe() so that the information can be used across multiple routines. Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com> --- Changes in v3: - Address review comments from Hans. Changes in v2: - Store the rdev info in amd_pmc_probe() as suggested by Hans. drivers/platform/x86/amd-pmc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)