Hi all,
Here are the patches for MAX 10 BMC core/SPI interface split and
addition of the PMCI interface. There are a few supporting rearrangement
patches prior to the actual split. In this version, the indirect register
access became part of the BMC PMCI module.
The current downside of the split is that there's not that much code
remaining in the core part when all the type specific definitions are
moved to the file with the relevant interface.
The patch set is based on top of the "fpga: m10bmc-sec: Fix probe
rollback" and commit dfd10332596e ("fpga: m10bmc-sec: Fix kconfig
dependencies") to avoid triggering conflicts.
v5:
- Explain flash MUX rollback path in commit message
- Fix RSU status field register location
v4:
- Use board type for naming defines & structs
- Set .reg_read/write and call devm_regmap_init() directly
- Rename indirect_bus_*() funcs to indirect_*()
- Move indirect code into intel-m10-bmc-pmci so funcs can be static
- Drop module licence GPL v2 - GPL change
v3:
- Move regmap indirect into BMC PMCI module
- Get rid of "generalization" of cmd offsets and values, back to v1 #defines
- Tweak Kconfig & Makefile
- Rename intel-m10-bmc-pmci to intel-m10-bmc-pmci-main
- Rework sec update read/write paths
- Sec update driver code effectively uses the SPI side code from v2
- Rename m10bmc_sec_write() to m10bmc_sec_fw_write()
- Rename flash_ops to flash_bulk_ops and make them optional
- Move flash MUX and flash_mutex handling into sec update driver
- Prevent simultaneous flash bulk write & read using flash_mutex
- Keep M10BMC_STAGING_BASE in header (now used in the sec update code)
- Rebased on top of leak fix "fpga: m10bmc-sec: Fix probe rollback"
v2:
- Drop type from mfd side, the platform info takes care of differentation
- Explain introducing ->info to struct m10bmc in commit message (belongs logically there)
- Intro PMCI better
- Improve naming
- Rename M10BMC_PMCI_FLASH_CTRL to M10BMC_PMCI_FLASH_MUX_CTRL
- Use m10bmc_pmci/M10BMC_PMCI prefix consistently
- Use M10BMC_SPI prefix for SPI related defines
- Newly added stuff gets m10bmc_spi prefix
- Removed dev from struct m10bmc_pmci_device
- Rename "n_offset" variable to "offset" in PMCI flash ops
- Always issue idle command in regmap indirect to clear RD/WR/ACK bits
- Handle stride misaligned sizes in flash read/write ops
Ilpo Järvinen (10):
mfd: intel-m10-bmc: Create m10bmc_platform_info for type specific info
mfd: intel-m10-bmc: Rename the local variables
mfd: intel-m10-bmc: Split into core and spi specific parts
mfd: intel-m10-bmc: Support multiple CSR register layouts
fpga: intel-m10-bmc: Rework flash read/write
mfd: intel-m10-bmc: Downscope SPI defines & prefix with M10BMC_N3000
fpga: m10bmc-sec: Create helpers for rsu status/progress checks
fpga: m10bmc-sec: Differentiate rsu status from doorbell in csr map
mfd: intel-m10-bmc: Add PMCI driver
fpga: m10bmc-sec: Add support for N6000
.../ABI/testing/sysfs-driver-intel-m10-bmc | 8 +-
MAINTAINERS | 2 +-
drivers/fpga/Kconfig | 2 +-
drivers/fpga/intel-m10-bmc-sec-update.c | 386 +++++++++++------
drivers/hwmon/Kconfig | 2 +-
drivers/mfd/Kconfig | 32 +-
drivers/mfd/Makefile | 5 +-
drivers/mfd/intel-m10-bmc-core.c | 133 ++++++
drivers/mfd/intel-m10-bmc-pmci.c | 398 ++++++++++++++++++
drivers/mfd/intel-m10-bmc-spi.c | 206 +++++++++
drivers/mfd/intel-m10-bmc.c | 238 -----------
include/linux/mfd/intel-m10-bmc.h | 111 ++---
12 files changed, 1094 insertions(+), 429 deletions(-)
create mode 100644 drivers/mfd/intel-m10-bmc-core.c
create mode 100644 drivers/mfd/intel-m10-bmc-pmci.c
create mode 100644 drivers/mfd/intel-m10-bmc-spi.c
delete mode 100644 drivers/mfd/intel-m10-bmc.c
--
2.30.2
Access to flash staging area is different for N6000 from that of the
SPI interfaced counterparts. To make it easier to differentiate flash
access path, move read/write into new functions where the new access
path can be easily placed into. Rework the unaligned access such the
behavior it matches for both read and write.
This change also renames m10bmc_sec_write() to m10bmc_sec_fw_write() as
it would have a name conflict otherwise.
Co-developed-by: Tianfei zhang <[email protected]>
Signed-off-by: Tianfei zhang <[email protected]>
Co-developed-by: Russ Weight <[email protected]>
Signed-off-by: Russ Weight <[email protected]>
Acked-by: Xu Yilun <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/fpga/intel-m10-bmc-sec-update.c | 143 +++++++++++++-----------
1 file changed, 79 insertions(+), 64 deletions(-)
diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c
index 798c1828899b..9922027856a4 100644
--- a/drivers/fpga/intel-m10-bmc-sec-update.c
+++ b/drivers/fpga/intel-m10-bmc-sec-update.c
@@ -31,6 +31,65 @@ static DEFINE_XARRAY_ALLOC(fw_upload_xa);
#define REH_MAGIC GENMASK(15, 0)
#define REH_SHA_NUM_BYTES GENMASK(31, 16)
+static int m10bmc_sec_write(struct m10bmc_sec *sec, const u8 *buf, u32 offset, u32 size)
+{
+ struct intel_m10bmc *m10bmc = sec->m10bmc;
+ unsigned int stride = regmap_get_reg_stride(m10bmc->regmap);
+ u32 write_count = size / stride;
+ u32 leftover_offset = write_count * stride;
+ u32 leftover_size = size - leftover_offset;
+ u32 leftover_tmp = 0;
+ int ret;
+
+ if (WARN_ON_ONCE(stride > sizeof(leftover_tmp)))
+ return -EINVAL;
+
+ ret = regmap_bulk_write(m10bmc->regmap, M10BMC_STAGING_BASE + offset,
+ buf + offset, write_count);
+ if (ret)
+ return ret;
+
+ /* If size is not aligned to stride, handle the remainder bytes with regmap_write() */
+ if (leftover_size) {
+ memcpy(&leftover_tmp, buf + leftover_offset, leftover_size);
+ ret = regmap_write(m10bmc->regmap, M10BMC_STAGING_BASE + offset + leftover_offset,
+ leftover_tmp);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int m10bmc_sec_read(struct m10bmc_sec *sec, u8 *buf, u32 addr, u32 size)
+{
+ struct intel_m10bmc *m10bmc = sec->m10bmc;
+ unsigned int stride = regmap_get_reg_stride(m10bmc->regmap);
+ u32 read_count = size / stride;
+ u32 leftover_offset = read_count * stride;
+ u32 leftover_size = size - leftover_offset;
+ u32 leftover_tmp;
+ int ret;
+
+ if (WARN_ON_ONCE(stride > sizeof(leftover_tmp)))
+ return -EINVAL;
+
+ ret = regmap_bulk_read(m10bmc->regmap, addr, buf, read_count);
+ if (ret)
+ return ret;
+
+ /* If size is not aligned to stride, handle the remainder bytes with regmap_read() */
+ if (leftover_size) {
+ ret = regmap_read(m10bmc->regmap, addr + leftover_offset, &leftover_tmp);
+ if (ret)
+ return ret;
+ memcpy(buf + leftover_offset, &leftover_tmp, leftover_size);
+ }
+
+ return 0;
+}
+
+
static ssize_t
show_root_entry_hash(struct device *dev, u32 exp_magic,
u32 prog_addr, u32 reh_addr, char *buf)
@@ -38,11 +97,9 @@ show_root_entry_hash(struct device *dev, u32 exp_magic,
struct m10bmc_sec *sec = dev_get_drvdata(dev);
int sha_num_bytes, i, ret, cnt = 0;
u8 hash[REH_SHA384_SIZE];
- unsigned int stride;
u32 magic;
- stride = regmap_get_reg_stride(sec->m10bmc->regmap);
- ret = m10bmc_raw_read(sec->m10bmc, prog_addr, &magic);
+ ret = m10bmc_sec_read(sec, (u8 *)&magic, prog_addr, sizeof(magic));
if (ret)
return ret;
@@ -50,19 +107,16 @@ show_root_entry_hash(struct device *dev, u32 exp_magic,
return sysfs_emit(buf, "hash not programmed\n");
sha_num_bytes = FIELD_GET(REH_SHA_NUM_BYTES, magic) / 8;
- if ((sha_num_bytes % stride) ||
- (sha_num_bytes != REH_SHA256_SIZE &&
- sha_num_bytes != REH_SHA384_SIZE)) {
+ if (sha_num_bytes != REH_SHA256_SIZE &&
+ sha_num_bytes != REH_SHA384_SIZE) {
dev_err(sec->dev, "%s bad sha num bytes %d\n", __func__,
sha_num_bytes);
return -EINVAL;
}
- ret = regmap_bulk_read(sec->m10bmc->regmap, reh_addr,
- hash, sha_num_bytes / stride);
+ ret = m10bmc_sec_read(sec, hash, reh_addr, sha_num_bytes);
if (ret) {
- dev_err(dev, "failed to read root entry hash: %x cnt %x: %d\n",
- reh_addr, sha_num_bytes / stride, ret);
+ dev_err(dev, "failed to read root entry hash\n");
return ret;
}
@@ -98,27 +152,16 @@ DEVICE_ATTR_SEC_REH_RO(pr);
static ssize_t
show_canceled_csk(struct device *dev, u32 addr, char *buf)
{
- unsigned int i, stride, size = CSK_32ARRAY_SIZE * sizeof(u32);
+ unsigned int i, size = CSK_32ARRAY_SIZE * sizeof(u32);
struct m10bmc_sec *sec = dev_get_drvdata(dev);
DECLARE_BITMAP(csk_map, CSK_BIT_LEN);
__le32 csk_le32[CSK_32ARRAY_SIZE];
u32 csk32[CSK_32ARRAY_SIZE];
int ret;
- stride = regmap_get_reg_stride(sec->m10bmc->regmap);
- if (size % stride) {
- dev_err(sec->dev,
- "CSK vector size (0x%x) not aligned to stride (0x%x)\n",
- size, stride);
- WARN_ON_ONCE(1);
- return -EINVAL;
- }
-
- ret = regmap_bulk_read(sec->m10bmc->regmap, addr, csk_le32,
- size / stride);
+ ret = m10bmc_sec_read(sec, (u8 *)&csk_le32, addr, size);
if (ret) {
- dev_err(sec->dev, "failed to read CSK vector: %x cnt %x: %d\n",
- addr, size / stride, ret);
+ dev_err(sec->dev, "failed to read CSK vector\n");
return ret;
}
@@ -157,31 +200,20 @@ static ssize_t flash_count_show(struct device *dev,
{
struct m10bmc_sec *sec = dev_get_drvdata(dev);
const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;
- unsigned int stride, num_bits;
+ unsigned int num_bits;
u8 *flash_buf;
int cnt, ret;
- stride = regmap_get_reg_stride(sec->m10bmc->regmap);
num_bits = FLASH_COUNT_SIZE * 8;
- if (FLASH_COUNT_SIZE % stride) {
- dev_err(sec->dev,
- "FLASH_COUNT_SIZE (0x%x) not aligned to stride (0x%x)\n",
- FLASH_COUNT_SIZE, stride);
- WARN_ON_ONCE(1);
- return -EINVAL;
- }
-
flash_buf = kmalloc(FLASH_COUNT_SIZE, GFP_KERNEL);
if (!flash_buf)
return -ENOMEM;
- ret = regmap_bulk_read(sec->m10bmc->regmap, csr_map->rsu_update_counter,
- flash_buf, FLASH_COUNT_SIZE / stride);
+ ret = m10bmc_sec_read(sec, flash_buf, csr_map->rsu_update_counter,
+ FLASH_COUNT_SIZE);
if (ret) {
- dev_err(sec->dev,
- "failed to read flash count: %x cnt %x: %d\n",
- csr_map->rsu_update_counter, FLASH_COUNT_SIZE / stride, ret);
+ dev_err(sec->dev, "failed to read flash count\n");
goto exit_free;
}
cnt = num_bits - bitmap_weight((unsigned long *)flash_buf, num_bits);
@@ -465,20 +497,19 @@ static enum fw_upload_err m10bmc_sec_prepare(struct fw_upload *fwl,
#define WRITE_BLOCK_SIZE 0x4000 /* Default write-block size is 0x4000 bytes */
-static enum fw_upload_err m10bmc_sec_write(struct fw_upload *fwl, const u8 *data,
- u32 offset, u32 size, u32 *written)
+static enum fw_upload_err m10bmc_sec_fw_write(struct fw_upload *fwl, const u8 *data,
+ u32 offset, u32 size, u32 *written)
{
struct m10bmc_sec *sec = fwl->dd_handle;
const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;
- u32 blk_size, doorbell, extra_offset;
- unsigned int stride, extra = 0;
+ struct intel_m10bmc *m10bmc = sec->m10bmc;
+ u32 blk_size, doorbell;
int ret;
- stride = regmap_get_reg_stride(sec->m10bmc->regmap);
if (sec->cancel_request)
return rsu_cancel(sec);
- ret = m10bmc_sys_read(sec->m10bmc, csr_map->doorbell, &doorbell);
+ ret = m10bmc_sys_read(m10bmc, csr_map->doorbell, &doorbell);
if (ret) {
return FW_UPLOAD_ERR_RW_ERROR;
} else if (rsu_prog(doorbell) != RSU_PROG_READY) {
@@ -486,28 +517,12 @@ static enum fw_upload_err m10bmc_sec_write(struct fw_upload *fwl, const u8 *data
return FW_UPLOAD_ERR_HW_ERROR;
}
- WARN_ON_ONCE(WRITE_BLOCK_SIZE % stride);
+ WARN_ON_ONCE(WRITE_BLOCK_SIZE % regmap_get_reg_stride(m10bmc->regmap));
blk_size = min_t(u32, WRITE_BLOCK_SIZE, size);
- ret = regmap_bulk_write(sec->m10bmc->regmap,
- M10BMC_STAGING_BASE + offset,
- (void *)data + offset,
- blk_size / stride);
+ ret = m10bmc_sec_write(sec, data, offset, blk_size);
if (ret)
return FW_UPLOAD_ERR_RW_ERROR;
- /*
- * If blk_size is not aligned to stride, then handle the extra
- * bytes with regmap_write.
- */
- if (blk_size % stride) {
- extra_offset = offset + ALIGN_DOWN(blk_size, stride);
- memcpy(&extra, (u8 *)(data + extra_offset), blk_size % stride);
- ret = regmap_write(sec->m10bmc->regmap,
- M10BMC_STAGING_BASE + extra_offset, extra);
- if (ret)
- return FW_UPLOAD_ERR_RW_ERROR;
- }
-
*written = blk_size;
return FW_UPLOAD_ERR_NONE;
}
@@ -568,7 +583,7 @@ static void m10bmc_sec_cleanup(struct fw_upload *fwl)
static const struct fw_upload_ops m10bmc_ops = {
.prepare = m10bmc_sec_prepare,
- .write = m10bmc_sec_write,
+ .write = m10bmc_sec_fw_write,
.poll_complete = m10bmc_sec_poll_complete,
.cancel = m10bmc_sec_cancel,
.cleanup = m10bmc_sec_cleanup,
--
2.30.2
Add the mfd driver for the Platform Management Component Interface
(PMCI) based interface of Intel MAX10 BMC controller.
PMCI is a software-visible interface, connected to card BMC which
provided the basic functionality of read/write BMC register. The access
to the register is done indirectly via a hardware controller/bridge
that handles read/write/clear commands and acknowledgments for the
commands.
Previously, intel-m10-bmc provided sysfs under
/sys/bus/spi/devices/... which is generalized in this change because
not all MAX10 BMC appear under SPI anymore.
Co-developed-by: Tianfei zhang <[email protected]>
Signed-off-by: Tianfei zhang <[email protected]>
Co-developed-by: Russ Weight <[email protected]>
Signed-off-by: Russ Weight <[email protected]>
Co-developed-by: Matthew Gerlach <[email protected]>
Signed-off-by: Matthew Gerlach <[email protected]>
Reviewed-by: Xu Yilun <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
---
.../ABI/testing/sysfs-driver-intel-m10-bmc | 8 +-
drivers/mfd/Kconfig | 12 +
drivers/mfd/Makefile | 1 +
drivers/mfd/intel-m10-bmc-pmci.c | 253 ++++++++++++++++++
4 files changed, 270 insertions(+), 4 deletions(-)
create mode 100644 drivers/mfd/intel-m10-bmc-pmci.c
diff --git a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
index 9773925138af..a8ab58035c95 100644
--- a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
+++ b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
@@ -1,4 +1,4 @@
-What: /sys/bus/spi/devices/.../bmc_version
+What: /sys/bus/.../drivers/intel-m10-bmc/.../bmc_version
Date: June 2020
KernelVersion: 5.10
Contact: Xu Yilun <[email protected]>
@@ -6,7 +6,7 @@ Description: Read only. Returns the hardware build version of Intel
MAX10 BMC chip.
Format: "0x%x".
-What: /sys/bus/spi/devices/.../bmcfw_version
+What: /sys/bus/.../drivers/intel-m10-bmc/.../bmcfw_version
Date: June 2020
KernelVersion: 5.10
Contact: Xu Yilun <[email protected]>
@@ -14,7 +14,7 @@ Description: Read only. Returns the firmware version of Intel MAX10
BMC chip.
Format: "0x%x".
-What: /sys/bus/spi/devices/.../mac_address
+What: /sys/bus/.../drivers/intel-m10-bmc/.../mac_address
Date: January 2021
KernelVersion: 5.12
Contact: Russ Weight <[email protected]>
@@ -25,7 +25,7 @@ Description: Read only. Returns the first MAC address in a block
space.
Format: "%02x:%02x:%02x:%02x:%02x:%02x".
-What: /sys/bus/spi/devices/.../mac_count
+What: /sys/bus/.../drivers/intel-m10-bmc/.../mac_count
Date: January 2021
KernelVersion: 5.12
Contact: Russ Weight <[email protected]>
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index a09d4ac60dc7..82f13614d98a 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2238,6 +2238,18 @@ config MFD_INTEL_M10_BMC_SPI
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
+ select MFD_INTEL_M10_BMC_CORE
+ select REGMAP
+ 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 5d1f308ee2a7..c90fb96cad2a 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -274,6 +274,7 @@ obj-$(CONFIG_MFD_SIMPLE_MFD_I2C) += simple-mfd-i2c.o
obj-$(CONFIG_MFD_INTEL_M10_BMC_CORE) += intel-m10-bmc-core.o
obj-$(CONFIG_MFD_INTEL_M10_BMC_SPI) += intel-m10-bmc-spi.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..63ec0f6aba8b
--- /dev/null
+++ b/drivers/mfd/intel-m10-bmc-pmci.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MFD driver for Platform Management Component Interface (PMCI) based
+ * interface to MAX10 BMC.
+ *
+ * Copyright (C) 2020-2022 Intel Corporation.
+ */
+
+#include <linux/device.h>
+#include <linux/dfl.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/intel-m10-bmc.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#define M10BMC_PMCI_INDIRECT_BASE 0x400
+
+#define M10BMC_N6000_SYS_BASE 0x0
+#define M10BMC_N6000_SYS_END 0xfff
+
+#define M10BMC_N6000_DOORBELL 0x1c0
+#define M10BMC_N6000_AUTH_RESULT 0x1c4
+
+/* Telemetry registers */
+#define M10BMC_N6000_TELEM_START 0x400
+#define M10BMC_N6000_TELEM_END 0x78c
+
+#define M10BMC_N6000_BUILD_VER 0x0
+#define NIOS2_N6000_FW_VERSION 0x4
+#define M10BMC_N6000_MAC_LOW 0x20
+#define M10BMC_N6000_MAC_HIGH (M10BMC_N6000_MAC_LOW + 4)
+
+/* Addresses for security related data in FLASH */
+#define M10BMC_N6000_BMC_REH_ADDR 0x7ffc004
+#define M10BMC_N6000_BMC_PROG_ADDR 0x7ffc000
+#define M10BMC_N6000_BMC_PROG_MAGIC 0x5746
+
+#define M10BMC_N6000_SR_REH_ADDR 0x7ffd004
+#define M10BMC_N6000_SR_PROG_ADDR 0x7ffd000
+#define M10BMC_N6000_SR_PROG_MAGIC 0x5253
+
+#define M10BMC_N6000_PR_REH_ADDR 0x7ffe004
+#define M10BMC_N6000_PR_PROG_ADDR 0x7ffe000
+#define M10BMC_N6000_PR_PROG_MAGIC 0x5250
+
+#define M10BMC_N6000_STAGING_FLASH_COUNT 0x7ff5000
+
+struct m10bmc_pmci_device {
+ void __iomem *base;
+ struct intel_m10bmc m10bmc;
+};
+
+/*
+ * Intel FGPA indirect register access via hardware controller/bridge.
+ */
+#define INDIRECT_CMD_OFF 0
+#define INDIRECT_CMD_CLR 0
+#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
+
+#define INDIRECT_INT_US 1
+#define INDIRECT_TIMEOUT_US 10000
+
+struct indirect_ctx {
+ void __iomem *base;
+ struct device *dev;
+};
+
+static int indirect_clear_cmd(struct indirect_ctx *ctx)
+{
+ unsigned int cmd;
+ int ret;
+
+ writel(INDIRECT_CMD_CLR, ctx->base + INDIRECT_CMD_OFF);
+
+ ret = readl_poll_timeout(ctx->base + INDIRECT_CMD_OFF, cmd,
+ cmd == INDIRECT_CMD_CLR,
+ INDIRECT_INT_US, INDIRECT_TIMEOUT_US);
+ if (ret)
+ dev_err(ctx->dev, "timed out waiting clear cmd (residual cmd=0x%x)\n", cmd);
+
+ return ret;
+}
+
+static int indirect_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+ struct indirect_ctx *ctx = context;
+ unsigned int cmd, ack, tmpval;
+ int ret;
+
+ cmd = readl(ctx->base + INDIRECT_CMD_OFF);
+ if (cmd != INDIRECT_CMD_CLR)
+ dev_warn(ctx->dev, "residual cmd 0x%x on read entry\n", 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, ack,
+ (ack & INDIRECT_CMD_ACK) == INDIRECT_CMD_ACK,
+ INDIRECT_INT_US, INDIRECT_TIMEOUT_US);
+ if (ret)
+ dev_err(ctx->dev, "read timed out on reg 0x%x ack 0x%x\n", reg, ack);
+ else
+ tmpval = readl(ctx->base + INDIRECT_RD_OFF);
+
+ if (indirect_clear_cmd(ctx)) {
+ if (!ret)
+ ret = -ETIMEDOUT;
+ goto out;
+ }
+
+ *val = tmpval;
+out:
+ return ret;
+}
+
+static int indirect_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+ struct indirect_ctx *ctx = context;
+ unsigned int cmd, ack;
+ int ret;
+
+ cmd = readl(ctx->base + INDIRECT_CMD_OFF);
+ if (cmd != INDIRECT_CMD_CLR)
+ dev_warn(ctx->dev, "residual cmd 0x%x on write entry\n", 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, ack,
+ (ack & INDIRECT_CMD_ACK) == INDIRECT_CMD_ACK,
+ INDIRECT_INT_US, INDIRECT_TIMEOUT_US);
+ if (ret)
+ dev_err(ctx->dev, "write timed out on reg 0x%x ack 0x%x\n", reg, ack);
+
+ if (indirect_clear_cmd(ctx)) {
+ if (!ret)
+ ret = -ETIMEDOUT;
+ }
+
+ return ret;
+}
+
+static const struct regmap_range m10bmc_pmci_regmap_range[] = {
+ regmap_reg_range(M10BMC_N6000_SYS_BASE, M10BMC_N6000_SYS_END),
+};
+
+static const struct regmap_access_table m10bmc_pmci_access_table = {
+ .yes_ranges = m10bmc_pmci_regmap_range,
+ .n_yes_ranges = ARRAY_SIZE(m10bmc_pmci_regmap_range),
+};
+
+static struct regmap_config m10bmc_pmci_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .wr_table = &m10bmc_pmci_access_table,
+ .rd_table = &m10bmc_pmci_access_table,
+ .reg_read = &indirect_reg_read,
+ .reg_write = &indirect_reg_write,
+ .max_register = M10BMC_N6000_SYS_END,
+};
+
+static struct mfd_cell m10bmc_pmci_n6000_bmc_subdevs[] = {
+ { .name = "n6000bmc-hwmon" },
+};
+
+static const struct m10bmc_csr_map m10bmc_n6000_csr_map = {
+ .base = M10BMC_N6000_SYS_BASE,
+ .build_version = M10BMC_N6000_BUILD_VER,
+ .fw_version = NIOS2_N6000_FW_VERSION,
+ .mac_low = M10BMC_N6000_MAC_LOW,
+ .mac_high = M10BMC_N6000_MAC_HIGH,
+ .doorbell = M10BMC_N6000_DOORBELL,
+ .auth_result = M10BMC_N6000_AUTH_RESULT,
+ .rsu_status = M10BMC_N6000_AUTH_RESULT,
+ .bmc_prog_addr = M10BMC_N6000_BMC_PROG_ADDR,
+ .bmc_reh_addr = M10BMC_N6000_BMC_REH_ADDR,
+ .bmc_magic = M10BMC_N6000_BMC_PROG_MAGIC,
+ .sr_prog_addr = M10BMC_N6000_SR_PROG_ADDR,
+ .sr_reh_addr = M10BMC_N6000_SR_REH_ADDR,
+ .sr_magic = M10BMC_N6000_SR_PROG_MAGIC,
+ .pr_prog_addr = M10BMC_N6000_PR_PROG_ADDR,
+ .pr_reh_addr = M10BMC_N6000_PR_REH_ADDR,
+ .pr_magic = M10BMC_N6000_PR_PROG_MAGIC,
+ .rsu_update_counter = M10BMC_N6000_STAGING_FLASH_COUNT,
+};
+
+static const struct intel_m10bmc_platform_info m10bmc_pmci_n6000 = {
+ .cells = m10bmc_pmci_n6000_bmc_subdevs,
+ .n_cells = ARRAY_SIZE(m10bmc_pmci_n6000_bmc_subdevs),
+ .csr_map = &m10bmc_n6000_csr_map,
+};
+
+static int m10bmc_pmci_probe(struct dfl_device *ddev)
+{
+ struct device *dev = &ddev->dev;
+ struct m10bmc_pmci_device *pmci;
+ struct indirect_ctx *ctx;
+
+ pmci = devm_kzalloc(dev, sizeof(*pmci), GFP_KERNEL);
+ if (!pmci)
+ return -ENOMEM;
+
+ pmci->m10bmc.dev = dev;
+
+ pmci->base = devm_ioremap_resource(dev, &ddev->mmio_res);
+ if (IS_ERR(pmci->base))
+ return PTR_ERR(pmci->base);
+
+ ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+ if (!ctx)
+ return -ENOMEM;
+
+ ctx->base = pmci->base + M10BMC_PMCI_INDIRECT_BASE;
+ ctx->dev = dev;
+ indirect_clear_cmd(ctx);
+ pmci->m10bmc.regmap = devm_regmap_init(dev, NULL, ctx, &m10bmc_pmci_regmap_config);
+
+ if (IS_ERR(pmci->m10bmc.regmap))
+ return PTR_ERR(pmci->m10bmc.regmap);
+
+ return m10bmc_dev_init(&pmci->m10bmc, &m10bmc_pmci_n6000);
+}
+
+#define FME_FEATURE_ID_M10BMC_PMCI 0x12
+
+static const struct dfl_device_id m10bmc_pmci_ids[] = {
+ { FME_ID, FME_FEATURE_ID_M10BMC_PMCI },
+ { }
+};
+MODULE_DEVICE_TABLE(dfl, m10bmc_pmci_ids);
+
+static struct dfl_driver m10bmc_pmci_driver = {
+ .drv = {
+ .name = "intel-m10-bmc",
+ .dev_groups = m10bmc_dev_groups,
+ },
+ .id_table = m10bmc_pmci_ids,
+ .probe = m10bmc_pmci_probe,
+};
+
+module_dfl_driver(m10bmc_pmci_driver);
+
+MODULE_DESCRIPTION("MAX10 BMC PMCI-based interface");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL");
--
2.30.2
Move SPI based board definitions to per interface file from the global
header. This makes it harder to use them accidently in the
generic/interface agnostic code. Prefix the defines with M10BMC_N3000
to make it more obvious these are related to some board type. All
current non-N3000 board types have the same layout so they'll be
reused.
Some bitfield defs are also moved to intel-m10-bmc-core which seems
more appropriate for them.
Reviewed-by: Russ Weight <[email protected]>
Reviewed-by: Xu Yilun <[email protected]>
Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/mfd/intel-m10-bmc-core.c | 11 ++++
drivers/mfd/intel-m10-bmc-spi.c | 89 ++++++++++++++++++++++---------
include/linux/mfd/intel-m10-bmc.h | 46 ----------------
3 files changed, 74 insertions(+), 72 deletions(-)
diff --git a/drivers/mfd/intel-m10-bmc-core.c b/drivers/mfd/intel-m10-bmc-core.c
index 51b78b868235..50a4ec758bdb 100644
--- a/drivers/mfd/intel-m10-bmc-core.c
+++ b/drivers/mfd/intel-m10-bmc-core.c
@@ -12,6 +12,17 @@
#include <linux/mfd/intel-m10-bmc.h>
#include <linux/module.h>
+/* Register fields of system registers */
+#define M10BMC_MAC_BYTE4 GENMASK(7, 0)
+#define M10BMC_MAC_BYTE3 GENMASK(15, 8)
+#define M10BMC_MAC_BYTE2 GENMASK(23, 16)
+#define M10BMC_MAC_BYTE1 GENMASK(31, 24)
+#define M10BMC_MAC_BYTE6 GENMASK(7, 0)
+#define M10BMC_MAC_BYTE5 GENMASK(15, 8)
+#define M10BMC_MAC_COUNT GENMASK(23, 16)
+#define M10BMC_VER_MAJOR_MSK GENMASK(23, 16)
+#define M10BMC_VER_PCB_INFO_MSK GENMASK(31, 24)
+
static ssize_t bmc_version_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
diff --git a/drivers/mfd/intel-m10-bmc-spi.c b/drivers/mfd/intel-m10-bmc-spi.c
index e8986154e965..04c83f9c6492 100644
--- a/drivers/mfd/intel-m10-bmc-spi.c
+++ b/drivers/mfd/intel-m10-bmc-spi.c
@@ -13,10 +13,47 @@
#include <linux/regmap.h>
#include <linux/spi/spi.h>
+#define M10BMC_N3000_LEGACY_BUILD_VER 0x300468
+#define M10BMC_N3000_SYS_BASE 0x300800
+#define M10BMC_N3000_SYS_END 0x300fff
+#define M10BMC_N3000_FLASH_BASE 0x10000000
+#define M10BMC_N3000_FLASH_END 0x1fffffff
+#define M10BMC_N3000_MEM_END M10BMC_N3000_FLASH_END
+
+/* Register offset of system registers */
+#define NIOS2_FW_VERSION 0x0
+#define M10BMC_N3000_MAC_LOW 0x10
+#define M10BMC_N3000_MAC_HIGH 0x14
+#define M10BMC_N3000_TEST_REG 0x3c
+#define M10BMC_N3000_BUILD_VER 0x68
+#define M10BMC_N3000_VER_LEGACY_INVALID 0xffffffff
+
+/* Secure update doorbell register, in system register region */
+#define M10BMC_N3000_DOORBELL 0x400
+
+/* Authorization Result register, in system register region */
+#define M10BMC_N3000_AUTH_RESULT 0x404
+
+/* Addresses for security related data in FLASH */
+#define M10BMC_N3000_BMC_REH_ADDR 0x17ffc004
+#define M10BMC_N3000_BMC_PROG_ADDR 0x17ffc000
+#define M10BMC_N3000_BMC_PROG_MAGIC 0x5746
+
+#define M10BMC_N3000_SR_REH_ADDR 0x17ffd004
+#define M10BMC_N3000_SR_PROG_ADDR 0x17ffd000
+#define M10BMC_N3000_SR_PROG_MAGIC 0x5253
+
+#define M10BMC_N3000_PR_REH_ADDR 0x17ffe004
+#define M10BMC_N3000_PR_PROG_ADDR 0x17ffe000
+#define M10BMC_N3000_PR_PROG_MAGIC 0x5250
+
+/* Address of 4KB inverted bit vector containing staging area FLASH count */
+#define M10BMC_N3000_STAGING_FLASH_COUNT 0x17ffb000
+
static const struct regmap_range m10bmc_regmap_range[] = {
- regmap_reg_range(M10BMC_LEGACY_BUILD_VER, M10BMC_LEGACY_BUILD_VER),
- regmap_reg_range(M10BMC_SYS_BASE, M10BMC_SYS_END),
- regmap_reg_range(M10BMC_FLASH_BASE, M10BMC_FLASH_END),
+ regmap_reg_range(M10BMC_N3000_LEGACY_BUILD_VER, M10BMC_N3000_LEGACY_BUILD_VER),
+ regmap_reg_range(M10BMC_N3000_SYS_BASE, M10BMC_N3000_SYS_END),
+ regmap_reg_range(M10BMC_N3000_FLASH_BASE, M10BMC_N3000_FLASH_END),
};
static const struct regmap_access_table m10bmc_access_table = {
@@ -30,7 +67,7 @@ static struct regmap_config intel_m10bmc_regmap_config = {
.reg_stride = 4,
.wr_table = &m10bmc_access_table,
.rd_table = &m10bmc_access_table,
- .max_register = M10BMC_MEM_END,
+ .max_register = M10BMC_N3000_MEM_END,
};
static int check_m10bmc_version(struct intel_m10bmc *ddata)
@@ -41,16 +78,16 @@ static int check_m10bmc_version(struct intel_m10bmc *ddata)
/*
* This check is to filter out the very old legacy BMC versions. In the
* old BMC chips, the BMC version info is stored in the old version
- * register (M10BMC_LEGACY_BUILD_VER), so its read out value would have
- * not been M10BMC_VER_LEGACY_INVALID (0xffffffff). But in new BMC
+ * register (M10BMC_N3000_LEGACY_BUILD_VER), so its read out value would have
+ * not been M10BMC_N3000_VER_LEGACY_INVALID (0xffffffff). But in new BMC
* chips that the driver supports, the value of this register should be
- * M10BMC_VER_LEGACY_INVALID.
+ * M10BMC_N3000_VER_LEGACY_INVALID.
*/
- ret = m10bmc_raw_read(ddata, M10BMC_LEGACY_BUILD_VER, &v);
+ ret = m10bmc_raw_read(ddata, M10BMC_N3000_LEGACY_BUILD_VER, &v);
if (ret)
return -ENODEV;
- if (v != M10BMC_VER_LEGACY_INVALID) {
+ if (v != M10BMC_N3000_VER_LEGACY_INVALID) {
dev_err(ddata->dev, "bad version M10BMC detected\n");
return -ENODEV;
}
@@ -92,24 +129,24 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
}
static const struct m10bmc_csr_map m10bmc_n3000_csr_map = {
- .base = M10BMC_SYS_BASE,
- .build_version = M10BMC_BUILD_VER,
+ .base = M10BMC_N3000_SYS_BASE,
+ .build_version = M10BMC_N3000_BUILD_VER,
.fw_version = NIOS2_FW_VERSION,
- .mac_low = M10BMC_MAC_LOW,
- .mac_high = M10BMC_MAC_HIGH,
- .doorbell = M10BMC_DOORBELL,
- .auth_result = M10BMC_AUTH_RESULT,
- .rsu_status = M10BMC_DOORBELL,
- .bmc_prog_addr = BMC_PROG_ADDR,
- .bmc_reh_addr = BMC_REH_ADDR,
- .bmc_magic = BMC_PROG_MAGIC,
- .sr_prog_addr = SR_PROG_ADDR,
- .sr_reh_addr = SR_REH_ADDR,
- .sr_magic = SR_PROG_MAGIC,
- .pr_prog_addr = PR_PROG_ADDR,
- .pr_reh_addr = PR_REH_ADDR,
- .pr_magic = PR_PROG_MAGIC,
- .rsu_update_counter = STAGING_FLASH_COUNT,
+ .mac_low = M10BMC_N3000_MAC_LOW,
+ .mac_high = M10BMC_N3000_MAC_HIGH,
+ .doorbell = M10BMC_N3000_DOORBELL,
+ .auth_result = M10BMC_N3000_AUTH_RESULT,
+ .rsu_status = M10BMC_N3000_DOORBELL,
+ .bmc_prog_addr = M10BMC_N3000_BMC_PROG_ADDR,
+ .bmc_reh_addr = M10BMC_N3000_BMC_REH_ADDR,
+ .bmc_magic = M10BMC_N3000_BMC_PROG_MAGIC,
+ .sr_prog_addr = M10BMC_N3000_SR_PROG_ADDR,
+ .sr_reh_addr = M10BMC_N3000_SR_REH_ADDR,
+ .sr_magic = M10BMC_N3000_SR_PROG_MAGIC,
+ .pr_prog_addr = M10BMC_N3000_PR_PROG_ADDR,
+ .pr_reh_addr = M10BMC_N3000_PR_REH_ADDR,
+ .pr_magic = M10BMC_N3000_PR_PROG_MAGIC,
+ .rsu_update_counter = M10BMC_N3000_STAGING_FLASH_COUNT,
};
static struct mfd_cell m10bmc_d5005_subdevs[] = {
diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
index a009dd82f698..42e2ce7fe439 100644
--- a/include/linux/mfd/intel-m10-bmc.h
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -9,39 +9,9 @@
#include <linux/regmap.h>
-#define M10BMC_LEGACY_BUILD_VER 0x300468
-#define M10BMC_SYS_BASE 0x300800
-#define M10BMC_SYS_END 0x300fff
-#define M10BMC_FLASH_BASE 0x10000000
-#define M10BMC_FLASH_END 0x1fffffff
-#define M10BMC_MEM_END M10BMC_FLASH_END
-
#define M10BMC_STAGING_BASE 0x18000000
#define M10BMC_STAGING_SIZE 0x3800000
-/* Register offset of system registers */
-#define NIOS2_FW_VERSION 0x0
-#define M10BMC_MAC_LOW 0x10
-#define M10BMC_MAC_BYTE4 GENMASK(7, 0)
-#define M10BMC_MAC_BYTE3 GENMASK(15, 8)
-#define M10BMC_MAC_BYTE2 GENMASK(23, 16)
-#define M10BMC_MAC_BYTE1 GENMASK(31, 24)
-#define M10BMC_MAC_HIGH 0x14
-#define M10BMC_MAC_BYTE6 GENMASK(7, 0)
-#define M10BMC_MAC_BYTE5 GENMASK(15, 8)
-#define M10BMC_MAC_COUNT GENMASK(23, 16)
-#define M10BMC_TEST_REG 0x3c
-#define M10BMC_BUILD_VER 0x68
-#define M10BMC_VER_MAJOR_MSK GENMASK(23, 16)
-#define M10BMC_VER_PCB_INFO_MSK GENMASK(31, 24)
-#define M10BMC_VER_LEGACY_INVALID 0xffffffff
-
-/* Secure update doorbell register, in system register region */
-#define M10BMC_DOORBELL 0x400
-
-/* Authorization Result register, in system register region */
-#define M10BMC_AUTH_RESULT 0x404
-
/* Doorbell register fields */
#define DRBL_RSU_REQUEST BIT(0)
#define DRBL_RSU_PROGRESS GENMASK(7, 4)
@@ -102,22 +72,6 @@
#define RSU_COMPLETE_INTERVAL_MS 1000
#define RSU_COMPLETE_TIMEOUT_MS (40 * 60 * 1000)
-/* Addresses for security related data in FLASH */
-#define BMC_REH_ADDR 0x17ffc004
-#define BMC_PROG_ADDR 0x17ffc000
-#define BMC_PROG_MAGIC 0x5746
-
-#define SR_REH_ADDR 0x17ffd004
-#define SR_PROG_ADDR 0x17ffd000
-#define SR_PROG_MAGIC 0x5253
-
-#define PR_REH_ADDR 0x17ffe004
-#define PR_PROG_ADDR 0x17ffe000
-#define PR_PROG_MAGIC 0x5250
-
-/* Address of 4KB inverted bit vector containing staging area FLASH count */
-#define STAGING_FLASH_COUNT 0x17ffb000
-
/**
* struct m10bmc_csr_map - Intel MAX 10 BMC CSR register map
*/
--
2.30.2
On Mon, 26 Dec 2022, Ilpo Järvinen wrote:
> Move SPI based board definitions to per interface file from the global
> header. This makes it harder to use them accidently in the
> generic/interface agnostic code. Prefix the defines with M10BMC_N3000
> to make it more obvious these are related to some board type. All
> current non-N3000 board types have the same layout so they'll be
> reused.
>
> Some bitfield defs are also moved to intel-m10-bmc-core which seems
> more appropriate for them.
>
> Reviewed-by: Russ Weight <[email protected]>
> Reviewed-by: Xu Yilun <[email protected]>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
> drivers/mfd/intel-m10-bmc-core.c | 11 ++++
> drivers/mfd/intel-m10-bmc-spi.c | 89 ++++++++++++++++++++++---------
> include/linux/mfd/intel-m10-bmc.h | 46 ----------------
> 3 files changed, 74 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/mfd/intel-m10-bmc-core.c b/drivers/mfd/intel-m10-bmc-core.c
> index 51b78b868235..50a4ec758bdb 100644
> --- a/drivers/mfd/intel-m10-bmc-core.c
> +++ b/drivers/mfd/intel-m10-bmc-core.c
> @@ -12,6 +12,17 @@
> #include <linux/mfd/intel-m10-bmc.h>
> #include <linux/module.h>
>
> +/* Register fields of system registers */
> +#define M10BMC_MAC_BYTE4 GENMASK(7, 0)
> +#define M10BMC_MAC_BYTE3 GENMASK(15, 8)
> +#define M10BMC_MAC_BYTE2 GENMASK(23, 16)
> +#define M10BMC_MAC_BYTE1 GENMASK(31, 24)
> +#define M10BMC_MAC_BYTE6 GENMASK(7, 0)
> +#define M10BMC_MAC_BYTE5 GENMASK(15, 8)
> +#define M10BMC_MAC_COUNT GENMASK(23, 16)
> +#define M10BMC_VER_MAJOR_MSK GENMASK(23, 16)
> +#define M10BMC_VER_PCB_INFO_MSK GENMASK(31, 24)
> +
> static ssize_t bmc_version_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> diff --git a/drivers/mfd/intel-m10-bmc-spi.c b/drivers/mfd/intel-m10-bmc-spi.c
> index e8986154e965..04c83f9c6492 100644
> --- a/drivers/mfd/intel-m10-bmc-spi.c
> +++ b/drivers/mfd/intel-m10-bmc-spi.c
> @@ -13,10 +13,47 @@
> #include <linux/regmap.h>
> #include <linux/spi/spi.h>
>
> +#define M10BMC_N3000_LEGACY_BUILD_VER 0x300468
> +#define M10BMC_N3000_SYS_BASE 0x300800
> +#define M10BMC_N3000_SYS_END 0x300fff
> +#define M10BMC_N3000_FLASH_BASE 0x10000000
> +#define M10BMC_N3000_FLASH_END 0x1fffffff
> +#define M10BMC_N3000_MEM_END M10BMC_N3000_FLASH_END
> +
> +/* Register offset of system registers */
> +#define NIOS2_FW_VERSION 0x0
> +#define M10BMC_N3000_MAC_LOW 0x10
> +#define M10BMC_N3000_MAC_HIGH 0x14
> +#define M10BMC_N3000_TEST_REG 0x3c
> +#define M10BMC_N3000_BUILD_VER 0x68
> +#define M10BMC_N3000_VER_LEGACY_INVALID 0xffffffff
> +
> +/* Secure update doorbell register, in system register region */
> +#define M10BMC_N3000_DOORBELL 0x400
> +
> +/* Authorization Result register, in system register region */
> +#define M10BMC_N3000_AUTH_RESULT 0x404
> +
> +/* Addresses for security related data in FLASH */
> +#define M10BMC_N3000_BMC_REH_ADDR 0x17ffc004
> +#define M10BMC_N3000_BMC_PROG_ADDR 0x17ffc000
> +#define M10BMC_N3000_BMC_PROG_MAGIC 0x5746
> +
> +#define M10BMC_N3000_SR_REH_ADDR 0x17ffd004
> +#define M10BMC_N3000_SR_PROG_ADDR 0x17ffd000
> +#define M10BMC_N3000_SR_PROG_MAGIC 0x5253
> +
> +#define M10BMC_N3000_PR_REH_ADDR 0x17ffe004
> +#define M10BMC_N3000_PR_PROG_ADDR 0x17ffe000
> +#define M10BMC_N3000_PR_PROG_MAGIC 0x5250
My preference would definitely be to keep these blocks of defines tucked
away inside a header file somewhere.
Premise of the change looks fine, however.
--
Lee Jones [李琼斯]
On Mon, 26 Dec 2022, Ilpo Järvinen wrote:
> Add the mfd driver for the Platform Management Component Interface
> (PMCI) based interface of Intel MAX10 BMC controller.
>
> PMCI is a software-visible interface, connected to card BMC which
> provided the basic functionality of read/write BMC register. The access
> to the register is done indirectly via a hardware controller/bridge
> that handles read/write/clear commands and acknowledgments for the
> commands.
>
> Previously, intel-m10-bmc provided sysfs under
> /sys/bus/spi/devices/... which is generalized in this change because
> not all MAX10 BMC appear under SPI anymore.
>
> Co-developed-by: Tianfei zhang <[email protected]>
> Signed-off-by: Tianfei zhang <[email protected]>
> Co-developed-by: Russ Weight <[email protected]>
> Signed-off-by: Russ Weight <[email protected]>
> Co-developed-by: Matthew Gerlach <[email protected]>
> Signed-off-by: Matthew Gerlach <[email protected]>
> Reviewed-by: Xu Yilun <[email protected]>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
> .../ABI/testing/sysfs-driver-intel-m10-bmc | 8 +-
> drivers/mfd/Kconfig | 12 +
> drivers/mfd/Makefile | 1 +
> drivers/mfd/intel-m10-bmc-pmci.c | 253 ++++++++++++++++++
> 4 files changed, 270 insertions(+), 4 deletions(-)
> create mode 100644 drivers/mfd/intel-m10-bmc-pmci.c
>
> diff --git a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
> index 9773925138af..a8ab58035c95 100644
> --- a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
> +++ b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
> @@ -1,4 +1,4 @@
> -What: /sys/bus/spi/devices/.../bmc_version
> +What: /sys/bus/.../drivers/intel-m10-bmc/.../bmc_version
> Date: June 2020
> KernelVersion: 5.10
> Contact: Xu Yilun <[email protected]>
> @@ -6,7 +6,7 @@ Description: Read only. Returns the hardware build version of Intel
> MAX10 BMC chip.
> Format: "0x%x".
>
> -What: /sys/bus/spi/devices/.../bmcfw_version
> +What: /sys/bus/.../drivers/intel-m10-bmc/.../bmcfw_version
> Date: June 2020
> KernelVersion: 5.10
> Contact: Xu Yilun <[email protected]>
> @@ -14,7 +14,7 @@ Description: Read only. Returns the firmware version of Intel MAX10
> BMC chip.
> Format: "0x%x".
>
> -What: /sys/bus/spi/devices/.../mac_address
> +What: /sys/bus/.../drivers/intel-m10-bmc/.../mac_address
> Date: January 2021
> KernelVersion: 5.12
> Contact: Russ Weight <[email protected]>
> @@ -25,7 +25,7 @@ Description: Read only. Returns the first MAC address in a block
> space.
> Format: "%02x:%02x:%02x:%02x:%02x:%02x".
>
> -What: /sys/bus/spi/devices/.../mac_count
> +What: /sys/bus/.../drivers/intel-m10-bmc/.../mac_count
> Date: January 2021
> KernelVersion: 5.12
> Contact: Russ Weight <[email protected]>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index a09d4ac60dc7..82f13614d98a 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2238,6 +2238,18 @@ config MFD_INTEL_M10_BMC_SPI
> 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
> + select MFD_INTEL_M10_BMC_CORE
> + select REGMAP
> + 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 5d1f308ee2a7..c90fb96cad2a 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -274,6 +274,7 @@ obj-$(CONFIG_MFD_SIMPLE_MFD_I2C) += simple-mfd-i2c.o
>
> obj-$(CONFIG_MFD_INTEL_M10_BMC_CORE) += intel-m10-bmc-core.o
> obj-$(CONFIG_MFD_INTEL_M10_BMC_SPI) += intel-m10-bmc-spi.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..63ec0f6aba8b
> --- /dev/null
> +++ b/drivers/mfd/intel-m10-bmc-pmci.c
> @@ -0,0 +1,253 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * MFD driver for Platform Management Component Interface (PMCI) based
Please remove any mention of MFD.
This includes the MODULE_*() macros in all patches please.
Better to use terms like 'core' instead.
> + * interface to MAX10 BMC.
> + *
> + * Copyright (C) 2020-2022 Intel Corporation.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/dfl.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/intel-m10-bmc.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +
> +#define M10BMC_PMCI_INDIRECT_BASE 0x400
> +
> +#define M10BMC_N6000_SYS_BASE 0x0
> +#define M10BMC_N6000_SYS_END 0xfff
> +
> +#define M10BMC_N6000_DOORBELL 0x1c0
> +#define M10BMC_N6000_AUTH_RESULT 0x1c4
> +
> +/* Telemetry registers */
> +#define M10BMC_N6000_TELEM_START 0x400
> +#define M10BMC_N6000_TELEM_END 0x78c
> +
> +#define M10BMC_N6000_BUILD_VER 0x0
> +#define NIOS2_N6000_FW_VERSION 0x4
> +#define M10BMC_N6000_MAC_LOW 0x20
> +#define M10BMC_N6000_MAC_HIGH (M10BMC_N6000_MAC_LOW + 4)
> +
> +/* Addresses for security related data in FLASH */
> +#define M10BMC_N6000_BMC_REH_ADDR 0x7ffc004
> +#define M10BMC_N6000_BMC_PROG_ADDR 0x7ffc000
> +#define M10BMC_N6000_BMC_PROG_MAGIC 0x5746
> +
> +#define M10BMC_N6000_SR_REH_ADDR 0x7ffd004
> +#define M10BMC_N6000_SR_PROG_ADDR 0x7ffd000
> +#define M10BMC_N6000_SR_PROG_MAGIC 0x5253
> +
> +#define M10BMC_N6000_PR_REH_ADDR 0x7ffe004
> +#define M10BMC_N6000_PR_PROG_ADDR 0x7ffe000
> +#define M10BMC_N6000_PR_PROG_MAGIC 0x5250
> +
> +#define M10BMC_N6000_STAGING_FLASH_COUNT 0x7ff5000
> +
> +struct m10bmc_pmci_device {
> + void __iomem *base;
> + struct intel_m10bmc m10bmc;
> +};
> +
> +/*
> + * Intel FGPA indirect register access via hardware controller/bridge.
> + */
> +#define INDIRECT_CMD_OFF 0
> +#define INDIRECT_CMD_CLR 0
> +#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
> +
> +#define INDIRECT_INT_US 1
> +#define INDIRECT_TIMEOUT_US 10000
> +
> +struct indirect_ctx {
> + void __iomem *base;
> + struct device *dev;
> +};
> +
> +static int indirect_clear_cmd(struct indirect_ctx *ctx)
> +{
> + unsigned int cmd;
> + int ret;
> +
> + writel(INDIRECT_CMD_CLR, ctx->base + INDIRECT_CMD_OFF);
> +
> + ret = readl_poll_timeout(ctx->base + INDIRECT_CMD_OFF, cmd,
> + cmd == INDIRECT_CMD_CLR,
> + INDIRECT_INT_US, INDIRECT_TIMEOUT_US);
> + if (ret)
> + dev_err(ctx->dev, "timed out waiting clear cmd (residual cmd=0x%x)\n", cmd);
> +
> + return ret;
> +}
> +
> +static int indirect_reg_read(void *context, unsigned int reg, unsigned int *val)
> +{
> + struct indirect_ctx *ctx = context;
> + unsigned int cmd, ack, tmpval;
> + int ret;
> +
> + cmd = readl(ctx->base + INDIRECT_CMD_OFF);
> + if (cmd != INDIRECT_CMD_CLR)
> + dev_warn(ctx->dev, "residual cmd 0x%x on read entry\n", 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, ack,
> + (ack & INDIRECT_CMD_ACK) == INDIRECT_CMD_ACK,
> + INDIRECT_INT_US, INDIRECT_TIMEOUT_US);
> + if (ret)
> + dev_err(ctx->dev, "read timed out on reg 0x%x ack 0x%x\n", reg, ack);
> + else
> + tmpval = readl(ctx->base + INDIRECT_RD_OFF);
> +
> + if (indirect_clear_cmd(ctx))
What are the chances of the above readl_poll_timeout() failing, then the
subsequent one in indirect_clear_cmd() passing?
As I rule, I tend to dislike calling functions from inside if()
statements. If there is no way to work this hunk around, probably
better to place the return value inside another variable or use another
variable entirely and call indirect_clear_cmd() in the traditional way,
like you do everywhere else.
> + if (!ret)
> + ret = -ETIMEDOUT;
> + goto out;
> + }
> +
> + *val = tmpval;
If readl_poll_timeout() fails and indirect_clear_cmd() succeeds, you'll
return, at best, junk.
> +out:
> + return ret;
> +}
> +
> +static int indirect_reg_write(void *context, unsigned int reg, unsigned int val)
> +{
> + struct indirect_ctx *ctx = context;
> + unsigned int cmd, ack;
> + int ret;
> +
> + cmd = readl(ctx->base + INDIRECT_CMD_OFF);
> + if (cmd != INDIRECT_CMD_CLR)
> + dev_warn(ctx->dev, "residual cmd 0x%x on write entry\n", 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, ack,
> + (ack & INDIRECT_CMD_ACK) == INDIRECT_CMD_ACK,
> + INDIRECT_INT_US, INDIRECT_TIMEOUT_US);
> + if (ret)
> + dev_err(ctx->dev, "write timed out on reg 0x%x ack 0x%x\n", reg, ack);
> +
> + if (indirect_clear_cmd(ctx)) {
> + if (!ret)
> + ret = -ETIMEDOUT;
> + }
> +
> + return ret;
> +}
> +
> +static const struct regmap_range m10bmc_pmci_regmap_range[] = {
> + regmap_reg_range(M10BMC_N6000_SYS_BASE, M10BMC_N6000_SYS_END),
> +};
> +
> +static const struct regmap_access_table m10bmc_pmci_access_table = {
> + .yes_ranges = m10bmc_pmci_regmap_range,
> + .n_yes_ranges = ARRAY_SIZE(m10bmc_pmci_regmap_range),
> +};
> +
> +static struct regmap_config m10bmc_pmci_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .wr_table = &m10bmc_pmci_access_table,
> + .rd_table = &m10bmc_pmci_access_table,
> + .reg_read = &indirect_reg_read,
> + .reg_write = &indirect_reg_write,
> + .max_register = M10BMC_N6000_SYS_END,
> +};
> +
> +static struct mfd_cell m10bmc_pmci_n6000_bmc_subdevs[] = {
> + { .name = "n6000bmc-hwmon" },
> +};
> +
> +static const struct m10bmc_csr_map m10bmc_n6000_csr_map = {
> + .base = M10BMC_N6000_SYS_BASE,
> + .build_version = M10BMC_N6000_BUILD_VER,
> + .fw_version = NIOS2_N6000_FW_VERSION,
> + .mac_low = M10BMC_N6000_MAC_LOW,
> + .mac_high = M10BMC_N6000_MAC_HIGH,
> + .doorbell = M10BMC_N6000_DOORBELL,
> + .auth_result = M10BMC_N6000_AUTH_RESULT,
> + .rsu_status = M10BMC_N6000_AUTH_RESULT,
> + .bmc_prog_addr = M10BMC_N6000_BMC_PROG_ADDR,
> + .bmc_reh_addr = M10BMC_N6000_BMC_REH_ADDR,
> + .bmc_magic = M10BMC_N6000_BMC_PROG_MAGIC,
> + .sr_prog_addr = M10BMC_N6000_SR_PROG_ADDR,
> + .sr_reh_addr = M10BMC_N6000_SR_REH_ADDR,
> + .sr_magic = M10BMC_N6000_SR_PROG_MAGIC,
> + .pr_prog_addr = M10BMC_N6000_PR_PROG_ADDR,
> + .pr_reh_addr = M10BMC_N6000_PR_REH_ADDR,
> + .pr_magic = M10BMC_N6000_PR_PROG_MAGIC,
> + .rsu_update_counter = M10BMC_N6000_STAGING_FLASH_COUNT,
> +};
> +
> +static const struct intel_m10bmc_platform_info m10bmc_pmci_n6000 = {
> + .cells = m10bmc_pmci_n6000_bmc_subdevs,
> + .n_cells = ARRAY_SIZE(m10bmc_pmci_n6000_bmc_subdevs),
> + .csr_map = &m10bmc_n6000_csr_map,
> +};
> +
> +static int m10bmc_pmci_probe(struct dfl_device *ddev)
> +{
> + struct device *dev = &ddev->dev;
> + struct m10bmc_pmci_device *pmci;
> + struct indirect_ctx *ctx;
> +
> + pmci = devm_kzalloc(dev, sizeof(*pmci), GFP_KERNEL);
> + if (!pmci)
> + return -ENOMEM;
> +
> + pmci->m10bmc.dev = dev;
> +
> + pmci->base = devm_ioremap_resource(dev, &ddev->mmio_res);
> + if (IS_ERR(pmci->base))
> + return PTR_ERR(pmci->base);
> +
> + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + ctx->base = pmci->base + M10BMC_PMCI_INDIRECT_BASE;
> + ctx->dev = dev;
Why do we need to cache dev twice in two different structures?
> + indirect_clear_cmd(ctx);
> + pmci->m10bmc.regmap = devm_regmap_init(dev, NULL, ctx, &m10bmc_pmci_regmap_config);
> +
> + if (IS_ERR(pmci->m10bmc.regmap))
> + return PTR_ERR(pmci->m10bmc.regmap);
> +
> + return m10bmc_dev_init(&pmci->m10bmc, &m10bmc_pmci_n6000);
> +}
> +
> +#define FME_FEATURE_ID_M10BMC_PMCI 0x12
> +
> +static const struct dfl_device_id m10bmc_pmci_ids[] = {
> + { FME_ID, FME_FEATURE_ID_M10BMC_PMCI },
> + { }
> +};
> +MODULE_DEVICE_TABLE(dfl, m10bmc_pmci_ids);
> +
> +static struct dfl_driver m10bmc_pmci_driver = {
> + .drv = {
> + .name = "intel-m10-bmc",
> + .dev_groups = m10bmc_dev_groups,
> + },
> + .id_table = m10bmc_pmci_ids,
> + .probe = m10bmc_pmci_probe,
b> +};
> +
> +module_dfl_driver(m10bmc_pmci_driver);
> +
> +MODULE_DESCRIPTION("MAX10 BMC PMCI-based interface");
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL");
> --
> 2.30.2
>
--
Lee Jones [李琼斯]
On Fri, 13 Jan 2023, Lee Jones wrote:
> On Mon, 26 Dec 2022, Ilpo Järvinen wrote:
>
> > Add the mfd driver for the Platform Management Component Interface
> > (PMCI) based interface of Intel MAX10 BMC controller.
> >
> > PMCI is a software-visible interface, connected to card BMC which
> > provided the basic functionality of read/write BMC register. The access
> > to the register is done indirectly via a hardware controller/bridge
> > that handles read/write/clear commands and acknowledgments for the
> > commands.
> >
> > Previously, intel-m10-bmc provided sysfs under
> > /sys/bus/spi/devices/... which is generalized in this change because
> > not all MAX10 BMC appear under SPI anymore.
> >
> > Co-developed-by: Tianfei zhang <[email protected]>
> > Signed-off-by: Tianfei zhang <[email protected]>
> > Co-developed-by: Russ Weight <[email protected]>
> > Signed-off-by: Russ Weight <[email protected]>
> > Co-developed-by: Matthew Gerlach <[email protected]>
> > Signed-off-by: Matthew Gerlach <[email protected]>
> > Reviewed-by: Xu Yilun <[email protected]>
> > Signed-off-by: Ilpo Järvinen <[email protected]>
> > ---
> > .../ABI/testing/sysfs-driver-intel-m10-bmc | 8 +-
> > drivers/mfd/Kconfig | 12 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/intel-m10-bmc-pmci.c | 253 ++++++++++++++++++
> > 4 files changed, 270 insertions(+), 4 deletions(-)
> > create mode 100644 drivers/mfd/intel-m10-bmc-pmci.c
> >
> > diff --git a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
> > index 9773925138af..a8ab58035c95 100644
> > --- a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
> > +++ b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
> > @@ -1,4 +1,4 @@
> > -What: /sys/bus/spi/devices/.../bmc_version
> > +What: /sys/bus/.../drivers/intel-m10-bmc/.../bmc_version
> > Date: June 2020
> > KernelVersion: 5.10
> > Contact: Xu Yilun <[email protected]>
> > @@ -6,7 +6,7 @@ Description: Read only. Returns the hardware build version of Intel
> > MAX10 BMC chip.
> > Format: "0x%x".
> >
> > -What: /sys/bus/spi/devices/.../bmcfw_version
> > +What: /sys/bus/.../drivers/intel-m10-bmc/.../bmcfw_version
> > Date: June 2020
> > KernelVersion: 5.10
> > Contact: Xu Yilun <[email protected]>
> > @@ -14,7 +14,7 @@ Description: Read only. Returns the firmware version of Intel MAX10
> > BMC chip.
> > Format: "0x%x".
> >
> > -What: /sys/bus/spi/devices/.../mac_address
> > +What: /sys/bus/.../drivers/intel-m10-bmc/.../mac_address
> > Date: January 2021
> > KernelVersion: 5.12
> > Contact: Russ Weight <[email protected]>
> > @@ -25,7 +25,7 @@ Description: Read only. Returns the first MAC address in a block
> > space.
> > Format: "%02x:%02x:%02x:%02x:%02x:%02x".
> >
> > -What: /sys/bus/spi/devices/.../mac_count
> > +What: /sys/bus/.../drivers/intel-m10-bmc/.../mac_count
> > Date: January 2021
> > KernelVersion: 5.12
> > Contact: Russ Weight <[email protected]>
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index a09d4ac60dc7..82f13614d98a 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -2238,6 +2238,18 @@ config MFD_INTEL_M10_BMC_SPI
> > 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
> > + select MFD_INTEL_M10_BMC_CORE
> > + select REGMAP
> > + 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 5d1f308ee2a7..c90fb96cad2a 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -274,6 +274,7 @@ obj-$(CONFIG_MFD_SIMPLE_MFD_I2C) += simple-mfd-i2c.o
> >
> > obj-$(CONFIG_MFD_INTEL_M10_BMC_CORE) += intel-m10-bmc-core.o
> > obj-$(CONFIG_MFD_INTEL_M10_BMC_SPI) += intel-m10-bmc-spi.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..63ec0f6aba8b
> > --- /dev/null
> > +++ b/drivers/mfd/intel-m10-bmc-pmci.c
> > @@ -0,0 +1,253 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * MFD driver for Platform Management Component Interface (PMCI) based
>
> Please remove any mention of MFD.
>
> This includes the MODULE_*() macros in all patches please.
>
> Better to use terms like 'core' instead.
>
> > + * interface to MAX10 BMC.
> > + *
> > + * Copyright (C) 2020-2022 Intel Corporation.
> > + */
> > +
> > +#include <linux/device.h>
> > +#include <linux/dfl.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/mfd/intel-m10-bmc.h>
> > +#include <linux/module.h>
> > +#include <linux/regmap.h>
> > +
> > +#define M10BMC_PMCI_INDIRECT_BASE 0x400
> > +
> > +#define M10BMC_N6000_SYS_BASE 0x0
> > +#define M10BMC_N6000_SYS_END 0xfff
> > +
> > +#define M10BMC_N6000_DOORBELL 0x1c0
> > +#define M10BMC_N6000_AUTH_RESULT 0x1c4
> > +
> > +/* Telemetry registers */
> > +#define M10BMC_N6000_TELEM_START 0x400
> > +#define M10BMC_N6000_TELEM_END 0x78c
> > +
> > +#define M10BMC_N6000_BUILD_VER 0x0
> > +#define NIOS2_N6000_FW_VERSION 0x4
> > +#define M10BMC_N6000_MAC_LOW 0x20
> > +#define M10BMC_N6000_MAC_HIGH (M10BMC_N6000_MAC_LOW + 4)
> > +
> > +/* Addresses for security related data in FLASH */
> > +#define M10BMC_N6000_BMC_REH_ADDR 0x7ffc004
> > +#define M10BMC_N6000_BMC_PROG_ADDR 0x7ffc000
> > +#define M10BMC_N6000_BMC_PROG_MAGIC 0x5746
> > +
> > +#define M10BMC_N6000_SR_REH_ADDR 0x7ffd004
> > +#define M10BMC_N6000_SR_PROG_ADDR 0x7ffd000
> > +#define M10BMC_N6000_SR_PROG_MAGIC 0x5253
> > +
> > +#define M10BMC_N6000_PR_REH_ADDR 0x7ffe004
> > +#define M10BMC_N6000_PR_PROG_ADDR 0x7ffe000
> > +#define M10BMC_N6000_PR_PROG_MAGIC 0x5250
> > +
> > +#define M10BMC_N6000_STAGING_FLASH_COUNT 0x7ff5000
> > +
> > +struct m10bmc_pmci_device {
> > + void __iomem *base;
> > + struct intel_m10bmc m10bmc;
> > +};
> > +
> > +/*
> > + * Intel FGPA indirect register access via hardware controller/bridge.
> > + */
> > +#define INDIRECT_CMD_OFF 0
> > +#define INDIRECT_CMD_CLR 0
> > +#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
> > +
> > +#define INDIRECT_INT_US 1
> > +#define INDIRECT_TIMEOUT_US 10000
> > +
> > +struct indirect_ctx {
> > + void __iomem *base;
> > + struct device *dev;
> > +};
> > +
> > +static int indirect_clear_cmd(struct indirect_ctx *ctx)
> > +{
> > + unsigned int cmd;
> > + int ret;
> > +
> > + writel(INDIRECT_CMD_CLR, ctx->base + INDIRECT_CMD_OFF);
> > +
> > + ret = readl_poll_timeout(ctx->base + INDIRECT_CMD_OFF, cmd,
> > + cmd == INDIRECT_CMD_CLR,
> > + INDIRECT_INT_US, INDIRECT_TIMEOUT_US);
> > + if (ret)
> > + dev_err(ctx->dev, "timed out waiting clear cmd (residual cmd=0x%x)\n", cmd);
> > +
> > + return ret;
> > +}
> > +
> > +static int indirect_reg_read(void *context, unsigned int reg, unsigned int *val)
> > +{
> > + struct indirect_ctx *ctx = context;
> > + unsigned int cmd, ack, tmpval;
> > + int ret;
> > +
> > + cmd = readl(ctx->base + INDIRECT_CMD_OFF);
> > + if (cmd != INDIRECT_CMD_CLR)
> > + dev_warn(ctx->dev, "residual cmd 0x%x on read entry\n", 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, ack,
> > + (ack & INDIRECT_CMD_ACK) == INDIRECT_CMD_ACK,
> > + INDIRECT_INT_US, INDIRECT_TIMEOUT_US);
> > + if (ret)
> > + dev_err(ctx->dev, "read timed out on reg 0x%x ack 0x%x\n", reg, ack);
> > + else
> > + tmpval = readl(ctx->base + INDIRECT_RD_OFF);
> > +
> > + if (indirect_clear_cmd(ctx))
>
> What are the chances of the above readl_poll_timeout() failing, then the
> subsequent one in indirect_clear_cmd() passing?
I don't have a good answer to what the chances are but the clear command
deals with the indirect controller/bridge so failures aren't necessarily
1:1. At worst it just causes wait that is double of the poll timeout which
doesn't seem that big problem.
> As I rule, I tend to dislike calling functions from inside if()
> statements. If there is no way to work this hunk around, probably
> better to place the return value inside another variable or use another
> variable entirely and call indirect_clear_cmd() in the traditional way,
> like you do everywhere else.
I can add the second variable for that.
> > + if (!ret)
> > + ret = -ETIMEDOUT;
> > + goto out;
> > + }
> > +
> > + *val = tmpval;
>
> If readl_poll_timeout() fails and indirect_clear_cmd() succeeds, you'll
> return, at best, junk.
Thanks, I already fixed this problem once and now managed to reintroduce
it myself. :-(
> > +out:
> > + return ret;
> > +}
> > +
> > +static int indirect_reg_write(void *context, unsigned int reg, unsigned int val)
> > +{
> > + struct indirect_ctx *ctx = context;
> > + unsigned int cmd, ack;
> > + int ret;
> > +
> > + cmd = readl(ctx->base + INDIRECT_CMD_OFF);
> > + if (cmd != INDIRECT_CMD_CLR)
> > + dev_warn(ctx->dev, "residual cmd 0x%x on write entry\n", 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, ack,
> > + (ack & INDIRECT_CMD_ACK) == INDIRECT_CMD_ACK,
> > + INDIRECT_INT_US, INDIRECT_TIMEOUT_US);
> > + if (ret)
> > + dev_err(ctx->dev, "write timed out on reg 0x%x ack 0x%x\n", reg, ack);
> > +
> > + if (indirect_clear_cmd(ctx)) {
> > + if (!ret)
> > + ret = -ETIMEDOUT;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static const struct regmap_range m10bmc_pmci_regmap_range[] = {
> > + regmap_reg_range(M10BMC_N6000_SYS_BASE, M10BMC_N6000_SYS_END),
> > +};
> > +
> > +static const struct regmap_access_table m10bmc_pmci_access_table = {
> > + .yes_ranges = m10bmc_pmci_regmap_range,
> > + .n_yes_ranges = ARRAY_SIZE(m10bmc_pmci_regmap_range),
> > +};
> > +
> > +static struct regmap_config m10bmc_pmci_regmap_config = {
> > + .reg_bits = 32,
> > + .reg_stride = 4,
> > + .val_bits = 32,
> > + .wr_table = &m10bmc_pmci_access_table,
> > + .rd_table = &m10bmc_pmci_access_table,
> > + .reg_read = &indirect_reg_read,
> > + .reg_write = &indirect_reg_write,
> > + .max_register = M10BMC_N6000_SYS_END,
> > +};
> > +
> > +static struct mfd_cell m10bmc_pmci_n6000_bmc_subdevs[] = {
> > + { .name = "n6000bmc-hwmon" },
> > +};
> > +
> > +static const struct m10bmc_csr_map m10bmc_n6000_csr_map = {
> > + .base = M10BMC_N6000_SYS_BASE,
> > + .build_version = M10BMC_N6000_BUILD_VER,
> > + .fw_version = NIOS2_N6000_FW_VERSION,
> > + .mac_low = M10BMC_N6000_MAC_LOW,
> > + .mac_high = M10BMC_N6000_MAC_HIGH,
> > + .doorbell = M10BMC_N6000_DOORBELL,
> > + .auth_result = M10BMC_N6000_AUTH_RESULT,
> > + .rsu_status = M10BMC_N6000_AUTH_RESULT,
> > + .bmc_prog_addr = M10BMC_N6000_BMC_PROG_ADDR,
> > + .bmc_reh_addr = M10BMC_N6000_BMC_REH_ADDR,
> > + .bmc_magic = M10BMC_N6000_BMC_PROG_MAGIC,
> > + .sr_prog_addr = M10BMC_N6000_SR_PROG_ADDR,
> > + .sr_reh_addr = M10BMC_N6000_SR_REH_ADDR,
> > + .sr_magic = M10BMC_N6000_SR_PROG_MAGIC,
> > + .pr_prog_addr = M10BMC_N6000_PR_PROG_ADDR,
> > + .pr_reh_addr = M10BMC_N6000_PR_REH_ADDR,
> > + .pr_magic = M10BMC_N6000_PR_PROG_MAGIC,
> > + .rsu_update_counter = M10BMC_N6000_STAGING_FLASH_COUNT,
> > +};
> > +
> > +static const struct intel_m10bmc_platform_info m10bmc_pmci_n6000 = {
> > + .cells = m10bmc_pmci_n6000_bmc_subdevs,
> > + .n_cells = ARRAY_SIZE(m10bmc_pmci_n6000_bmc_subdevs),
> > + .csr_map = &m10bmc_n6000_csr_map,
> > +};
> > +
> > +static int m10bmc_pmci_probe(struct dfl_device *ddev)
> > +{
> > + struct device *dev = &ddev->dev;
> > + struct m10bmc_pmci_device *pmci;
> > + struct indirect_ctx *ctx;
> > +
> > + pmci = devm_kzalloc(dev, sizeof(*pmci), GFP_KERNEL);
> > + if (!pmci)
> > + return -ENOMEM;
> > +
> > + pmci->m10bmc.dev = dev;
> > +
> > + pmci->base = devm_ioremap_resource(dev, &ddev->mmio_res);
> > + if (IS_ERR(pmci->base))
> > + return PTR_ERR(pmci->base);
> > +
> > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> > + if (!ctx)
> > + return -ENOMEM;
> > +
> > + ctx->base = pmci->base + M10BMC_PMCI_INDIRECT_BASE;
> > + ctx->dev = dev;
>
> Why do we need to cache dev twice in two different structures?
This indirect register access thing is kinda different from this PMCI
driver. It's a generic mechanism to access registers in Intel FPGAs. The
indirect register access code used to be separately among regmap code in
the earlier versions of this series but since PMCI driver is currently the
only user and lacks in the genericness of the indirect code, it eventually
got included into this PMCI driver file.
Do you prefer I use m10bmc_pmci_device instead?
--
i.