Hi, On 6/22/23 06:19, Choong Yong Liang wrote: > From: "David E. Box" <david.e.box@xxxxxxxxxxxxxxx> > > - Exports intel_pmc_core_ipc() for host access to the PMC IPC mailbox > - Add support to use IPC command allows host to access SoC registers > through PMC firmware that are otherwise inaccessible to the host due to > security policies. > > Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx> > Signed-off-by: Chao Qin <chao.qin@xxxxxxxxx> > Signed-off-by: Choong Yong Liang <yong.liang.choong@xxxxxxxxxxxxxxx> This seem to be 2 patches in 1: 1. Move core.h to include/linux/intel_pmc_core.h 2. The actual adding of IPC mailbox accessor function and add SoC register access I wonder if you really need to move the entire core.h ? IMHO it would be better to just add a new header with just the bits which you actually need to export the desired functionality. If you do believe that you really need to move core.h please split this into 2 separate patches and please place the header in a x86 specific place, e.g. : include/linux/platform_data/x86/ Also note that a somewhat big refactor, to add support for multiple PMCs for Meteor Lake is on its way to linux-next. Currently this is available in my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Please base a next version of this on this. There also is the question of how to merge this. Assuming this is ready for merging once 6.5-rc1 is out then I can merge this intel_pmc_core change into an immutable branch and send a pull-req to the net folks for this. Regards, Hans > --- > MAINTAINERS | 1 + > drivers/platform/x86/intel/pmc/adl.c | 2 +- > drivers/platform/x86/intel/pmc/cnp.c | 2 +- > drivers/platform/x86/intel/pmc/core.c | 63 ++++++++++++++++++- > drivers/platform/x86/intel/pmc/icl.c | 2 +- > drivers/platform/x86/intel/pmc/mtl.c | 2 +- > drivers/platform/x86/intel/pmc/spt.c | 2 +- > drivers/platform/x86/intel/pmc/tgl.c | 2 +- > .../core.h => include/linux/intel_pmc_core.h | 27 +++++++- > 9 files changed, 95 insertions(+), 8 deletions(-) > rename drivers/platform/x86/intel/pmc/core.h => include/linux/intel_pmc_core.h (95%) > > diff --git a/MAINTAINERS b/MAINTAINERS > index cb14589d14ab..bdb08a79a5f8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10581,6 +10581,7 @@ L: platform-driver-x86@xxxxxxxxxxxxxxx > S: Maintained > F: Documentation/ABI/testing/sysfs-platform-intel-pmc > F: drivers/platform/x86/intel/pmc/ > +F: include/linux/intel_pmc_core* > > INTEL PMIC GPIO DRIVERS > M: Andy Shevchenko <andy@xxxxxxxxxx> > diff --git a/drivers/platform/x86/intel/pmc/adl.c b/drivers/platform/x86/intel/pmc/adl.c > index 5cbd40979f2a..b6a376c536c0 100644 > --- a/drivers/platform/x86/intel/pmc/adl.c > +++ b/drivers/platform/x86/intel/pmc/adl.c > @@ -8,7 +8,7 @@ > * > */ > > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > /* Alder Lake: PGD PFET Enable Ack Status Register(s) bitmap */ > const struct pmc_bit_map adl_pfear_map[] = { > diff --git a/drivers/platform/x86/intel/pmc/cnp.c b/drivers/platform/x86/intel/pmc/cnp.c > index 7fb38815c4eb..504034cc5ec3 100644 > --- a/drivers/platform/x86/intel/pmc/cnp.c > +++ b/drivers/platform/x86/intel/pmc/cnp.c > @@ -8,7 +8,7 @@ > * > */ > > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > /* Cannon Lake: PGD PFET Enable Ack Status Register(s) bitmap */ > const struct pmc_bit_map cnp_pfear_map[] = { > diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c > index da6e7206d38b..0d60763c5144 100644 > --- a/drivers/platform/x86/intel/pmc/core.c > +++ b/drivers/platform/x86/intel/pmc/core.c > @@ -16,6 +16,7 @@ > #include <linux/delay.h> > #include <linux/dmi.h> > #include <linux/io.h> > +#include <linux/intel_pmc_core.h> > #include <linux/module.h> > #include <linux/pci.h> > #include <linux/slab.h> > @@ -26,7 +27,9 @@ > #include <asm/msr.h> > #include <asm/tsc.h> > > -#include "core.h" > +#define PMC_IPCS_PARAM_COUNT 7 > + > +static const struct x86_cpu_id *pmc_cpu_id; > > /* Maximum number of modes supported by platfoms that has low power mode capability */ > const char *pmc_lpm_modes[] = { > @@ -53,6 +56,63 @@ const struct pmc_bit_map msr_map[] = { > {} > }; > > +int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf) > +{ > + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > + union acpi_object params[PMC_IPCS_PARAM_COUNT] = { > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + }; > + struct acpi_object_list arg_list = { PMC_IPCS_PARAM_COUNT, params }; > + union acpi_object *obj; > + int status; > + > + if (!pmc_cpu_id || !ipc_cmd || !rbuf) > + return -EINVAL; > + > + /* > + * 0: IPC Command > + * 1: IPC Sub Command > + * 2: Size > + * 3-6: Write Buffer for offset > + */ > + params[0].integer.value = ipc_cmd->cmd; > + params[1].integer.value = ipc_cmd->sub_cmd; > + params[2].integer.value = ipc_cmd->size; > + params[3].integer.value = ipc_cmd->wbuf[0]; > + params[4].integer.value = ipc_cmd->wbuf[1]; > + params[5].integer.value = ipc_cmd->wbuf[2]; > + params[6].integer.value = ipc_cmd->wbuf[3]; > + > + status = acpi_evaluate_object(NULL, "\\IPCS", &arg_list, &buffer); > + if (ACPI_FAILURE(status)) > + return -ENODEV; > + > + obj = buffer.pointer; > + /* Check if the number of elements in package is 5 */ > + if (obj && obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 5) { > + const union acpi_object *objs = obj->package.elements; > + > + if ((u8)objs[0].integer.value != 0) > + return -EINVAL; > + > + rbuf[0] = objs[1].integer.value; > + rbuf[1] = objs[2].integer.value; > + rbuf[2] = objs[3].integer.value; > + rbuf[3] = objs[4].integer.value; > + } else { > + return -EINVAL; > + } > + > + return 0; > +} > +EXPORT_SYMBOL(intel_pmc_core_ipc); > + > static inline u32 pmc_core_reg_read(struct pmc_dev *pmcdev, int reg_offset) > { > return readl(pmcdev->regbase + reg_offset); > @@ -1130,6 +1190,7 @@ static int pmc_core_probe(struct platform_device *pdev) > mutex_init(&pmcdev->lock); > core_init(pmcdev); > > + pmc_cpu_id = cpu_id; > > if (lpit_read_residency_count_address(&slp_s0_addr)) { > pmcdev->base_addr = PMC_BASE_ADDR_DEFAULT; > diff --git a/drivers/platform/x86/intel/pmc/icl.c b/drivers/platform/x86/intel/pmc/icl.c > index 2f11b1a6daeb..f18048ff9382 100644 > --- a/drivers/platform/x86/intel/pmc/icl.c > +++ b/drivers/platform/x86/intel/pmc/icl.c > @@ -8,7 +8,7 @@ > * > */ > > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > const struct pmc_bit_map icl_pfear_map[] = { > {"RES_65", BIT(0)}, > diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c > index e8cc156412ce..7897f5fe9881 100644 > --- a/drivers/platform/x86/intel/pmc/mtl.c > +++ b/drivers/platform/x86/intel/pmc/mtl.c > @@ -9,7 +9,7 @@ > */ > > #include <linux/pci.h> > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > const struct pmc_reg_map mtl_reg_map = { > .pfear_sts = ext_tgl_pfear_map, > diff --git a/drivers/platform/x86/intel/pmc/spt.c b/drivers/platform/x86/intel/pmc/spt.c > index e16982236778..95ce490cf5d6 100644 > --- a/drivers/platform/x86/intel/pmc/spt.c > +++ b/drivers/platform/x86/intel/pmc/spt.c > @@ -8,7 +8,7 @@ > * > */ > > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > const struct pmc_bit_map spt_pll_map[] = { > {"MIPI PLL", SPT_PMC_BIT_MPHY_CMN_LANE0}, > diff --git a/drivers/platform/x86/intel/pmc/tgl.c b/drivers/platform/x86/intel/pmc/tgl.c > index c245ada849d0..a1719d809497 100644 > --- a/drivers/platform/x86/intel/pmc/tgl.c > +++ b/drivers/platform/x86/intel/pmc/tgl.c > @@ -8,7 +8,7 @@ > * > */ > > -#include "core.h" > +#include <linux/intel_pmc_core.h> > > #define ACPI_S0IX_DSM_UUID "57a6512e-3979-4e9d-9708-ff13b2508972" > #define ACPI_GET_LOW_MODE_REGISTERS 1 > diff --git a/drivers/platform/x86/intel/pmc/core.h b/include/linux/intel_pmc_core.h > similarity index 95% > rename from drivers/platform/x86/intel/pmc/core.h > rename to include/linux/intel_pmc_core.h > index 9ca9b9746719..82810e8b92a2 100644 > --- a/drivers/platform/x86/intel/pmc/core.h > +++ b/include/linux/intel_pmc_core.h > @@ -250,7 +250,16 @@ enum ppfear_regs { > #define MTL_LPM_STATUS_OFFSET 0x1700 > #define MTL_LPM_LIVE_STATUS_OFFSET 0x175C > > -extern const char *pmc_lpm_modes[]; > +#define IPC_SOC_REGISTER_ACCESS 0xAA > +#define IPC_SOC_SUB_CMD_READ 0x00 > +#define IPC_SOC_SUB_CMD_WRITE 0x01 > + > +struct pmc_ipc_cmd { > + u32 cmd; > + u32 sub_cmd; > + u32 size; > + u32 wbuf[4]; > +}; > > struct pmc_bit_map { > const char *name; > @@ -427,4 +436,20 @@ static const struct file_operations __name ## _fops = { \ > .release = single_release, \ > } > > +#if IS_ENABLED(CONFIG_INTEL_PMC_CORE) > +/** > + * intel_pmc_core_ipc() - PMC IPC Mailbox accessor > + * @ipc_cmd: struct pmc_ipc_cmd prepared with input to send > + * @rbuf: Allocated u32[4] array for returned IPC data > + * > + * Return: 0 on success. Non-zero on mailbox error > + */ > +int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf); > +#else > +static inline int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf) > +{ > + return -ENODEV; > +} > +#endif /* CONFIG_INTEL_PMC_CORE */ > + > #endif /* PMC_CORE_H */
