| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 25 Jun 2022 06:52:57 -0700
From: Tom Rix <trix@...hat.com>
To: Tianfei Zhang <tianfei.zhang@...el.com>, yilun.xu@...el.com,
lee.jones@...aro.org
Cc: hao.wu@...el.com, linux-kernel@...r.kernel.org,
linux-fpga@...r.kernel.org, russell.h.weight@...el.com,
matthew.gerlach@...ux.intel.com
Subject: Re: [PATCH v3 2/3] mfd: intel-m10-bmc: add PMCI driver
On 6/24/22 2:22 AM, Tianfei Zhang wrote:
> Adding a driver for the PMCI-base interface of Intel MAX10
> BMC controller.
>
> PMCI(Platform Management Control Interface) is a software-visible
> interface, connected to card BMC which provided the basic functionality
> of read/write BMC register. On the other hand, this driver leverages
> the regmap APIs to support Intel specific Indirect Register Interface
> for register read/write on PMCI.
>
> Signed-off-by: Tianfei Zhang <tianfei.zhang@...el.com>
> Signed-off-by: Russ Weight <russell.h.weight@...el.com>
> Signed-off-by: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
> ---
> v3:
> - create a new driver for intel-m10-bmc-pmci driver
> - remove the regmap_access_table
> - create a new file for sysfs-driver-intel-m10-bmc-pmci ABI
> v2:
> - fix compile warning reported by lkp
> - use regmap API for Indirect Register Interface.
> ---
> .../testing/sysfs-driver-intel-m10-bmc-pmci | 36 +++
> drivers/mfd/Kconfig | 10 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/intel-m10-bmc-pmci.c | 277 ++++++++++++++++++
> include/linux/mfd/intel-m10-bmc.h | 8 +
> 5 files changed, 332 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-pmci
> create mode 100644 drivers/mfd/intel-m10-bmc-pmci.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-pmci b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-pmci
> new file mode 100644
> index 000000000000..03371b8022ae
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-pmci
> @@ -0,0 +1,36 @@
> +What: /sys/bus/dfl/drivers/dfl-pmci-bmc/dfl_dev.X/bmc_version
> +Date: June 2022
> +KernelVersion: 5.19
> +Contact: Tianfei Zhang <tianfei.zhang@...el.com>
> +Description: Read only. Returns the hardware build version of Intel
> + MAX10 BMC chip.
> + Format: "0x%x".
> +
> +What: /sys/bus/dfl/drivers/dfl-pmci-bmc/dfl_dev.X/bmcfw_version
> +Date: June 2022
> +KernelVersion: 5.19
> +Contact: Tianfei Zhang <tianfei.zhang@...el.com>
> +Description: Read only. Returns the firmware version of Intel MAX10
> + BMC chip.
> + Format: "0x%x".
> +
> +What: /sys/bus/dfl/drivers/dfl-pmci-bmc/dfl_dev.X/mac_address
> +Date: June 2022
> +KernelVersion: 5.19
> +Contact: Tianfei Zhang <tianfei.zhang@...el.com>
> +Description: Read only. Returns the first MAC address in a block
> + of sequential MAC addresses assigned to the board
> + that is managed by the Intel MAX10 BMC. It is stored in
> + FLASH storage and is mirrored in the MAX10 BMC register
> + space.
> + Format: "%02x:%02x:%02x:%02x:%02x:%02x".
> +
> +What: /sys/bus/dfl/drivers/dfl-pmci-bmc/dfl_dev.X/mac_count
> +Date: June 2022
> +KernelVersion: 5.19
> +Contact: Tianfei Zhang <tianfei.zhang@...el.com>
> +Description: Read only. Returns the number of sequential MAC
> + addresses assigned to the board managed by the Intel
> + MAX10 BMC. This value is stored in FLASH and is mirrored
> + in the MAX10 BMC register space.
> + Format: "%u".
This file looks like a duplicate at sys-driver-intel-m10-bmc.
So the show commands below are duplicates.
Refactoring is necessary.
My recommendation is the existing intel-m10-bmc.c hold the common parts.
intel-m10-bmc.c is spi, so the spi specific bits be split to new
intel-m10-bmc-spi.c
Tom
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 3b59456f5545..ee196c41a9db 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2171,6 +2171,16 @@ config MFD_INTEL_M10_BMC
> additional drivers must be enabled in order to use the functionality
> of the device.
>
> +config MFD_INTEL_M10_BMC_PMCI
> + tristate "Intel MAX 10 Board Management Controller with PMCI"
> + depends on FPGA_DFL
> + help
> + Support for the Intel MAX 10 board management controller via PMCI.
> +
> + This driver provides common support for accessing the device,
> + additional drivers must be enabled in order to use the functionality
> + of the device.
> +
> config MFD_RSMU_I2C
> tristate "Renesas Synchronization Management Unit with I2C"
> depends on I2C && OF
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 858cacf659d6..f737bd7b7d98 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -267,6 +267,7 @@ obj-$(CONFIG_MFD_QCOM_PM8008) += qcom-pm8008.o
> obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o
> obj-$(CONFIG_MFD_SIMPLE_MFD_I2C) += simple-mfd-i2c.o
> obj-$(CONFIG_MFD_INTEL_M10_BMC) += intel-m10-bmc.o
> +obj-$(CONFIG_MFD_INTEL_M10_BMC_PMCI) += intel-m10-bmc-pmci.o
>
> obj-$(CONFIG_MFD_ATC260X) += atc260x-core.o
> obj-$(CONFIG_MFD_ATC260X_I2C) += atc260x-i2c.o
> diff --git a/drivers/mfd/intel-m10-bmc-pmci.c b/drivers/mfd/intel-m10-bmc-pmci.c
> new file mode 100644
> index 000000000000..93eca4483ac7
> --- /dev/null
> +++ b/drivers/mfd/intel-m10-bmc-pmci.c
> @@ -0,0 +1,277 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PMCI-based interface to MAX10 BMC
> + *
> + * Copyright (C) 2020-2022 Intel Corporation, Inc.
> + *
> + */
> +#include <linux/bitfield.h>
> +#include <linux/dfl.h>
> +#include <linux/mfd/intel-m10-bmc.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/core.h>
> +
> +#define M10BMC_PMCI_INDIRECT_BASE 0x400
> +#define INDIRECT_INT_US 1
> +#define INDIRECT_TIMEOUT_US 10000
> +
> +#define INDIRECT_CMD_OFF 0x0
> +#define INDIRECT_CMD_RD BIT(0)
> +#define INDIRECT_CMD_WR BIT(1)
> +#define INDIRECT_CMD_ACK BIT(2)
> +
> +#define INDIRECT_ADDR_OFF 0x4
> +#define INDIRECT_RD_OFF 0x8
> +#define INDIRECT_WR_OFF 0xc
> +
> +struct indirect_ctx {
> + void __iomem *base;
> + struct device *dev;
> + unsigned long sleep_us;
> + unsigned long timeout_us;
> +};
> +
> +struct pmci_device {
> + void __iomem *base;
> + struct device *dev;
> + struct intel_m10bmc m10bmc;
> + struct indirect_ctx ctx;
> +};
> +
> +static int pmci_indirect_bus_clr_cmd(struct indirect_ctx *ctx)
> +{
> + unsigned int cmd;
> + int ret;
> +
> + writel(0, ctx->base + INDIRECT_CMD_OFF);
> + ret = readl_poll_timeout((ctx->base + INDIRECT_CMD_OFF), cmd,
> + (!cmd), ctx->sleep_us, ctx->timeout_us);
> + if (ret)
> + dev_err(ctx->dev, "%s timed out on clearing cmd 0x%xn", __func__, cmd);
> +
> + return ret;
> +}
> +
> +static int pmci_indirect_bus_reg_read(void *context, unsigned int reg,
> + unsigned int *val)
> +{
> + struct indirect_ctx *ctx = context;
> + unsigned int cmd;
> + int ret;
> +
> + cmd = readl(ctx->base + INDIRECT_CMD_OFF);
> + if (cmd)
> + dev_warn(ctx->dev, "%s non-zero cmd 0x%x\n", __func__, cmd);
> +
> + writel(reg, ctx->base + INDIRECT_ADDR_OFF);
> + writel(INDIRECT_CMD_RD, ctx->base + INDIRECT_CMD_OFF);
> + ret = readl_poll_timeout((ctx->base + INDIRECT_CMD_OFF), cmd,
> + (cmd & INDIRECT_CMD_ACK), ctx->sleep_us,
> + ctx->timeout_us);
> + if (ret) {
> + dev_err(ctx->dev, "%s timed out on reg 0x%x cmd 0x%x\n", __func__, reg, cmd);
> + goto out;
> + }
> +
> + *val = readl(ctx->base + INDIRECT_RD_OFF);
> +
> + if (pmci_indirect_bus_clr_cmd(ctx))
> + ret = -ETIMEDOUT;
> +
> +out:
> + return ret;
> +}
> +
> +static int pmci_indirect_bus_reg_write(void *context, unsigned int reg,
> + unsigned int val)
> +{
> + struct indirect_ctx *ctx = context;
> + unsigned int cmd;
> + int ret;
> +
> + cmd = readl(ctx->base + INDIRECT_CMD_OFF);
> + if (cmd)
> + dev_warn(ctx->dev, "%s non-zero cmd 0x%x\n", __func__, cmd);
> +
> + writel(val, ctx->base + INDIRECT_WR_OFF);
> + writel(reg, ctx->base + INDIRECT_ADDR_OFF);
> + writel(INDIRECT_CMD_WR, ctx->base + INDIRECT_CMD_OFF);
> + ret = readl_poll_timeout((ctx->base + INDIRECT_CMD_OFF), cmd,
> + (cmd & INDIRECT_CMD_ACK), ctx->sleep_us,
> + ctx->timeout_us);
> + if (ret) {
> + dev_err(ctx->dev, "%s timed out on reg 0x%x cmd 0x%x\n", __func__, reg, cmd);
> + goto out;
> + }
> +
> + if (pmci_indirect_bus_clr_cmd(ctx))
> + ret = -ETIMEDOUT;
> +
> +out:
> + return ret;
> +}
> +
> +static const struct regmap_bus pmci_indirect_bus = {
> + .reg_write = pmci_indirect_bus_reg_write,
> + .reg_read = pmci_indirect_bus_reg_read,
> +};
> +
> +static struct mfd_cell m10bmc_n6000_bmc_subdevs[] = {
> + { .name = "n6000bmc-hwmon" },
> + { .name = "n6000bmc-sec-update" }
> +};
> +
> +static struct regmap_config m10bmc_pmci_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = M10BMC_PMCI_SYS_END,
> +};
> +
> +static ssize_t bmc_version_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct intel_m10bmc *ddata = dev_get_drvdata(dev);
> + unsigned int val;
> + int ret;
> +
> + ret = m10bmc_sys_read(ddata, M10BMC_PMCI_BUILD_VER, &val);
> + if (ret)
> + return ret;
> +
> + return sprintf(buf, "0x%x\n", val);
> +}
> +static DEVICE_ATTR_RO(bmc_version);
> +
> +static ssize_t bmcfw_version_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct intel_m10bmc *ddata = dev_get_drvdata(dev);
> + unsigned int val;
> + int ret;
> +
> + ret = m10bmc_sys_read(ddata, NIOS2_PMCI_FW_VERSION, &val);
> + if (ret)
> + return ret;
> +
> + return sprintf(buf, "0x%x\n", val);
> +}
> +static DEVICE_ATTR_RO(bmcfw_version);
> +
> +static ssize_t mac_address_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct intel_m10bmc *max10 = dev_get_drvdata(dev);
> + unsigned int macaddr_low, macaddr_high;
> + int ret;
> +
> + ret = m10bmc_sys_read(max10, M10BMC_PMCI_MAC_LOW, &macaddr_low);
> + if (ret)
> + return ret;
> +
> + ret = m10bmc_sys_read(max10, M10BMC_PMCI_MAC_HIGH, &macaddr_high);
> + if (ret)
> + return ret;
> +
> + return sysfs_emit(buf, "%02x:%02x:%02x:%02x:%02x:%02x\n",
> + (u8)FIELD_GET(M10BMC_MAC_BYTE1, macaddr_low),
> + (u8)FIELD_GET(M10BMC_MAC_BYTE2, macaddr_low),
> + (u8)FIELD_GET(M10BMC_MAC_BYTE3, macaddr_low),
> + (u8)FIELD_GET(M10BMC_MAC_BYTE4, macaddr_low),
> + (u8)FIELD_GET(M10BMC_MAC_BYTE5, macaddr_high),
> + (u8)FIELD_GET(M10BMC_MAC_BYTE6, macaddr_high));
> +}
> +static DEVICE_ATTR_RO(mac_address);
> +
> +static ssize_t mac_count_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct intel_m10bmc *max10 = dev_get_drvdata(dev);
> + unsigned int macaddr_high;
> + int ret;
> +
> + ret = m10bmc_sys_read(max10, M10BMC_PMCI_MAC_HIGH, &macaddr_high);
> + if (ret)
> + return ret;
> +
> + return sysfs_emit(buf, "%u\n",
> + (u8)FIELD_GET(M10BMC_MAC_COUNT, macaddr_high));
> +}
> +static DEVICE_ATTR_RO(mac_count);
> +
> +static struct attribute *m10bmc_attrs[] = {
> + &dev_attr_bmc_version.attr,
> + &dev_attr_bmcfw_version.attr,
> + &dev_attr_mac_address.attr,
> + &dev_attr_mac_count.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(m10bmc);
> +
> +static int pmci_probe(struct dfl_device *ddev)
> +{
> + struct device *dev = &ddev->dev;
> + struct mfd_cell *cells;
> + struct pmci_device *pmci;
> + int ret, n_cell;
> +
> + pmci = devm_kzalloc(dev, sizeof(*pmci), GFP_KERNEL);
> + if (!pmci)
> + return -ENOMEM;
> +
> + pmci->m10bmc.dev = dev;
> + pmci->dev = dev;
> +
> + pmci->base = devm_ioremap_resource(dev, &ddev->mmio_res);
> + if (IS_ERR(pmci->base))
> + return PTR_ERR(pmci->base);
> +
> + dev_set_drvdata(dev, &pmci->m10bmc);
> +
> + pmci->ctx.base = pmci->base + M10BMC_PMCI_INDIRECT_BASE;
> + pmci->ctx.sleep_us = INDIRECT_INT_US;
> + pmci->ctx.timeout_us = INDIRECT_TIMEOUT_US;
> + pmci->ctx.dev = dev;
> + pmci->m10bmc.regmap =
> + devm_regmap_init(dev,
> + &pmci_indirect_bus,
> + &pmci->ctx,
> + &m10bmc_pmci_regmap_config);
> + if (IS_ERR(pmci->m10bmc.regmap))
> + return PTR_ERR(pmci->m10bmc.regmap);
> +
> + cells = m10bmc_n6000_bmc_subdevs;
> + n_cell = ARRAY_SIZE(m10bmc_n6000_bmc_subdevs);
> +
> + ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
> + cells, n_cell, NULL, 0, NULL);
> + if (ret)
> + dev_err(dev, "Failed to register sub-devices: %d\n",
> + ret);
> +
> + return ret;
> +}
> +
> +#define FME_FEATURE_ID_PMCI_BMC 0x12
> +
> +static const struct dfl_device_id pmci_ids[] = {
> + { FME_ID, FME_FEATURE_ID_PMCI_BMC },
> + { }
> +};
> +MODULE_DEVICE_TABLE(dfl, pmci_ids);
> +
> +static struct dfl_driver pmci_driver = {
> + .drv = {
> + .name = "dfl-pmci-bmc",
> + .dev_groups = m10bmc_groups,
> + },
> + .id_table = pmci_ids,
> + .probe = pmci_probe,
> +};
> +
> +module_dfl_driver(pmci_driver);
> +
> +MODULE_DESCRIPTION("MAX10 BMC PMCI-based interface");
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
> index f0044b14136e..7b58af207b72 100644
> --- a/include/linux/mfd/intel-m10-bmc.h
> +++ b/include/linux/mfd/intel-m10-bmc.h
> @@ -118,6 +118,14 @@
> /* Address of 4KB inverted bit vector containing staging area FLASH count */
> #define STAGING_FLASH_COUNT 0x17ffb000
>
> +#define M10BMC_PMCI_SYS_BASE 0x0
> +#define M10BMC_PMCI_SYS_END 0xfff
> +
> +#define M10BMC_PMCI_BUILD_VER 0x0
> +#define NIOS2_PMCI_FW_VERSION 0x4
> +#define M10BMC_PMCI_MAC_LOW 0x20
> +#define M10BMC_PMCI_MAC_HIGH 0x24
> +
> /**
> * struct intel_m10bmc - Intel MAX 10 BMC parent driver data structure
> * @dev: this device
Powered by blists - more mailing lists