[V3,10/16] platform/x86/intel/pmc: Split pmc_core_ssram_get_pmc()
Commit Message
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@linux.intel.com>
---
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(-)
Comments
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@linux.intel.com>
> ---
> 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?)
On Thu, 2023-10-12 at 18:14 +0300, Ilpo Järvinen wrote:
> 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@linux.intel.com>
> > ---
> > 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?)
No, I didn't think to add it. I'll try and send in the next version.
David
>
@@ -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));
+
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;