On Wed, 18 Oct 2023, David E. Box wrote:
> Currently, if the PMC SSRAM initialization fails, no error is returned and
> the only indication is that a PMC device has not been created. Instead,
> allow an error to be returned and handled directly by the caller. Don't use
> the return value yet. This is in preparation for a future rework of
> pmc_core_sram_init().
>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
> ---
> V4 - Remove return value check in mtl.c. Proper use of the value would
> require returning an error status from pmc_core_add_pmc(). But
> the function that calls it will be removed in the next patch so wait
> to use it then.
>
> V3 - New patch split from V2 PATCH 9
> - Add dev_warn on pmc_core_ssram_init fail
>
> drivers/platform/x86/intel/pmc/core.h | 2 +-
> drivers/platform/x86/intel/pmc/core_ssram.c | 21 +++++++++++++--------
> 2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index ccf24e0f5e50..edaa70067e41 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -492,7 +492,7 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev);
> int get_primary_reg_base(struct pmc *pmc);
> extern void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev);
>
> -extern void pmc_core_ssram_init(struct pmc_dev *pmcdev);
> +extern int pmc_core_ssram_init(struct pmc_dev *pmcdev);
>
> int spt_core_init(struct pmc_dev *pmcdev);
> int cnp_core_init(struct pmc_dev *pmcdev);
> diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
> index 13fa16f0d52e..815950713e25 100644
> --- a/drivers/platform/x86/intel/pmc/core_ssram.c
> +++ b/drivers/platform/x86/intel/pmc/core_ssram.c
> @@ -35,20 +35,20 @@ static inline u64 get_base(void __iomem *addr, u32 offset)
> return lo_hi_readq(addr + offset) & GENMASK_ULL(63, 3);
> }
>
> -static void
> +static int
> pmc_core_pmc_add(struct pmc_dev *pmcdev, u64 pwrm_base,
> const struct pmc_reg_map *reg_map, int pmc_index)
> {
> struct pmc *pmc = pmcdev->pmcs[pmc_index];
>
> if (!pwrm_base)
> - return;
> + return -ENODEV;
>
> /* Memory for primary PMC has been allocated in core.c */
> if (!pmc) {
> pmc = devm_kzalloc(&pmcdev->pdev->dev, sizeof(*pmc), GFP_KERNEL);
> if (!pmc)
> - return;
> + return -ENOMEM;
> }
>
> pmc->map = reg_map;
> @@ -57,10 +57,12 @@ pmc_core_pmc_add(struct pmc_dev *pmcdev, u64 pwrm_base,
>
> if (!pmc->regbase) {
> devm_kfree(&pmcdev->pdev->dev, pmc);
> - return;
> + return -ENOMEM;
> }
>
> pmcdev->pmcs[pmc_index] = pmc;
> +
> + return 0;
> }
>
> static void
> @@ -96,7 +98,7 @@ pmc_core_ssram_get_pmc(struct pmc_dev *pmcdev, void __iomem *ssram, u32 offset,
> iounmap(ssram);
> }
>
> -void pmc_core_ssram_init(struct pmc_dev *pmcdev)
> +int pmc_core_ssram_init(struct pmc_dev *pmcdev)
> {
> void __iomem *ssram;
> struct pci_dev *pcidev;
> @@ -105,7 +107,7 @@ void pmc_core_ssram_init(struct pmc_dev *pmcdev)
>
> pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, 2));
> if (!pcidev)
> - goto out;
> + return -ENODEV;
>
> ret = pcim_enable_device(pcidev);
> if (ret)
> @@ -123,11 +125,14 @@ void pmc_core_ssram_init(struct pmc_dev *pmcdev)
> pmc_core_ssram_get_pmc(pmcdev, ssram, SSRAM_PCH_OFFSET, PMC_IDX_PCH);
>
> iounmap(ssram);
> -out:
> - return;
> +
> + return 0;
>
> disable_dev:
> + pmcdev->ssram_pcidev = NULL;
> pci_disable_device(pcidev);
> release_dev:
> pci_dev_put(pcidev);
> +
> + return ret;
> }
>