| Message ID | 20191211194624.2872-12-jae.hyun.yoo@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | PECI device driver introduction | expand |
On Wed, 11 Dec 2019, Jae Hyun Yoo wrote: > This commit adds Intel PECI client driver. > > Cc: Lee Jones <lee.jones@linaro.org> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Rob Herring <robh+dt@kernel.org> > Cc: Andrew Jeffery <andrew@aj.id.au> > Cc: James Feist <james.feist@linux.intel.com> > Cc: Jason M Biils <jason.m.bills@linux.intel.com> > Cc: Joel Stanley <joel@jms.id.au> > Cc: Vernon Mauery <vernon.mauery@linux.intel.com> > Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> > --- > Changes since v10: > - Fixed minor style issues. > > drivers/mfd/Kconfig | 17 +++ > drivers/mfd/Makefile | 1 + > drivers/mfd/intel-peci-client.c | 149 ++++++++++++++++++++++++++ > include/linux/mfd/intel-peci-client.h | 117 ++++++++++++++++++++ > 4 files changed, 284 insertions(+) > create mode 100644 drivers/mfd/intel-peci-client.c > create mode 100644 include/linux/mfd/intel-peci-client.h > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 420900852166..7022e54a4703 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -632,6 +632,23 @@ config MFD_INTEL_MSIC > Passage) chip. This chip embeds audio, battery, GPIO, etc. > devices used in Intel Medfield platforms. > > +config MFD_INTEL_PECI_CLIENT > + tristate "Intel PECI client" > + depends on (PECI || COMPILE_TEST) > + select MFD_CORE > + help > + If you say yes to this option, support will be included for the > + Intel PECI (Platform Environment Control Interface) client. PECI is a > + one-wire bus interface that provides a communication channel from PECI > + clients in Intel processors and chipset components to external > + monitoring or control devices. > + > + Additional drivers must be enabled in order to use the functionality > + of the device. > + > + This driver can also be built as a module. If so, the module > + will be called intel-peci-client. > + > config MFD_IPAQ_MICRO > bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support" > depends on SA1100_H3100 || SA1100_H3600 > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > index aed99f08739f..91c6fda5cec6 100644 > --- a/drivers/mfd/Makefile > +++ b/drivers/mfd/Makefile > @@ -211,6 +211,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o > obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o > obj-$(CONFIG_MFD_INTEL_LPSS_ACPI) += intel-lpss-acpi.o > obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o > +obj-$(CONFIG_MFD_INTEL_PECI_CLIENT) += intel-peci-client.o > obj-$(CONFIG_MFD_PALMAS) += palmas.o > obj-$(CONFIG_MFD_VIPERBOARD) += viperboard.o > obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o > diff --git a/drivers/mfd/intel-peci-client.c b/drivers/mfd/intel-peci-client.c > new file mode 100644 > index 000000000000..18bf0af0e09e > --- /dev/null > +++ b/drivers/mfd/intel-peci-client.c > @@ -0,0 +1,149 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (c) 2018-2019 Intel Corporation > + > +#include <linux/bitfield.h> > +#include <linux/mfd/core.h> > +#include <linux/mfd/intel-peci-client.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/peci.h> > + > +#define CPU_ID_MODEL_MASK GENMASK(7, 4) > +#define CPU_ID_FAMILY_MASK GENMASK(11, 8) > +#define CPU_ID_EXT_MODEL_MASK GENMASK(19, 16) > +#define CPU_ID_EXT_FAMILY_MASK GENMASK(27, 20) > + > +#define LOWER_NIBBLE_MASK GENMASK(3, 0) > +#define UPPER_NIBBLE_MASK GENMASK(7, 4) > +#define LOWER_BYTE_MASK GENMASK(7, 0) > +#define UPPER_BYTE_MASK GENMASK(16, 8) > + > +static struct mfd_cell peci_functions[] = { > + { .name = "peci-cputemp", }, > + { .name = "peci-dimmtemp", }, > + /* TODO: Add additional PECI sideband functions into here */ No need for this comment. It's implied. > +}; > + > +static const struct cpu_gen_info cpu_gen_info_table[] = { > + { /* Haswell Xeon */ > + .family = 6, /* Family code */ Nit: Why don't you just define the number, instead of feeling the need to further clarify by providing a comment? > + .model = INTEL_FAM6_HASWELL_X, > + .core_max = CORE_MAX_ON_HSX, > + .chan_rank_max = CHAN_RANK_MAX_ON_HSX, > + .dimm_idx_max = DIMM_IDX_MAX_ON_HSX }, > + { /* Broadwell Xeon */ > + .family = 6, /* Family code */ > + .model = INTEL_FAM6_BROADWELL_X, > + .core_max = CORE_MAX_ON_BDX, > + .chan_rank_max = CHAN_RANK_MAX_ON_BDX, > + .dimm_idx_max = DIMM_IDX_MAX_ON_BDX }, > + { /* Skylake Xeon */ > + .family = 6, /* Family code */ > + .model = INTEL_FAM6_SKYLAKE_X, > + .core_max = CORE_MAX_ON_SKX, > + .chan_rank_max = CHAN_RANK_MAX_ON_SKX, > + .dimm_idx_max = DIMM_IDX_MAX_ON_SKX }, > + { /* Skylake Xeon D */ > + .family = 6, /* Family code */ > + .model = INTEL_FAM6_SKYLAKE_XD, > + .core_max = CORE_MAX_ON_SKXD, > + .chan_rank_max = CHAN_RANK_MAX_ON_SKXD, > + .dimm_idx_max = DIMM_IDX_MAX_ON_SKXD }, > +}; > + > +static int peci_client_get_cpu_gen_info(struct peci_client_manager *priv) > +{ > + struct device *dev = &priv->client->dev; > + u32 cpu_id; > + u16 family; > + u8 model; > + int ret; > + int i; > + > + ret = peci_get_cpu_id(priv->client->adapter, priv->client->addr, > + &cpu_id); > + if (ret) > + return ret; > + > + family = FIELD_PREP(LOWER_BYTE_MASK, > + FIELD_GET(CPU_ID_FAMILY_MASK, cpu_id)) | > + FIELD_PREP(UPPER_BYTE_MASK, > + FIELD_GET(CPU_ID_EXT_FAMILY_MASK, cpu_id)); > + model = FIELD_PREP(LOWER_NIBBLE_MASK, > + FIELD_GET(CPU_ID_MODEL_MASK, cpu_id)) | > + FIELD_PREP(UPPER_NIBBLE_MASK, > + FIELD_GET(CPU_ID_EXT_MODEL_MASK, cpu_id)); > + > + for (i = 0; i < ARRAY_SIZE(cpu_gen_info_table); i++) { > + const struct cpu_gen_info *cpu_info = &cpu_gen_info_table[i]; > + > + if (family == cpu_info->family && model == cpu_info->model) { > + priv->gen_info = cpu_info; > + break; > + } > + } > + > + if (!priv->gen_info) { > + dev_err(dev, "Can't support this CPU: 0x%x\n", cpu_id); > + ret = -ENODEV; > + } > + > + return ret; > +} > + > +static int peci_client_probe(struct peci_client *client) > +{ > + struct device *dev = &client->dev; > + struct peci_client_manager *priv; > + uint cpu_no; > + int ret; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > + return -ENOMEM; > + > + dev_set_drvdata(dev, priv); > + priv->client = client; > + cpu_no = client->addr - PECI_BASE_ADDR; > + > + ret = peci_client_get_cpu_gen_info(priv); > + if (ret) > + return ret; > + > + ret = devm_mfd_add_devices(dev, cpu_no, peci_functions, > + ARRAY_SIZE(peci_functions), NULL, 0, NULL); > + if (ret < 0) { > + dev_err(dev, "Failed to register child devices: %d\n", ret); > + return ret; > + } > + > + return 0; > +} > + > +#if IS_ENABLED(CONFIG_OF) #ifdef CONFIG_OF > +static const struct of_device_id peci_client_of_table[] = { > + { .compatible = "intel,peci-client" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, peci_client_of_table); > +#endif /* CONFIG_OF */ Please remove this comment. It doesn't provide anything here. > +static const struct peci_device_id peci_client_ids[] = { > + { .name = "peci-client" }, > + { } > +}; > +MODULE_DEVICE_TABLE(peci, peci_client_ids); Is this a requirement? If so, why? We're trying to get rid of unnecessary tables. Please grep for "probe_new". > +static struct peci_driver peci_client_driver = { > + .probe = peci_client_probe, > + .id_table = peci_client_ids, > + .driver = { > + .name = KBUILD_MODNAME, > + .of_match_table = of_match_ptr(peci_client_of_table), > + }, > +}; > +module_peci_driver(peci_client_driver); > + > +MODULE_AUTHOR("Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>"); > +MODULE_DESCRIPTION("PECI client driver"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/linux/mfd/intel-peci-client.h b/include/linux/mfd/intel-peci-client.h > new file mode 100644 > index 000000000000..9854303bbc26 > --- /dev/null > +++ b/include/linux/mfd/intel-peci-client.h > @@ -0,0 +1,117 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright (c) 2018-2019 Intel Corporation */ > + > +#ifndef __LINUX_MFD_INTEL_PECI_CLIENT_H > +#define __LINUX_MFD_INTEL_PECI_CLIENT_H > + > +#include <linux/peci.h> > + > +#if IS_ENABLED(CONFIG_X86) > +#include <asm/intel-family.h> > +#else > +/* > + * Architectures other than x86 cannot include the header file so define these > + * at here. These are needed for detecting type of client x86 CPUs behind a PECI > + * connection. > + */ > +#define INTEL_FAM6_HASWELL_X 0x3F > +#define INTEL_FAM6_BROADWELL_X 0x4F > +#define INTEL_FAM6_SKYLAKE_X 0x55 > +#define INTEL_FAM6_SKYLAKE_XD 0x56 > +#endif > + > +#define CORE_MAX_ON_HSX 18 /* Max number of cores on Haswell */ > +#define CHAN_RANK_MAX_ON_HSX 8 /* Max number of channel ranks on Haswell */ > +#define DIMM_IDX_MAX_ON_HSX 3 /* Max DIMM index per channel on Haswell */ > + > +#define CORE_MAX_ON_BDX 24 /* Max number of cores on Broadwell */ > +#define CHAN_RANK_MAX_ON_BDX 4 /* Max number of channel ranks on Broadwell */ > +#define DIMM_IDX_MAX_ON_BDX 3 /* Max DIMM index per channel on Broadwell */ > + > +#define CORE_MAX_ON_SKX 28 /* Max number of cores on Skylake */ > +#define CHAN_RANK_MAX_ON_SKX 6 /* Max number of channel ranks on Skylake */ > +#define DIMM_IDX_MAX_ON_SKX 2 /* Max DIMM index per channel on Skylake */ > + > +#define CORE_MAX_ON_SKXD 16 /* Max number of cores on Skylake D */ > +#define CHAN_RANK_MAX_ON_SKXD 2 /* Max number of channel ranks on Skylake D */ > +#define DIMM_IDX_MAX_ON_SKXD 2 /* Max DIMM index per channel on Skylake D */ > + > +#define CORE_NUMS_MAX CORE_MAX_ON_SKX > +#define CHAN_RANK_MAX CHAN_RANK_MAX_ON_HSX > +#define DIMM_IDX_MAX DIMM_IDX_MAX_ON_HSX > +#define DIMM_NUMS_MAX (CHAN_RANK_MAX * DIMM_IDX_MAX) > + > +/** > + * struct cpu_gen_info - CPU generation specific information > + * @family: CPU family ID > + * @model: CPU model > + * @core_max: max number of cores > + * @chan_rank_max: max number of channel ranks > + * @dimm_idx_max: max number of DIMM indices > + * > + * CPU generation specific information to identify maximum number of cores and > + * DIMM slots. > + */ > +struct cpu_gen_info { > + u16 family; > + u8 model; > + uint core_max; > + uint chan_rank_max; > + uint dimm_idx_max; > +}; > + > +/** > + * struct peci_client_manager - PECI client manager information > + * @client; pointer to the PECI client > + * @name: PECI client manager name > + * @gen_info: CPU generation info of the detected CPU > + * > + * PECI client manager information for managing PECI sideband functions on a CPU > + * client. > + */ > +struct peci_client_manager { > + struct peci_client *client; > + char name[PECI_NAME_SIZE]; > + const struct cpu_gen_info *gen_info; > +}; > + > +/** > + * peci_client_read_package_config - read from the Package Configuration Space > + * @priv: driver private data structure > + * @index: encoding index for the requested service > + * @param: parameter to specify the exact data being requested > + * @data: data buffer to store the result > + * Context: can sleep > + * > + * A generic PECI command that provides read access to the > + * "Package Configuration Space" that is maintained by the PCU, including > + * various power and thermal management functions. Typical PCS read services > + * supported by the processor may include access to temperature data, energy > + * status, run time information, DIMM temperatures and so on. > + * > + * Return: zero on success, else a negative error code. > + */ > +static inline int > +peci_client_read_package_config(struct peci_client_manager *priv, > + u8 index, u16 param, u8 *data) > +{ > + struct peci_rd_pkg_cfg_msg msg; > + int ret; > + > + msg.addr = priv->client->addr; > + msg.index = index; > + msg.param = param; > + msg.rx_len = 4; > + > + ret = peci_command(priv->client->adapter, PECI_CMD_RD_PKG_CFG, &msg); > + if (msg.cc != PECI_DEV_CC_SUCCESS) > + ret = -EAGAIN; > + if (ret) > + return ret; > + > + memcpy(data, msg.pkg_config, 4); > + > + return 0; > +} Where is this function used? > +#endif /* __LINUX_MFD_INTEL_PECI_CLIENT_H */
Hi Lee, On 12/16/2019 8:01 AM, Lee Jones wrote: > On Wed, 11 Dec 2019, Jae Hyun Yoo wrote: > >> This commit adds Intel PECI client driver. >> >> Cc: Lee Jones <lee.jones@linaro.org> >> Cc: Randy Dunlap <rdunlap@infradead.org> >> Cc: Rob Herring <robh+dt@kernel.org> >> Cc: Andrew Jeffery <andrew@aj.id.au> >> Cc: James Feist <james.feist@linux.intel.com> >> Cc: Jason M Biils <jason.m.bills@linux.intel.com> >> Cc: Joel Stanley <joel@jms.id.au> >> Cc: Vernon Mauery <vernon.mauery@linux.intel.com> >> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> >> --- >> Changes since v10: >> - Fixed minor style issues. >> >> drivers/mfd/Kconfig | 17 +++ >> drivers/mfd/Makefile | 1 + >> drivers/mfd/intel-peci-client.c | 149 ++++++++++++++++++++++++++ >> include/linux/mfd/intel-peci-client.h | 117 ++++++++++++++++++++ >> 4 files changed, 284 insertions(+) >> create mode 100644 drivers/mfd/intel-peci-client.c >> create mode 100644 include/linux/mfd/intel-peci-client.h >> >> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >> index 420900852166..7022e54a4703 100644 >> --- a/drivers/mfd/Kconfig >> +++ b/drivers/mfd/Kconfig >> @@ -632,6 +632,23 @@ config MFD_INTEL_MSIC >> Passage) chip. This chip embeds audio, battery, GPIO, etc. >> devices used in Intel Medfield platforms. >> >> +config MFD_INTEL_PECI_CLIENT >> + tristate "Intel PECI client" >> + depends on (PECI || COMPILE_TEST) >> + select MFD_CORE >> + help >> + If you say yes to this option, support will be included for the >> + Intel PECI (Platform Environment Control Interface) client. PECI is a >> + one-wire bus interface that provides a communication channel from PECI >> + clients in Intel processors and chipset components to external >> + monitoring or control devices. >> + >> + Additional drivers must be enabled in order to use the functionality >> + of the device. >> + >> + This driver can also be built as a module. If so, the module >> + will be called intel-peci-client. >> + >> config MFD_IPAQ_MICRO >> bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support" >> depends on SA1100_H3100 || SA1100_H3600 >> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile >> index aed99f08739f..91c6fda5cec6 100644 >> --- a/drivers/mfd/Makefile >> +++ b/drivers/mfd/Makefile >> @@ -211,6 +211,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o >> obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o >> obj-$(CONFIG_MFD_INTEL_LPSS_ACPI) += intel-lpss-acpi.o >> obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o >> +obj-$(CONFIG_MFD_INTEL_PECI_CLIENT) += intel-peci-client.o >> obj-$(CONFIG_MFD_PALMAS) += palmas.o >> obj-$(CONFIG_MFD_VIPERBOARD) += viperboard.o >> obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o >> diff --git a/drivers/mfd/intel-peci-client.c b/drivers/mfd/intel-peci-client.c >> new file mode 100644 >> index 000000000000..18bf0af0e09e >> --- /dev/null >> +++ b/drivers/mfd/intel-peci-client.c >> @@ -0,0 +1,149 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +// Copyright (c) 2018-2019 Intel Corporation >> + >> +#include <linux/bitfield.h> >> +#include <linux/mfd/core.h> >> +#include <linux/mfd/intel-peci-client.h> >> +#include <linux/module.h> >> +#include <linux/of_device.h> >> +#include <linux/peci.h> >> + >> +#define CPU_ID_MODEL_MASK GENMASK(7, 4) >> +#define CPU_ID_FAMILY_MASK GENMASK(11, 8) >> +#define CPU_ID_EXT_MODEL_MASK GENMASK(19, 16) >> +#define CPU_ID_EXT_FAMILY_MASK GENMASK(27, 20) >> + >> +#define LOWER_NIBBLE_MASK GENMASK(3, 0) >> +#define UPPER_NIBBLE_MASK GENMASK(7, 4) >> +#define LOWER_BYTE_MASK GENMASK(7, 0) >> +#define UPPER_BYTE_MASK GENMASK(16, 8) >> + >> +static struct mfd_cell peci_functions[] = { >> + { .name = "peci-cputemp", }, >> + { .name = "peci-dimmtemp", }, >> + /* TODO: Add additional PECI sideband functions into here */ > > No need for this comment. It's implied. I see. I'll remove this comment. >> +}; >> + >> +static const struct cpu_gen_info cpu_gen_info_table[] = { >> + { /* Haswell Xeon */ >> + .family = 6, /* Family code */ > > Nit: Why don't you just define the number, instead of feeling the need > to further clarify by providing a comment? Makes sense. Will define the number. >> + .model = INTEL_FAM6_HASWELL_X, >> + .core_max = CORE_MAX_ON_HSX, >> + .chan_rank_max = CHAN_RANK_MAX_ON_HSX, >> + .dimm_idx_max = DIMM_IDX_MAX_ON_HSX }, >> + { /* Broadwell Xeon */ >> + .family = 6, /* Family code */ >> + .model = INTEL_FAM6_BROADWELL_X, >> + .core_max = CORE_MAX_ON_BDX, >> + .chan_rank_max = CHAN_RANK_MAX_ON_BDX, >> + .dimm_idx_max = DIMM_IDX_MAX_ON_BDX }, >> + { /* Skylake Xeon */ >> + .family = 6, /* Family code */ >> + .model = INTEL_FAM6_SKYLAKE_X, >> + .core_max = CORE_MAX_ON_SKX, >> + .chan_rank_max = CHAN_RANK_MAX_ON_SKX, >> + .dimm_idx_max = DIMM_IDX_MAX_ON_SKX }, >> + { /* Skylake Xeon D */ >> + .family = 6, /* Family code */ >> + .model = INTEL_FAM6_SKYLAKE_XD, >> + .core_max = CORE_MAX_ON_SKXD, >> + .chan_rank_max = CHAN_RANK_MAX_ON_SKXD, >> + .dimm_idx_max = DIMM_IDX_MAX_ON_SKXD }, >> +}; >> + >> +static int peci_client_get_cpu_gen_info(struct peci_client_manager *priv) >> +{ >> + struct device *dev = &priv->client->dev; >> + u32 cpu_id; >> + u16 family; >> + u8 model; >> + int ret; >> + int i; >> + >> + ret = peci_get_cpu_id(priv->client->adapter, priv->client->addr, >> + &cpu_id); >> + if (ret) >> + return ret; >> + >> + family = FIELD_PREP(LOWER_BYTE_MASK, >> + FIELD_GET(CPU_ID_FAMILY_MASK, cpu_id)) | >> + FIELD_PREP(UPPER_BYTE_MASK, >> + FIELD_GET(CPU_ID_EXT_FAMILY_MASK, cpu_id)); >> + model = FIELD_PREP(LOWER_NIBBLE_MASK, >> + FIELD_GET(CPU_ID_MODEL_MASK, cpu_id)) | >> + FIELD_PREP(UPPER_NIBBLE_MASK, >> + FIELD_GET(CPU_ID_EXT_MODEL_MASK, cpu_id)); >> + >> + for (i = 0; i < ARRAY_SIZE(cpu_gen_info_table); i++) { >> + const struct cpu_gen_info *cpu_info = &cpu_gen_info_table[i]; >> + >> + if (family == cpu_info->family && model == cpu_info->model) { >> + priv->gen_info = cpu_info; >> + break; >> + } >> + } >> + >> + if (!priv->gen_info) { >> + dev_err(dev, "Can't support this CPU: 0x%x\n", cpu_id); >> + ret = -ENODEV; >> + } >> + >> + return ret; >> +} >> + >> +static int peci_client_probe(struct peci_client *client) >> +{ >> + struct device *dev = &client->dev; >> + struct peci_client_manager *priv; >> + uint cpu_no; >> + int ret; >> + >> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); >> + if (!priv) >> + return -ENOMEM; >> + >> + dev_set_drvdata(dev, priv); >> + priv->client = client; >> + cpu_no = client->addr - PECI_BASE_ADDR; >> + >> + ret = peci_client_get_cpu_gen_info(priv); >> + if (ret) >> + return ret; >> + >> + ret = devm_mfd_add_devices(dev, cpu_no, peci_functions, >> + ARRAY_SIZE(peci_functions), NULL, 0, NULL); >> + if (ret < 0) { >> + dev_err(dev, "Failed to register child devices: %d\n", ret); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> +#if IS_ENABLED(CONFIG_OF) > > #ifdef CONFIG_OF Will fix it. >> +static const struct of_device_id peci_client_of_table[] = { >> + { .compatible = "intel,peci-client" }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, peci_client_of_table); >> +#endif /* CONFIG_OF */ > > Please remove this comment. It doesn't provide anything here. I see. I'll remove this comment. >> +static const struct peci_device_id peci_client_ids[] = { >> + { .name = "peci-client" }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(peci, peci_client_ids); > > Is this a requirement? If so, why? > > We're trying to get rid of unnecessary tables. > > Please grep for "probe_new". This is needed because peci-core.c provides sub tree searching while registering a peci bus using of_match_device, also it provides runtime registration through sysfs interface using ID match just like I2C subsystem does. The probe member in 'struct peci_driver' is defined as the new style like this: struct peci_driver { int (*probe)(struct peci_client *client); [....] } >> +static struct peci_driver peci_client_driver = { >> + .probe = peci_client_probe, >> + .id_table = peci_client_ids, >> + .driver = { >> + .name = KBUILD_MODNAME, >> + .of_match_table = of_match_ptr(peci_client_of_table), >> + }, >> +}; >> +module_peci_driver(peci_client_driver); >> + >> +MODULE_AUTHOR("Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>"); >> +MODULE_DESCRIPTION("PECI client driver"); >> +MODULE_LICENSE("GPL v2"); >> diff --git a/include/linux/mfd/intel-peci-client.h b/include/linux/mfd/intel-peci-client.h >> new file mode 100644 >> index 000000000000..9854303bbc26 >> --- /dev/null >> +++ b/include/linux/mfd/intel-peci-client.h >> @@ -0,0 +1,117 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* Copyright (c) 2018-2019 Intel Corporation */ >> + >> +#ifndef __LINUX_MFD_INTEL_PECI_CLIENT_H >> +#define __LINUX_MFD_INTEL_PECI_CLIENT_H >> + >> +#include <linux/peci.h> >> + >> +#if IS_ENABLED(CONFIG_X86) >> +#include <asm/intel-family.h> >> +#else >> +/* >> + * Architectures other than x86 cannot include the header file so define these >> + * at here. These are needed for detecting type of client x86 CPUs behind a PECI >> + * connection. >> + */ >> +#define INTEL_FAM6_HASWELL_X 0x3F >> +#define INTEL_FAM6_BROADWELL_X 0x4F >> +#define INTEL_FAM6_SKYLAKE_X 0x55 >> +#define INTEL_FAM6_SKYLAKE_XD 0x56 >> +#endif >> + >> +#define CORE_MAX_ON_HSX 18 /* Max number of cores on Haswell */ >> +#define CHAN_RANK_MAX_ON_HSX 8 /* Max number of channel ranks on Haswell */ >> +#define DIMM_IDX_MAX_ON_HSX 3 /* Max DIMM index per channel on Haswell */ >> + >> +#define CORE_MAX_ON_BDX 24 /* Max number of cores on Broadwell */ >> +#define CHAN_RANK_MAX_ON_BDX 4 /* Max number of channel ranks on Broadwell */ >> +#define DIMM_IDX_MAX_ON_BDX 3 /* Max DIMM index per channel on Broadwell */ >> + >> +#define CORE_MAX_ON_SKX 28 /* Max number of cores on Skylake */ >> +#define CHAN_RANK_MAX_ON_SKX 6 /* Max number of channel ranks on Skylake */ >> +#define DIMM_IDX_MAX_ON_SKX 2 /* Max DIMM index per channel on Skylake */ >> + >> +#define CORE_MAX_ON_SKXD 16 /* Max number of cores on Skylake D */ >> +#define CHAN_RANK_MAX_ON_SKXD 2 /* Max number of channel ranks on Skylake D */ >> +#define DIMM_IDX_MAX_ON_SKXD 2 /* Max DIMM index per channel on Skylake D */ >> + >> +#define CORE_NUMS_MAX CORE_MAX_ON_SKX >> +#define CHAN_RANK_MAX CHAN_RANK_MAX_ON_HSX >> +#define DIMM_IDX_MAX DIMM_IDX_MAX_ON_HSX >> +#define DIMM_NUMS_MAX (CHAN_RANK_MAX * DIMM_IDX_MAX) >> + >> +/** >> + * struct cpu_gen_info - CPU generation specific information >> + * @family: CPU family ID >> + * @model: CPU model >> + * @core_max: max number of cores >> + * @chan_rank_max: max number of channel ranks >> + * @dimm_idx_max: max number of DIMM indices >> + * >> + * CPU generation specific information to identify maximum number of cores and >> + * DIMM slots. >> + */ >> +struct cpu_gen_info { >> + u16 family; >> + u8 model; >> + uint core_max; >> + uint chan_rank_max; >> + uint dimm_idx_max; >> +}; >> + >> +/** >> + * struct peci_client_manager - PECI client manager information >> + * @client; pointer to the PECI client >> + * @name: PECI client manager name >> + * @gen_info: CPU generation info of the detected CPU >> + * >> + * PECI client manager information for managing PECI sideband functions on a CPU >> + * client. >> + */ >> +struct peci_client_manager { >> + struct peci_client *client; >> + char name[PECI_NAME_SIZE]; >> + const struct cpu_gen_info *gen_info; >> +}; >> + >> +/** >> + * peci_client_read_package_config - read from the Package Configuration Space >> + * @priv: driver private data structure >> + * @index: encoding index for the requested service >> + * @param: parameter to specify the exact data being requested >> + * @data: data buffer to store the result >> + * Context: can sleep >> + * >> + * A generic PECI command that provides read access to the >> + * "Package Configuration Space" that is maintained by the PCU, including >> + * various power and thermal management functions. Typical PCS read services >> + * supported by the processor may include access to temperature data, energy >> + * status, run time information, DIMM temperatures and so on. >> + * >> + * Return: zero on success, else a negative error code. >> + */ >> +static inline int >> +peci_client_read_package_config(struct peci_client_manager *priv, >> + u8 index, u16 param, u8 *data) >> +{ >> + struct peci_rd_pkg_cfg_msg msg; >> + int ret; >> + >> + msg.addr = priv->client->addr; >> + msg.index = index; >> + msg.param = param; >> + msg.rx_len = 4; >> + >> + ret = peci_command(priv->client->adapter, PECI_CMD_RD_PKG_CFG, &msg); >> + if (msg.cc != PECI_DEV_CC_SUCCESS) >> + ret = -EAGAIN; >> + if (ret) >> + return ret; >> + >> + memcpy(data, msg.pkg_config, 4); >> + >> + return 0; >> +} > > Where is this function used? This function is used in MFD cell drivers that are included in this patch set, peci-cputemp and peci-dimmtemp drivers. I moved it from intel-peci-client.c to this header file as you commented in the previous review spin. I also thought about moving it into peci-hwmon.h but this command macro is a generic thing for Intel CPUs so I moved into here. Thanks a lot for your review! -Jae >> +#endif /* __LINUX_MFD_INTEL_PECI_CLIENT_H */ >
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index 420900852166..7022e54a4703 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -632,6 +632,23 @@ config MFD_INTEL_MSIC Passage) chip. This chip embeds audio, battery, GPIO, etc. devices used in Intel Medfield platforms. +config MFD_INTEL_PECI_CLIENT + tristate "Intel PECI client" + depends on (PECI || COMPILE_TEST) + select MFD_CORE + help + If you say yes to this option, support will be included for the + Intel PECI (Platform Environment Control Interface) client. PECI is a + one-wire bus interface that provides a communication channel from PECI + clients in Intel processors and chipset components to external + monitoring or control devices. + + Additional drivers must be enabled in order to use the functionality + of the device. + + This driver can also be built as a module. If so, the module + will be called intel-peci-client. + config MFD_IPAQ_MICRO bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support" depends on SA1100_H3100 || SA1100_H3600 diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index aed99f08739f..91c6fda5cec6 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -211,6 +211,7 @@ obj-$(CONFIG_MFD_INTEL_LPSS) += intel-lpss.o obj-$(CONFIG_MFD_INTEL_LPSS_PCI) += intel-lpss-pci.o obj-$(CONFIG_MFD_INTEL_LPSS_ACPI) += intel-lpss-acpi.o obj-$(CONFIG_MFD_INTEL_MSIC) += intel_msic.o +obj-$(CONFIG_MFD_INTEL_PECI_CLIENT) += intel-peci-client.o obj-$(CONFIG_MFD_PALMAS) += palmas.o obj-$(CONFIG_MFD_VIPERBOARD) += viperboard.o obj-$(CONFIG_MFD_RC5T583) += rc5t583.o rc5t583-irq.o diff --git a/drivers/mfd/intel-peci-client.c b/drivers/mfd/intel-peci-client.c new file mode 100644 index 000000000000..18bf0af0e09e --- /dev/null +++ b/drivers/mfd/intel-peci-client.c @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (c) 2018-2019 Intel Corporation + +#include <linux/bitfield.h> +#include <linux/mfd/core.h> +#include <linux/mfd/intel-peci-client.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/peci.h> + +#define CPU_ID_MODEL_MASK GENMASK(7, 4) +#define CPU_ID_FAMILY_MASK GENMASK(11, 8) +#define CPU_ID_EXT_MODEL_MASK GENMASK(19, 16) +#define CPU_ID_EXT_FAMILY_MASK GENMASK(27, 20) + +#define LOWER_NIBBLE_MASK GENMASK(3, 0) +#define UPPER_NIBBLE_MASK GENMASK(7, 4) +#define LOWER_BYTE_MASK GENMASK(7, 0) +#define UPPER_BYTE_MASK GENMASK(16, 8) + +static struct mfd_cell peci_functions[] = { + { .name = "peci-cputemp", }, + { .name = "peci-dimmtemp", }, + /* TODO: Add additional PECI sideband functions into here */ +}; + +static const struct cpu_gen_info cpu_gen_info_table[] = { + { /* Haswell Xeon */ + .family = 6, /* Family code */ + .model = INTEL_FAM6_HASWELL_X, + .core_max = CORE_MAX_ON_HSX, + .chan_rank_max = CHAN_RANK_MAX_ON_HSX, + .dimm_idx_max = DIMM_IDX_MAX_ON_HSX }, + { /* Broadwell Xeon */ + .family = 6, /* Family code */ + .model = INTEL_FAM6_BROADWELL_X, + .core_max = CORE_MAX_ON_BDX, + .chan_rank_max = CHAN_RANK_MAX_ON_BDX, + .dimm_idx_max = DIMM_IDX_MAX_ON_BDX }, + { /* Skylake Xeon */ + .family = 6, /* Family code */ + .model = INTEL_FAM6_SKYLAKE_X, + .core_max = CORE_MAX_ON_SKX, + .chan_rank_max = CHAN_RANK_MAX_ON_SKX, + .dimm_idx_max = DIMM_IDX_MAX_ON_SKX }, + { /* Skylake Xeon D */ + .family = 6, /* Family code */ + .model = INTEL_FAM6_SKYLAKE_XD, + .core_max = CORE_MAX_ON_SKXD, + .chan_rank_max = CHAN_RANK_MAX_ON_SKXD, + .dimm_idx_max = DIMM_IDX_MAX_ON_SKXD }, +}; + +static int peci_client_get_cpu_gen_info(struct peci_client_manager *priv) +{ + struct device *dev = &priv->client->dev; + u32 cpu_id; + u16 family; + u8 model; + int ret; + int i; + + ret = peci_get_cpu_id(priv->client->adapter, priv->client->addr, + &cpu_id); + if (ret) + return ret; + + family = FIELD_PREP(LOWER_BYTE_MASK, + FIELD_GET(CPU_ID_FAMILY_MASK, cpu_id)) | + FIELD_PREP(UPPER_BYTE_MASK, + FIELD_GET(CPU_ID_EXT_FAMILY_MASK, cpu_id)); + model = FIELD_PREP(LOWER_NIBBLE_MASK, + FIELD_GET(CPU_ID_MODEL_MASK, cpu_id)) | + FIELD_PREP(UPPER_NIBBLE_MASK, + FIELD_GET(CPU_ID_EXT_MODEL_MASK, cpu_id)); + + for (i = 0; i < ARRAY_SIZE(cpu_gen_info_table); i++) { + const struct cpu_gen_info *cpu_info = &cpu_gen_info_table[i]; + + if (family == cpu_info->family && model == cpu_info->model) { + priv->gen_info = cpu_info; + break; + } + } + + if (!priv->gen_info) { + dev_err(dev, "Can't support this CPU: 0x%x\n", cpu_id); + ret = -ENODEV; + } + + return ret; +} + +static int peci_client_probe(struct peci_client *client) +{ + struct device *dev = &client->dev; + struct peci_client_manager *priv; + uint cpu_no; + int ret; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + dev_set_drvdata(dev, priv); + priv->client = client; + cpu_no = client->addr - PECI_BASE_ADDR; + + ret = peci_client_get_cpu_gen_info(priv); + if (ret) + return ret; + + ret = devm_mfd_add_devices(dev, cpu_no, peci_functions, + ARRAY_SIZE(peci_functions), NULL, 0, NULL); + if (ret < 0) { + dev_err(dev, "Failed to register child devices: %d\n", ret); + return ret; + } + + return 0; +} + +#if IS_ENABLED(CONFIG_OF) +static const struct of_device_id peci_client_of_table[] = { + { .compatible = "intel,peci-client" }, + { } +}; +MODULE_DEVICE_TABLE(of, peci_client_of_table); +#endif /* CONFIG_OF */ + +static const struct peci_device_id peci_client_ids[] = { + { .name = "peci-client" }, + { } +}; +MODULE_DEVICE_TABLE(peci, peci_client_ids); + +static struct peci_driver peci_client_driver = { + .probe = peci_client_probe, + .id_table = peci_client_ids, + .driver = { + .name = KBUILD_MODNAME, + .of_match_table = of_match_ptr(peci_client_of_table), + }, +}; +module_peci_driver(peci_client_driver); + +MODULE_AUTHOR("Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>"); +MODULE_DESCRIPTION("PECI client driver"); +MODULE_LICENSE("GPL v2"); diff --git a/include/linux/mfd/intel-peci-client.h b/include/linux/mfd/intel-peci-client.h new file mode 100644 index 000000000000..9854303bbc26 --- /dev/null +++ b/include/linux/mfd/intel-peci-client.h @@ -0,0 +1,117 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (c) 2018-2019 Intel Corporation */ + +#ifndef __LINUX_MFD_INTEL_PECI_CLIENT_H +#define __LINUX_MFD_INTEL_PECI_CLIENT_H + +#include <linux/peci.h> + +#if IS_ENABLED(CONFIG_X86) +#include <asm/intel-family.h> +#else +/* + * Architectures other than x86 cannot include the header file so define these + * at here. These are needed for detecting type of client x86 CPUs behind a PECI + * connection. + */ +#define INTEL_FAM6_HASWELL_X 0x3F +#define INTEL_FAM6_BROADWELL_X 0x4F +#define INTEL_FAM6_SKYLAKE_X 0x55 +#define INTEL_FAM6_SKYLAKE_XD 0x56 +#endif + +#define CORE_MAX_ON_HSX 18 /* Max number of cores on Haswell */ +#define CHAN_RANK_MAX_ON_HSX 8 /* Max number of channel ranks on Haswell */ +#define DIMM_IDX_MAX_ON_HSX 3 /* Max DIMM index per channel on Haswell */ + +#define CORE_MAX_ON_BDX 24 /* Max number of cores on Broadwell */ +#define CHAN_RANK_MAX_ON_BDX 4 /* Max number of channel ranks on Broadwell */ +#define DIMM_IDX_MAX_ON_BDX 3 /* Max DIMM index per channel on Broadwell */ + +#define CORE_MAX_ON_SKX 28 /* Max number of cores on Skylake */ +#define CHAN_RANK_MAX_ON_SKX 6 /* Max number of channel ranks on Skylake */ +#define DIMM_IDX_MAX_ON_SKX 2 /* Max DIMM index per channel on Skylake */ + +#define CORE_MAX_ON_SKXD 16 /* Max number of cores on Skylake D */ +#define CHAN_RANK_MAX_ON_SKXD 2 /* Max number of channel ranks on Skylake D */ +#define DIMM_IDX_MAX_ON_SKXD 2 /* Max DIMM index per channel on Skylake D */ + +#define CORE_NUMS_MAX CORE_MAX_ON_SKX +#define CHAN_RANK_MAX CHAN_RANK_MAX_ON_HSX +#define DIMM_IDX_MAX DIMM_IDX_MAX_ON_HSX +#define DIMM_NUMS_MAX (CHAN_RANK_MAX * DIMM_IDX_MAX) + +/** + * struct cpu_gen_info - CPU generation specific information + * @family: CPU family ID + * @model: CPU model + * @core_max: max number of cores + * @chan_rank_max: max number of channel ranks + * @dimm_idx_max: max number of DIMM indices + * + * CPU generation specific information to identify maximum number of cores and + * DIMM slots. + */ +struct cpu_gen_info { + u16 family; + u8 model; + uint core_max; + uint chan_rank_max; + uint dimm_idx_max; +}; + +/** + * struct peci_client_manager - PECI client manager information + * @client; pointer to the PECI client + * @name: PECI client manager name + * @gen_info: CPU generation info of the detected CPU + * + * PECI client manager information for managing PECI sideband functions on a CPU + * client. + */ +struct peci_client_manager { + struct peci_client *client; + char name[PECI_NAME_SIZE]; + const struct cpu_gen_info *gen_info; +}; + +/** + * peci_client_read_package_config - read from the Package Configuration Space + * @priv: driver private data structure + * @index: encoding index for the requested service + * @param: parameter to specify the exact data being requested + * @data: data buffer to store the result + * Context: can sleep + * + * A generic PECI command that provides read access to the + * "Package Configuration Space" that is maintained by the PCU, including + * various power and thermal management functions. Typical PCS read services + * supported by the processor may include access to temperature data, energy + * status, run time information, DIMM temperatures and so on. + * + * Return: zero on success, else a negative error code. + */ +static inline int +peci_client_read_package_config(struct peci_client_manager *priv, + u8 index, u16 param, u8 *data) +{ + struct peci_rd_pkg_cfg_msg msg; + int ret; + + msg.addr = priv->client->addr; + msg.index = index; + msg.param = param; + msg.rx_len = 4; + + ret = peci_command(priv->client->adapter, PECI_CMD_RD_PKG_CFG, &msg); + if (msg.cc != PECI_DEV_CC_SUCCESS) + ret = -EAGAIN; + if (ret) + return ret; + + memcpy(data, msg.pkg_config, 4); + + return 0; +} + +#endif /* __LINUX_MFD_INTEL_PECI_CLIENT_H */
This commit adds Intel PECI client driver. Cc: Lee Jones <lee.jones@linaro.org> Cc: Randy Dunlap <rdunlap@infradead.org> Cc: Rob Herring <robh+dt@kernel.org> Cc: Andrew Jeffery <andrew@aj.id.au> Cc: James Feist <james.feist@linux.intel.com> Cc: Jason M Biils <jason.m.bills@linux.intel.com> Cc: Joel Stanley <joel@jms.id.au> Cc: Vernon Mauery <vernon.mauery@linux.intel.com> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com> --- Changes since v10: - Fixed minor style issues. drivers/mfd/Kconfig | 17 +++ drivers/mfd/Makefile | 1 + drivers/mfd/intel-peci-client.c | 149 ++++++++++++++++++++++++++ include/linux/mfd/intel-peci-client.h | 117 ++++++++++++++++++++ 4 files changed, 284 insertions(+) create mode 100644 drivers/mfd/intel-peci-client.c create mode 100644 include/linux/mfd/intel-peci-client.h