* [PATCH v3 0/3] add PMCI driver support
@ 2022-06-24 9:22 Tianfei Zhang
2022-06-24 9:22 ` [PATCH v3 1/3] mfd: intel-m10-bmc: rename the local variables Tianfei Zhang
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Tianfei Zhang @ 2022-06-24 9:22 UTC (permalink / raw)
To: yilun.xu, lee.jones
Cc: hao.wu, trix, linux-kernel, linux-fpga, russell.h.weight,
matthew.gerlach, Tianfei Zhang
PMCI(Platform Management Control Interface) is a software-visible
interface, connected to card BMC which provided basic register
access functionality from host to Card BMC. This pmci-bmc driver
leverages the regmap APIs to support Intel specific Indirect
Register Interface for register read/write on PMCI driver.
This patchset adding a driver for the PMCI-base interface of Intel
MAX10 BMC controller.
patch 1: use ddata for local variables which directly interacts with
dev_get_drvdata()/dev_set_drvdata().
patch 2: add a driver for PMCI.
patch 3: introduce a new member in intel_m10bmc for the different
base register address of MAX10 CSRs.
v3:
- create a new intel-m10-bmc-pmci driver, and discard the bmc-core
file which adds in v2.
- create a new file for sysfs-driver-intel-m10-bmc-pmci ABI.
- remove the regmap_access_table
- introduce a new member "base" in intel_m10bmc for different base
register address.
- rebased on 5.19-rc3
v2:
- use regmap APIs to support Intel specific Indirect Register Interface
on PMCI driver.
- fix compile warning reported by lkp.
- rebased on 5.19-rc2
Tianfei Zhang (3):
mfd: intel-m10-bmc: rename the local variables
mfd: intel-m10-bmc: add PMCI driver
mfd: intel-m10-bmc: support different BMC base register address
.../testing/sysfs-driver-intel-m10-bmc-pmci | 36 +++
drivers/mfd/Kconfig | 10 +
drivers/mfd/Makefile | 1 +
drivers/mfd/intel-m10-bmc-pmci.c | 278 ++++++++++++++++++
drivers/mfd/intel-m10-bmc.c | 11 +-
include/linux/mfd/intel-m10-bmc.h | 12 +-
6 files changed, 342 insertions(+), 6 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-m10-bmc-pmci
create mode 100644 drivers/mfd/intel-m10-bmc-pmci.c
--
2.26.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v3 1/3] mfd: intel-m10-bmc: rename the local variables 2022-06-24 9:22 [PATCH v3 0/3] add PMCI driver support Tianfei Zhang @ 2022-06-24 9:22 ` Tianfei Zhang 2022-06-25 13:28 ` Tom Rix 2022-06-24 9:22 ` [PATCH v3 2/3] mfd: intel-m10-bmc: add PMCI driver Tianfei Zhang 2022-06-24 9:22 ` [PATCH v3 3/3] mfd: intel-m10-bmc: support different BMC base register address Tianfei Zhang 2 siblings, 1 reply; 10+ messages in thread From: Tianfei Zhang @ 2022-06-24 9:22 UTC (permalink / raw) To: yilun.xu, lee.jones Cc: hao.wu, trix, linux-kernel, linux-fpga, russell.h.weight, matthew.gerlach, Tianfei Zhang It had better use ddata for local variables which directly interacts with dev_get_drvdata()/dev_set_drvdata(). Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com> --- drivers/mfd/intel-m10-bmc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc.c index 8db3bcf5fccc..7e521df29c72 100644 --- a/drivers/mfd/intel-m10-bmc.c +++ b/drivers/mfd/intel-m10-bmc.c @@ -86,15 +86,15 @@ 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); + struct intel_m10bmc *ddata = dev_get_drvdata(dev); unsigned int macaddr_low, macaddr_high; int ret; - ret = m10bmc_sys_read(max10, M10BMC_MAC_LOW, &macaddr_low); + ret = m10bmc_sys_read(ddata, M10BMC_MAC_LOW, &macaddr_low); if (ret) return ret; - ret = m10bmc_sys_read(max10, M10BMC_MAC_HIGH, &macaddr_high); + ret = m10bmc_sys_read(ddata, M10BMC_MAC_HIGH, &macaddr_high); if (ret) return ret; @@ -111,11 +111,11 @@ 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); + struct intel_m10bmc *ddata = dev_get_drvdata(dev); unsigned int macaddr_high; int ret; - ret = m10bmc_sys_read(max10, M10BMC_MAC_HIGH, &macaddr_high); + ret = m10bmc_sys_read(ddata, M10BMC_MAC_HIGH, &macaddr_high); if (ret) return ret; -- 2.26.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] mfd: intel-m10-bmc: rename the local variables 2022-06-24 9:22 ` [PATCH v3 1/3] mfd: intel-m10-bmc: rename the local variables Tianfei Zhang @ 2022-06-25 13:28 ` Tom Rix 2022-06-27 8:04 ` Lee Jones 0 siblings, 1 reply; 10+ messages in thread From: Tom Rix @ 2022-06-25 13:28 UTC (permalink / raw) To: Tianfei Zhang, yilun.xu, lee.jones Cc: hao.wu, linux-kernel, linux-fpga, russell.h.weight, matthew.gerlach On 6/24/22 2:22 AM, Tianfei Zhang wrote: > It had better use ddata for local variables which > directly interacts with dev_get_drvdata()/dev_set_drvdata(). This is a cleanup, not related to the patchset, it should be split from the patchset. Tom > > Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com> > --- > drivers/mfd/intel-m10-bmc.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc.c > index 8db3bcf5fccc..7e521df29c72 100644 > --- a/drivers/mfd/intel-m10-bmc.c > +++ b/drivers/mfd/intel-m10-bmc.c > @@ -86,15 +86,15 @@ 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); > + struct intel_m10bmc *ddata = dev_get_drvdata(dev); > unsigned int macaddr_low, macaddr_high; > int ret; > > - ret = m10bmc_sys_read(max10, M10BMC_MAC_LOW, &macaddr_low); > + ret = m10bmc_sys_read(ddata, M10BMC_MAC_LOW, &macaddr_low); > if (ret) > return ret; > > - ret = m10bmc_sys_read(max10, M10BMC_MAC_HIGH, &macaddr_high); > + ret = m10bmc_sys_read(ddata, M10BMC_MAC_HIGH, &macaddr_high); > if (ret) > return ret; > > @@ -111,11 +111,11 @@ 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); > + struct intel_m10bmc *ddata = dev_get_drvdata(dev); > unsigned int macaddr_high; > int ret; > > - ret = m10bmc_sys_read(max10, M10BMC_MAC_HIGH, &macaddr_high); > + ret = m10bmc_sys_read(ddata, M10BMC_MAC_HIGH, &macaddr_high); > if (ret) > return ret; > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] mfd: intel-m10-bmc: rename the local variables 2022-06-25 13:28 ` Tom Rix @ 2022-06-27 8:04 ` Lee Jones 0 siblings, 0 replies; 10+ messages in thread From: Lee Jones @ 2022-06-27 8:04 UTC (permalink / raw) To: Tom Rix Cc: Tianfei Zhang, yilun.xu, hao.wu, linux-kernel, linux-fpga, russell.h.weight, matthew.gerlach On Sat, 25 Jun 2022, Tom Rix wrote: > > On 6/24/22 2:22 AM, Tianfei Zhang wrote: > > It had better use ddata for local variables which > > directly interacts with dev_get_drvdata()/dev_set_drvdata(). > > This is a cleanup, not related to the patchset, it should be split from the > patchset. So long as they're contained in their own patch, clean-ups are fine. -- Lee Jones [李琼斯] Principal Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] mfd: intel-m10-bmc: add PMCI driver 2022-06-24 9:22 [PATCH v3 0/3] add PMCI driver support Tianfei Zhang 2022-06-24 9:22 ` [PATCH v3 1/3] mfd: intel-m10-bmc: rename the local variables Tianfei Zhang @ 2022-06-24 9:22 ` Tianfei Zhang 2022-06-25 13:52 ` Tom Rix 2022-06-24 9:22 ` [PATCH v3 3/3] mfd: intel-m10-bmc: support different BMC base register address Tianfei Zhang 2 siblings, 1 reply; 10+ messages in thread From: Tianfei Zhang @ 2022-06-24 9:22 UTC (permalink / raw) To: yilun.xu, lee.jones Cc: hao.wu, trix, linux-kernel, linux-fpga, russell.h.weight, matthew.gerlach, Tianfei Zhang 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@intel.com> Signed-off-by: Russ Weight <russell.h.weight@intel.com> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.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@intel.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@intel.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@intel.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@intel.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". 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 -- 2.26.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] mfd: intel-m10-bmc: add PMCI driver 2022-06-24 9:22 ` [PATCH v3 2/3] mfd: intel-m10-bmc: add PMCI driver Tianfei Zhang @ 2022-06-25 13:52 ` Tom Rix 2022-06-27 14:03 ` Zhang, Tianfei 0 siblings, 1 reply; 10+ messages in thread From: Tom Rix @ 2022-06-25 13:52 UTC (permalink / raw) To: Tianfei Zhang, yilun.xu, lee.jones Cc: hao.wu, linux-kernel, linux-fpga, russell.h.weight, matthew.gerlach 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@intel.com> > Signed-off-by: Russ Weight <russell.h.weight@intel.com> > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.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@intel.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@intel.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@intel.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@intel.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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3 2/3] mfd: intel-m10-bmc: add PMCI driver 2022-06-25 13:52 ` Tom Rix @ 2022-06-27 14:03 ` Zhang, Tianfei 2022-06-27 16:30 ` Xu Yilun 0 siblings, 1 reply; 10+ messages in thread From: Zhang, Tianfei @ 2022-06-27 14:03 UTC (permalink / raw) To: Tom Rix, Xu, Yilun, lee.jones@linaro.org Cc: Wu, Hao, linux-kernel@vger.kernel.org, linux-fpga@vger.kernel.org, Weight, Russell H, matthew.gerlach@linux.intel.com > -----Original Message----- > From: Tom Rix <trix@redhat.com> > Sent: Saturday, June 25, 2022 9:53 PM > To: Zhang, Tianfei <tianfei.zhang@intel.com>; Xu, Yilun <yilun.xu@intel.com>; > lee.jones@linaro.org > Cc: Wu, Hao <hao.wu@intel.com>; linux-kernel@vger.kernel.org; linux- > fpga@vger.kernel.org; Weight, Russell H <russell.h.weight@intel.com>; > matthew.gerlach@linux.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@intel.com> > > Signed-off-by: Russ Weight <russell.h.weight@intel.com> > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.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@intel.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@intel.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@intel.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@intel.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. Yes, I agree that the file " sysfs-driver-intel-m10-bmc" and " sysfs-driver-intel-m10-bmc-pmci" are duplicates, I will combine those into one. > > 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 In my V2 patch, I split the intel-m10-bmc.c into intel-m10-bmc-core.c and intel-m10-bmc-pmci.c, intel-m10-bmc-core.c hold the command parts. But Yilun suggested that it is only 40~50 line codes are duplicates, it is no necessary to hold the common parts. In V3 patch, I directly write a separate driver for intel-m10-bmc-pmci driver. > > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] mfd: intel-m10-bmc: add PMCI driver 2022-06-27 14:03 ` Zhang, Tianfei @ 2022-06-27 16:30 ` Xu Yilun 2022-06-28 3:51 ` Zhang, Tianfei 0 siblings, 1 reply; 10+ messages in thread From: Xu Yilun @ 2022-06-27 16:30 UTC (permalink / raw) To: Zhang, Tianfei Cc: Tom Rix, lee.jones@linaro.org, Wu, Hao, linux-kernel@vger.kernel.org, linux-fpga@vger.kernel.org, Weight, Russell H, matthew.gerlach@linux.intel.com On Mon, Jun 27, 2022 at 10:03:35PM +0800, Zhang, Tianfei wrote: > > > > -----Original Message----- > > From: Tom Rix <trix@redhat.com> > > Sent: Saturday, June 25, 2022 9:53 PM > > To: Zhang, Tianfei <tianfei.zhang@intel.com>; Xu, Yilun <yilun.xu@intel.com>; > > lee.jones@linaro.org > > Cc: Wu, Hao <hao.wu@intel.com>; linux-kernel@vger.kernel.org; linux- > > fpga@vger.kernel.org; Weight, Russell H <russell.h.weight@intel.com>; > > matthew.gerlach@linux.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@intel.com> > > > Signed-off-by: Russ Weight <russell.h.weight@intel.com> > > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.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@intel.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@intel.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@intel.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@intel.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. > > Yes, I agree that the file " sysfs-driver-intel-m10-bmc" and " sysfs-driver-intel-m10-bmc-pmci" are duplicates, I will combine those into one. > > > > 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 > > In my V2 patch, I split the intel-m10-bmc.c into intel-m10-bmc-core.c and intel-m10-bmc-pmci.c, intel-m10-bmc-core.c hold the command parts. > But Yilun suggested that it is only 40~50 line codes are duplicates, it is no necessary to hold the common parts. In V3 patch, I directly write a separate > driver for intel-m10-bmc-pmci driver. Hi tianfei: I see the driver still includes intel-m10-bmc.h, and uses struct intel_m10bmc. So I don't think this is a separate driver. I think to be a separate driver, it should remove the dependency to intel_m10bmc. The subdevice drivers should also remove the dependency to intel_m10bmc. So maybe you need to estimate whether to make the separate driver or still reuse the intel-m10-bmc. Thanks, Yilun > > > > > 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH v3 2/3] mfd: intel-m10-bmc: add PMCI driver 2022-06-27 16:30 ` Xu Yilun @ 2022-06-28 3:51 ` Zhang, Tianfei 0 siblings, 0 replies; 10+ messages in thread From: Zhang, Tianfei @ 2022-06-28 3:51 UTC (permalink / raw) To: Xu, Yilun Cc: Tom Rix, lee.jones@linaro.org, Wu, Hao, linux-kernel@vger.kernel.org, linux-fpga@vger.kernel.org, Weight, Russell H, matthew.gerlach@linux.intel.com > -----Original Message----- > From: Xu, Yilun <yilun.xu@intel.com> > Sent: Tuesday, June 28, 2022 12:31 AM > To: Zhang, Tianfei <tianfei.zhang@intel.com> > Cc: Tom Rix <trix@redhat.com>; lee.jones@linaro.org; Wu, Hao > <hao.wu@intel.com>; linux-kernel@vger.kernel.org; linux- > fpga@vger.kernel.org; Weight, Russell H <russell.h.weight@intel.com>; > matthew.gerlach@linux.intel.com > Subject: Re: [PATCH v3 2/3] mfd: intel-m10-bmc: add PMCI driver > > On Mon, Jun 27, 2022 at 10:03:35PM +0800, Zhang, Tianfei wrote: > > > > > > > -----Original Message----- > > > From: Tom Rix <trix@redhat.com> > > > Sent: Saturday, June 25, 2022 9:53 PM > > > To: Zhang, Tianfei <tianfei.zhang@intel.com>; Xu, Yilun > > > <yilun.xu@intel.com>; lee.jones@linaro.org > > > Cc: Wu, Hao <hao.wu@intel.com>; linux-kernel@vger.kernel.org; linux- > > > fpga@vger.kernel.org; Weight, Russell H > > > <russell.h.weight@intel.com>; matthew.gerlach@linux.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@intel.com> > > > > Signed-off-by: Russ Weight <russell.h.weight@intel.com> > > > > Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.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@intel.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@intel.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@intel.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@intel.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. > > > > Yes, I agree that the file " sysfs-driver-intel-m10-bmc" and " sysfs-driver-intel- > m10-bmc-pmci" are duplicates, I will combine those into one. > > > > > > 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 > > > > In my V2 patch, I split the intel-m10-bmc.c into intel-m10-bmc-core.c and > intel-m10-bmc-pmci.c, intel-m10-bmc-core.c hold the command parts. > > But Yilun suggested that it is only 40~50 line codes are duplicates, > > it is no necessary to hold the common parts. In V3 patch, I directly write a > separate driver for intel-m10-bmc-pmci driver. > > Hi tianfei: > > I see the driver still includes intel-m10-bmc.h, and uses struct intel_m10bmc. So I > don't think this is a separate driver. > > I think to be a separate driver, it should remove the dependency to > intel_m10bmc. The subdevice drivers should also remove the dependency to > intel_m10bmc. So maybe you need to estimate whether to make the separate > driver or still reuse the intel-m10-bmc. There are lots of logic and functionalities are very similar with the SPI bus type BMC and PMCI bus type BMC. In my opinion, I like to split into: intel-m10-bmc-core.c, intel-m10-bmc-spi.c and intel-m10-bmc-pmci.c, as Tom's suggestion that it will reduce some duplicate code. In V2 patch, I have done by this idea, https://lore.kernel.org/linux-fpga/20220617020405.128352-1-tianfei.zhang@intel.com if you are all agree this idea, I will send the v4 patchset base on it. > > Thanks, > Yilun > > > > > > > > > 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 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] mfd: intel-m10-bmc: support different BMC base register address 2022-06-24 9:22 [PATCH v3 0/3] add PMCI driver support Tianfei Zhang 2022-06-24 9:22 ` [PATCH v3 1/3] mfd: intel-m10-bmc: rename the local variables Tianfei Zhang 2022-06-24 9:22 ` [PATCH v3 2/3] mfd: intel-m10-bmc: add PMCI driver Tianfei Zhang @ 2022-06-24 9:22 ` Tianfei Zhang 2 siblings, 0 replies; 10+ messages in thread From: Tianfei Zhang @ 2022-06-24 9:22 UTC (permalink / raw) To: yilun.xu, lee.jones Cc: hao.wu, trix, linux-kernel, linux-fpga, russell.h.weight, matthew.gerlach, Tianfei Zhang There are different base addresses for the MAX10 CSR registers. Introducing a new member "base" in intel_m10bmc data structure to support different BMC base register addresses. Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com> --- v3: - use a new member "base" instead of m10bmc_csr data structure. --- drivers/mfd/intel-m10-bmc-pmci.c | 1 + drivers/mfd/intel-m10-bmc.c | 1 + include/linux/mfd/intel-m10-bmc.h | 4 +++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/mfd/intel-m10-bmc-pmci.c b/drivers/mfd/intel-m10-bmc-pmci.c index 93eca4483ac7..26eeda9720dc 100644 --- a/drivers/mfd/intel-m10-bmc-pmci.c +++ b/drivers/mfd/intel-m10-bmc-pmci.c @@ -221,6 +221,7 @@ static int pmci_probe(struct dfl_device *ddev) return -ENOMEM; pmci->m10bmc.dev = dev; + pmci->m10bmc.base = M10BMC_PMCI_SYS_BASE; pmci->dev = dev; pmci->base = devm_ioremap_resource(dev, &ddev->mmio_res); diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc.c index 7e521df29c72..f4cb67629404 100644 --- a/drivers/mfd/intel-m10-bmc.c +++ b/drivers/mfd/intel-m10-bmc.c @@ -171,6 +171,7 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi) return -ENOMEM; ddata->dev = dev; + ddata->base = M10BMC_SYS_BASE; ddata->regmap = devm_regmap_init_spi_avmm(spi, &intel_m10bmc_regmap_config); diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h index 7b58af207b72..0c81dbcdc3dc 100644 --- a/include/linux/mfd/intel-m10-bmc.h +++ b/include/linux/mfd/intel-m10-bmc.h @@ -130,10 +130,12 @@ * struct intel_m10bmc - Intel MAX 10 BMC parent driver data structure * @dev: this device * @regmap: the regmap used to access registers by m10bmc itself + * @base: the base address of MAX10 BMC registers */ struct intel_m10bmc { struct device *dev; struct regmap *regmap; + unsigned int base; }; /* @@ -165,6 +167,6 @@ m10bmc_raw_read(struct intel_m10bmc *m10bmc, unsigned int addr, * M10BMC_SYS_BASE accordingly. */ #define m10bmc_sys_read(m10bmc, offset, val) \ - m10bmc_raw_read(m10bmc, M10BMC_SYS_BASE + (offset), val) + m10bmc_raw_read(m10bmc, (m10bmc)->base + (offset), val) #endif /* __MFD_INTEL_M10_BMC_H */ -- 2.26.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-06-28 3:53 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-06-24 9:22 [PATCH v3 0/3] add PMCI driver support Tianfei Zhang 2022-06-24 9:22 ` [PATCH v3 1/3] mfd: intel-m10-bmc: rename the local variables Tianfei Zhang 2022-06-25 13:28 ` Tom Rix 2022-06-27 8:04 ` Lee Jones 2022-06-24 9:22 ` [PATCH v3 2/3] mfd: intel-m10-bmc: add PMCI driver Tianfei Zhang 2022-06-25 13:52 ` Tom Rix 2022-06-27 14:03 ` Zhang, Tianfei 2022-06-27 16:30 ` Xu Yilun 2022-06-28 3:51 ` Zhang, Tianfei 2022-06-24 9:22 ` [PATCH v3 3/3] mfd: intel-m10-bmc: support different BMC base register address Tianfei Zhang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.