[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <726cab7e-f15b-4319-ac62-1aeeddc9012d@linux.intel.com>
Date: Thu, 24 Apr 2025 12:57:25 -0700
From: Xi Pardee <xi.pardee@...ux.intel.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: irenic.rajneesh@...il.com, david.e.box@...ux.intel.com,
Hans de Goede <hdegoede@...hat.com>, platform-driver-x86@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>, linux-pm@...r.kernel.org
Subject: Re: [PATCH v4 2/4] platform/x86:intel/pmc: Create Intel PMC SSRAM
Telemetry driver
On 4/24/2025 7:01 AM, Ilpo Järvinen wrote:
> On Mon, 21 Apr 2025, Xi Pardee wrote:
>
>> Convert ssram device related functionalities to a new driver named Intel
>> PMC SSRAM Telemetry driver. Modify PMC Core driver to use API exported by
>> the driver to discover and achieve devid and PWRMBASE address information
>> for each available PMC. PMC Core driver needs to get PCI device when
>> reading from telemetry regions.
>>
>> The new SSRAM driver binds to the SSRAM device and provides the following
>> functionalities:
>> 1. Look for and register telemetry regions available in SSRAM device.
>> 2. Provide devid and PWRMBASE address information for the corresponding
>> PMCs.
>>
>> Signed-off-by: Xi Pardee <xi.pardee@...ux.intel.com>
>> ---
>> drivers/platform/x86/intel/pmc/Kconfig | 4 +
>> drivers/platform/x86/intel/pmc/Makefile | 8 +-
>> drivers/platform/x86/intel/pmc/core.c | 79 +++++++---
>> drivers/platform/x86/intel/pmc/core.h | 7 -
>> .../platform/x86/intel/pmc/ssram_telemetry.c | 147 ++++++++++++------
>> .../platform/x86/intel/pmc/ssram_telemetry.h | 24 +++
>> 6 files changed, 188 insertions(+), 81 deletions(-)
>> create mode 100644 drivers/platform/x86/intel/pmc/ssram_telemetry.h
>>
>> diff --git a/drivers/platform/x86/intel/pmc/Kconfig b/drivers/platform/x86/intel/pmc/Kconfig
>> index d2f651fbec2c..c6ef0bcf76af 100644
>> --- a/drivers/platform/x86/intel/pmc/Kconfig
>> +++ b/drivers/platform/x86/intel/pmc/Kconfig
>> @@ -8,6 +8,7 @@ config INTEL_PMC_CORE
>> depends on PCI
>> depends on ACPI
>> depends on INTEL_PMT_TELEMETRY
>> + select INTEL_PMC_SSRAM_TELEMETRY
>> help
>> The Intel Platform Controller Hub for Intel Core SoCs provides access
>> to Power Management Controller registers via various interfaces. This
>> @@ -24,3 +25,6 @@ config INTEL_PMC_CORE
>> - SLPS0 Debug registers (Cannonlake/Icelake PCH)
>> - Low Power Mode registers (Tigerlake and beyond)
>> - PMC quirks as needed to enable SLPS0/S0ix
>> +
>> +config INTEL_PMC_SSRAM_TELEMETRY
>> + tristate
>> diff --git a/drivers/platform/x86/intel/pmc/Makefile b/drivers/platform/x86/intel/pmc/Makefile
>> index e842647d3ced..5f68c8503a56 100644
>> --- a/drivers/platform/x86/intel/pmc/Makefile
>> +++ b/drivers/platform/x86/intel/pmc/Makefile
>> @@ -3,8 +3,12 @@
>> # Intel x86 Platform-Specific Drivers
>> #
>>
>> -intel_pmc_core-y := core.o ssram_telemetry.o spt.o cnp.o \
>> - icl.o tgl.o adl.o mtl.o arl.o lnl.o ptl.o
>> +intel_pmc_core-y := core.o spt.o cnp.o icl.o \
>> + tgl.o adl.o mtl.o arl.o lnl.o ptl.o
>> obj-$(CONFIG_INTEL_PMC_CORE) += intel_pmc_core.o
>> intel_pmc_core_pltdrv-y := pltdrv.o
>> obj-$(CONFIG_INTEL_PMC_CORE) += intel_pmc_core_pltdrv.o
>> +
>> +# Intel PMC SSRAM driver
>> +intel_pmc_ssram_telemetry-y += ssram_telemetry.o
>> +obj-$(CONFIG_INTEL_PMC_SSRAM_TELEMETRY) += intel_pmc_ssram_telemetry.o
>> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
>> index a53a7677122c..042b60c1185f 100644
>> --- a/drivers/platform/x86/intel/pmc/core.c
>> +++ b/drivers/platform/x86/intel/pmc/core.c
>> @@ -29,6 +29,7 @@
>> #include <asm/tsc.h>
>>
>> #include "core.h"
>> +#include "ssram_telemetry.h"
>> #include "../pmt/telemetry.h"
>>
>> /* Maximum number of modes supported by platfoms that has low power mode capability */
>> @@ -1354,7 +1355,7 @@ static u32 pmc_core_find_guid(struct pmc_info *list, const struct pmc_reg_map *m
>> return 0;
>> }
>>
>> -static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc)
>> +static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct pci_dev *pcidev)
>> {
>> struct telem_endpoint *ep;
>> const u8 *lpm_indices;
>> @@ -1371,7 +1372,7 @@ static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc)
>> if (!guid)
>> return -ENXIO;
>>
>> - ep = pmt_telem_find_and_register_endpoint(pmcdev->ssram_pcidev, guid, 0);
>> + ep = pmt_telem_find_and_register_endpoint(pcidev, guid, 0);
>> if (IS_ERR(ep)) {
>> dev_dbg(&pmcdev->pdev->dev, "couldn't get telem endpoint %ld",
>> PTR_ERR(ep));
>> @@ -1455,27 +1456,29 @@ static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc)
>> return ret;
>> }
>>
>> -static int pmc_core_ssram_get_lpm_reqs(struct pmc_dev *pmcdev)
>> +static int pmc_core_ssram_get_lpm_reqs(struct pmc_dev *pmcdev, int func)
>> {
>> + struct pci_dev *pcidev __free(pci_dev_put) = NULL;
>> unsigned int i;
>> - int ret;
>> + int ret = 0;
> After you remove the return changes below, this change is not needed
> either.
>
>>
>> - if (!pmcdev->ssram_pcidev)
>> + pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, func));
>> + if (!pcidev)
>> return -ENODEV;
>>
>> for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
>> if (!pmcdev->pmcs[i])
>> continue;
>>
>> - ret = pmc_core_get_lpm_req(pmcdev, pmcdev->pmcs[i]);
>> + ret = pmc_core_get_lpm_req(pmcdev, pmcdev->pmcs[i], pcidev);
>> if (ret)
>> - return ret;
>> + break;
> These two return changes are unnecessary.
Thanks for the comments.
Will remove these return changes in next version.
Xi
>> }
>>
>> - return 0;
>> + return ret;
Powered by blists - more mailing lists