Re: [PATCH 10/12] mfd: intel-m10-bmc: Add PMCI driver

From: Xu Yilun
Date: Fri Nov 11 2022 - 05:13:38 EST


On 2022-11-08 at 16:43:03 +0200, Ilpo Järvinen wrote:
> Adding an mfd driver for the PMCI-based interface of Intel MAX10 BMC
> controller.

Could you help describe what is PMCI? Also add the description somewhere
in code.

>
> Co-developed-by: Tianfei zhang <tianfei.zhang@xxxxxxxxx>
> Signed-off-by: Tianfei zhang <tianfei.zhang@xxxxxxxxx>
> Co-developed-by: Russ Weight <russell.h.weight@xxxxxxxxx>
> Signed-off-by: Russ Weight <russell.h.weight@xxxxxxxxx>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> ---
> .../ABI/testing/sysfs-driver-intel-m10-bmc | 8 +-
> drivers/mfd/Kconfig | 12 ++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/intel-m10-bmc-pmci.c | 154 ++++++++++++++++++
> include/linux/mfd/intel-m10-bmc.h | 1 +
> 5 files changed, 172 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

It's good. And could you add in commit message about why the changes to the ABI
description.

> Date: June 2020
> KernelVersion: 5.10
> Contact: Xu Yilun <yilun.xu@xxxxxxxxx>
> @@ -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 <yilun.xu@xxxxxxxxx>
> @@ -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 <russell.h.weight@xxxxxxxxx>
> @@ -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 <russell.h.weight@xxxxxxxxx>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index a09d4ac60dc7..38d53f6c4d7b 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_INDIRECT
> + 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 f32276cdd0c2..7559362cb438 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -275,6 +275,7 @@ obj-$(CONFIG_MFD_SIMPLE_MFD_I2C) += simple-mfd-i2c.o
> intel-m10-bmc-objs := intel-m10-bmc-core.o
> obj-$(CONFIG_MFD_INTEL_M10_BMC_CORE) += intel-m10-bmc.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..918378a78bdb
> --- /dev/null
> +++ b/drivers/mfd/intel-m10-bmc-pmci.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PMCI-based interface to MAX10 BMC
> + *
> + * Copyright (C) 2020-2022 Intel Corporation, Inc.
> + *
> + */
> +
> +#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_PMCI_SYS_BASE 0x0
> +#define M10BMC_PMCI_SYS_END 0xfff
> +
> +#define M10BMC_PMCI_DOORBELL 0x1c0
> +#define M10BMC_PMCI_AUTH_RESULT 0x1c4
> +
> +/* Telemetry registers */
> +#define M10BMC_PMCI_TELEM_START 0x400
> +#define M10BMC_PMCI_TELEM_END 0x78c
> +
> +#define M10BMC_PMCI_BUILD_VER 0x0
> +#define NIOS2_PMCI_FW_VERSION 0x4
> +#define M10BMC_PMCI_MAC_LOW 0x20
> +#define M10BMC_PMCI_MAC_HIGH (M10BMC_PMCI_MAC_LOW + 4)
> +
> +/* Addresses for security related data in FLASH */
> +#define PMCI_BMC_REH_ADDR 0x7ffc004
> +#define PMCI_BMC_PROG_ADDR 0x7ffc000
> +#define PMCI_BMC_PROG_MAGIC 0x5746
> +
> +#define PMCI_SR_REH_ADDR 0x7ffd004
> +#define PMCI_SR_PROG_ADDR 0x7ffd000
> +#define PMCI_SR_PROG_MAGIC 0x5253
> +
> +#define PMCI_PR_REH_ADDR 0x7ffe004
> +#define PMCI_PR_PROG_ADDR 0x7ffe000
> +#define PMCI_PR_PROG_MAGIC 0x5250
> +
> +#define PMCI_STAGING_FLASH_COUNT 0x7ff5000
> +
> +struct pmci_device {
> + void __iomem *base;
> + struct device *dev;
> + struct intel_m10bmc m10bmc;
> +};
> +
> +static const struct regmap_range m10bmc_pmci_regmap_range[] = {
> + regmap_reg_range(M10BMC_PMCI_SYS_BASE, M10BMC_PMCI_SYS_END),
> +};
> +
> +static const struct regmap_access_table m10_access_table = {
> + .yes_ranges = m10bmc_pmci_regmap_range,
> + .n_yes_ranges = ARRAY_SIZE(m10bmc_pmci_regmap_range),
> +};
> +
> +static const struct regmap_indirect_cfg indirect_cfg = {
> + INTEL_M10_REGMAP_INDIRECT_CFG,
> +};
> +
> +static struct regmap_config m10bmc_pmci_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .wr_table = &m10_access_table,
> + .rd_table = &m10_access_table,
> + .max_register = M10BMC_PMCI_SYS_END,
> + .indirect_cfg = &indirect_cfg,
> +};
> +
> +static struct mfd_cell m10bmc_n6000_bmc_subdevs[] = {
> + { .name = "n6000bmc-hwmon" },
> +};
> +
> +static const struct m10bmc_csr_map m10bmc_pmci_csr_map = {
> + .base = M10BMC_PMCI_SYS_BASE,
> + .build_version = M10BMC_PMCI_BUILD_VER,
> + .fw_version = NIOS2_PMCI_FW_VERSION,
> + .mac_low = M10BMC_PMCI_MAC_LOW,
> + .mac_high = M10BMC_PMCI_MAC_HIGH,
> + .doorbell = M10BMC_PMCI_DOORBELL,
> + .auth_result = M10BMC_PMCI_AUTH_RESULT,
> + .bmc_prog_addr = PMCI_BMC_PROG_ADDR,
> + .bmc_reh_addr = PMCI_BMC_REH_ADDR,
> + .bmc_magic = PMCI_BMC_PROG_MAGIC,
> + .sr_prog_addr = PMCI_SR_PROG_ADDR,
> + .sr_reh_addr = PMCI_SR_REH_ADDR,
> + .sr_magic = PMCI_SR_PROG_MAGIC,
> + .pr_prog_addr = PMCI_PR_PROG_ADDR,
> + .pr_reh_addr = PMCI_PR_REH_ADDR,
> + .pr_magic = PMCI_PR_PROG_MAGIC,
> + .rsu_update_counter = PMCI_STAGING_FLASH_COUNT,
> +};
> +
> +static const struct intel_m10bmc_platform_info m10bmc_m10_n6000 = {
> + .type = M10_N6000,
> + .cells = m10bmc_n6000_bmc_subdevs,
> + .n_cells = ARRAY_SIZE(m10bmc_n6000_bmc_subdevs),
> + .csr_map = &m10bmc_pmci_csr_map,
> +};
> +
> +static int pmci_probe(struct dfl_device *ddev)
> +{
> + struct device *dev = &ddev->dev;
> + struct pmci_device *pmci;
> +
> + pmci = devm_kzalloc(dev, sizeof(*pmci), GFP_KERNEL);
> + if (!pmci)
> + return -ENOMEM;
> +
> + pmci->m10bmc.dev = dev;
> + pmci->dev = dev;

I don't see its usage.

> +
> + pmci->base = devm_ioremap_resource(dev, &ddev->mmio_res);
> + if (IS_ERR(pmci->base))
> + return PTR_ERR(pmci->base);
> +
> + pmci->m10bmc.regmap =
> + devm_regmap_init_indirect(dev,
> + pmci->base + M10BMC_PMCI_INDIRECT_BASE,
> + &m10bmc_pmci_regmap_config);
> + if (IS_ERR(pmci->m10bmc.regmap))
> + return PTR_ERR(pmci->m10bmc.regmap);
> +
> + return m10bmc_dev_init(&pmci->m10bmc, &m10bmc_m10_n6000);
> +}
> +
> +#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 = "intel-m10-bmc",
> + .dev_groups = m10bmc_dev_groups,
> + },
> + .id_table = pmci_ids,
> + .probe = pmci_probe,
> +};
> +
> +module_dfl_driver(pmci_driver);

Maybe change the name. This is still a MAX10 bmc driver, pmci is just the
interface to max10.

Thanks,
Yilun

> +
> +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 1b907c1a176f..90f2a691ef46 100644
> --- a/include/linux/mfd/intel-m10-bmc.h
> +++ b/include/linux/mfd/intel-m10-bmc.h
> @@ -13,6 +13,7 @@ enum m10bmc_type {
> M10_N3000,
> M10_D5005,
> M10_N5010,
> + M10_N6000,
> };
>
> #define INTEL_M10_REGMAP_INDIRECT_CFG \
> --
> 2.30.2
>