On Wed, 11 Oct 2023, David E. Box wrote:
> On supported hardware, each PMC may have an associated SSRAM device for
> accessing additional counters. However, only the SSRAM of the first
> (primary) PMC is discoverable as a PCI device to the OS. The remaining
> (secondary) devices are hidden but their BARs are still accessible and
> their addresses are stored in the BAR of the exposed device. Clean up the
> code handling the SSRAM discovery. Create two separate functions for
> accessing the primary and secondary SSRAM devices.
>
> Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx>
> ---
> V3 - New patch split from previous PATCH 2
> - Update changelog
> - Use cleanup.h to cleanup ioremap
>
> V2 - no change
>
> drivers/platform/x86/intel/pmc/core_ssram.c | 93 ++++++++++++++-------
> 1 file changed, 61 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core_ssram.c b/drivers/platform/x86/intel/pmc/core_ssram.c
> index 815950713e25..af405d11919f 100644
> --- a/drivers/platform/x86/intel/pmc/core_ssram.c
> +++ b/drivers/platform/x86/intel/pmc/core_ssram.c
> @@ -8,6 +8,7 @@
> *
> */
>
> +#include <linux/cleanup.h>
> #include <linux/pci.h>
> #include <linux/io-64-nonatomic-lo-hi.h>
>
> @@ -21,6 +22,8 @@
> #define SSRAM_IOE_OFFSET 0x68
> #define SSRAM_DEVID_OFFSET 0x70
>
> +DEFINE_FREE(pmc_core_iounmap, void __iomem *, iounmap(_T));
> +
Was it that adding
DEFINE_FREE(iounmap, void __iomem *, iounmap(_T));
into some header did not work for some reason or why this? (Perhaps
because iounmap is also defined?)
--
i.
> static const struct pmc_reg_map *pmc_core_find_regmap(struct pmc_info *list, u16 devid)
> {
> for (; list->map; ++list)
> @@ -65,44 +68,74 @@ pmc_core_pmc_add(struct pmc_dev *pmcdev, u64 pwrm_base,
> return 0;
> }
>
> -static void
> -pmc_core_ssram_get_pmc(struct pmc_dev *pmcdev, void __iomem *ssram, u32 offset,
> - int pmc_idx)
> +static int
> +pmc_core_get_secondary_pmc(struct pmc_dev *pmcdev, int pmc_idx, u32 offset)
> {
> - u64 pwrm_base;
> + struct pci_dev *ssram_pcidev = pmcdev->ssram_pcidev;
> + void __iomem __free(pmc_core_iounmap) *main_ssram = NULL;
> + void __iomem __free(pmc_core_iounmap) *secondary_ssram = NULL;
> + const struct pmc_reg_map *map;
> + u64 ssram_base, pwrm_base;
> u16 devid;
>
> - if (pmc_idx != PMC_IDX_SOC) {
> - u64 ssram_base = get_base(ssram, offset);
> + if (!pmcdev->regmap_list)
> + return -ENOENT;
>
> - if (!ssram_base)
> - return;
> + /*
> + * The secondary PMC BARS (which are behind hidden PCI devices) are read
> + * from fixed offsets in MMIO of the primary PMC BAR.
> + */
> + ssram_base = ssram_pcidev->resource[0].start;
> + main_ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
> + if (!main_ssram)
> + return -ENOMEM;
>
> - ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
> - if (!ssram)
> - return;
> - }
> + ssram_base = get_base(main_ssram, offset);
> + secondary_ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
> + if (!secondary_ssram)
> + return -ENOMEM;
> +
> + pwrm_base = get_base(secondary_ssram, SSRAM_PWRM_OFFSET);
> + devid = readw(secondary_ssram + SSRAM_DEVID_OFFSET);
> +
> + map = pmc_core_find_regmap(pmcdev->regmap_list, devid);
> + if (!map)
> + return -ENODEV;
> +
> + return pmc_core_pmc_add(pmcdev, pwrm_base, map, pmc_idx);
> +}
> +
> +static int
> +pmc_core_get_primary_pmc(struct pmc_dev *pmcdev)
> +{
> + struct pci_dev *ssram_pcidev = pmcdev->ssram_pcidev;
> + void __iomem __free(pmc_core_iounmap) *ssram;
> + const struct pmc_reg_map *map;
> + u64 ssram_base, pwrm_base;
> + u16 devid;
> +
> + if (!pmcdev->regmap_list)
> + return -ENOENT;
> +
> + /* The primary PMC (SOC die) BAR is BAR 0 in config space. */
> + ssram_base = ssram_pcidev->resource[0].start;
> + ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
> + if (!ssram)
> + return -ENOMEM;
>
> pwrm_base = get_base(ssram, SSRAM_PWRM_OFFSET);
> devid = readw(ssram + SSRAM_DEVID_OFFSET);
>
> - if (pmcdev->regmap_list) {
> - const struct pmc_reg_map *map;
> + map = pmc_core_find_regmap(pmcdev->regmap_list, devid);
> + if (!map)
> + return -ENODEV;
>
> - map = pmc_core_find_regmap(pmcdev->regmap_list, devid);
> - if (map)
> - pmc_core_pmc_add(pmcdev, pwrm_base, map, pmc_idx);
> - }
> -
> - if (pmc_idx != PMC_IDX_SOC)
> - iounmap(ssram);
> + return pmc_core_pmc_add(pmcdev, pwrm_base, map, PMC_IDX_MAIN);
> }
>
> int pmc_core_ssram_init(struct pmc_dev *pmcdev)
> {
> - void __iomem *ssram;
> struct pci_dev *pcidev;
> - u64 ssram_base;
> int ret;
>
> pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, 2));
> @@ -113,18 +146,14 @@ int pmc_core_ssram_init(struct pmc_dev *pmcdev)
> if (ret)
> goto release_dev;
>
> - ssram_base = pcidev->resource[0].start;
> - ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
> - if (!ssram)
> - goto disable_dev;
> -
> pmcdev->ssram_pcidev = pcidev;
>
> - pmc_core_ssram_get_pmc(pmcdev, ssram, 0, PMC_IDX_SOC);
> - pmc_core_ssram_get_pmc(pmcdev, ssram, SSRAM_IOE_OFFSET, PMC_IDX_IOE);
> - pmc_core_ssram_get_pmc(pmcdev, ssram, SSRAM_PCH_OFFSET, PMC_IDX_PCH);
> + ret = pmc_core_get_primary_pmc(pmcdev);
> + if (ret)
> + goto disable_dev;
>
> - iounmap(ssram);
> + pmc_core_get_secondary_pmc(pmcdev, PMC_IDX_IOE, SSRAM_IOE_OFFSET);
> + pmc_core_get_secondary_pmc(pmcdev, PMC_IDX_PCH, SSRAM_PCH_OFFSET);
>
> return 0;
>
>
