[PATCH v2 00/11] intel-m10-bmc: Split BMC to core and SPI parts & add PMCI+N6000 support

[PATCH v2 00/11] intel-m10-bmc: Split BMC to core and SPI parts & add PMCI+N6000 support

[Ilpo Järvinen]

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. This series also introduced regmap for
indirect register access generic to Intel FPGA IPs.

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.

I'm not entirely sure if I did properly address Yilun's comment on
placement of the flash_ops related code. To me the original way which
keeps them in mfd/intel-m10-bmc-{spi,pmci}.c still seems to the best
way. If that is still not acceptable, I propose that I'll move just the
flash ops functions into intel-m10-bmc-sec-update-{spi,pmci}.c and
create sec related platform info struct to select the correct flash
ops. This would effectively be the "double split" approach I suggested
earlier (both mfd and sec have their own per interface splits, to me
the double split looks overkill but it would meet Yilun's goal of
keeping sec related code within the sec driver).

The patch set is based on top of commit dfd10332596e ("fpga:
m10bmc-sec: Fix kconfig dependencies") to avoid triggering a Kconfig
conflict.

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 (11):
  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: Add flash ops for sec update
  mfd: intel-m10-bmc: Downscope SPI defines & prefix with M10BMC_SPI
  regmap: indirect: Add indirect regmap support
  intel-m10-bmc: Add regmap_indirect_cfg for Intel FPGA IPs
  mfd: intel-m10-bmc: Add PMCI driver
  fpga: m10bmc-sec: Add support for N6000
  mfd: intel-m10-bmc: Change MODULE_LICENSE() to GPL

 .../ABI/testing/sysfs-driver-intel-m10-bmc    |   8 +-
 MAINTAINERS                                   |   2 +-
 drivers/base/regmap/Kconfig                   |   3 +
 drivers/base/regmap/Makefile                  |   1 +
 drivers/base/regmap/regmap-indirect.c         | 128 +++++++
 drivers/fpga/Kconfig                          |   2 +-
 drivers/fpga/intel-m10-bmc-sec-update.c       | 149 ++++----
 drivers/hwmon/Kconfig                         |   2 +-
 drivers/mfd/Kconfig                           |  34 +-
 drivers/mfd/Makefile                          |   6 +-
 drivers/mfd/intel-m10-bmc-core.c              | 133 +++++++
 drivers/mfd/intel-m10-bmc-pmci.c              | 361 ++++++++++++++++++
 drivers/mfd/intel-m10-bmc-spi.c               | 270 +++++++++++++
 drivers/mfd/intel-m10-bmc.c                   | 238 ------------
 include/linux/mfd/intel-m10-bmc.h             | 122 +++---
 include/linux/regmap.h                        |  55 +++
 16 files changed, 1131 insertions(+), 383 deletions(-)
 create mode 100644 drivers/base/regmap/regmap-indirect.c
 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

[PATCH v2 01/11] mfd: intel-m10-bmc: Create m10bmc_platform_info for type specific info

[Ilpo Järvinen]

BMC type specific info is currently set by a switch/case block. The
size of this info is expected to grow as more dev types and features
are added which would have made the switch block bloaty.

Store type specific info into struct and place them into .driver_data
instead because it makes things a bit cleaner.

The m10bmc_type enum can be dropped as the differentiation is now
fully handled by the platform info.

The info member of struct intel_m10bmc that is added here is not used
yet in this change but its addition logically still belongs to this
change. The CSR map change that comes after this change needs to have
the info member.

Reviewed-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/mfd/intel-m10-bmc.c       | 53 ++++++++++++++-----------------
 include/linux/mfd/intel-m10-bmc.h | 12 +++++++
 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc.c
index 7e3319e5b22f..12c522c16d83 100644
--- a/drivers/mfd/intel-m10-bmc.c
+++ b/drivers/mfd/intel-m10-bmc.c
@@ -13,12 +13,6 @@
 #include <linux/regmap.h>
 #include <linux/spi/spi.h>
 
-enum m10bmc_type {
-	M10_N3000,
-	M10_D5005,
-	M10_N5010,
-};
-
 static struct mfd_cell m10bmc_d5005_subdevs[] = {
 	{ .name = "d5005bmc-hwmon" },
 	{ .name = "d5005bmc-sec-update" }
@@ -162,15 +156,17 @@ static int check_m10bmc_version(struct intel_m10bmc *ddata)
 static int intel_m10_bmc_spi_probe(struct spi_device *spi)
 {
 	const struct spi_device_id *id = spi_get_device_id(spi);
+	const struct intel_m10bmc_platform_info *info;
 	struct device *dev = &spi->dev;
-	struct mfd_cell *cells;
 	struct intel_m10bmc *ddata;
-	int ret, n_cell;
+	int ret;
 
 	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
 	if (!ddata)
 		return -ENOMEM;
 
+	info = (struct intel_m10bmc_platform_info *)id->driver_data;
+	ddata->info = info;
 	ddata->dev = dev;
 
 	ddata->regmap =
@@ -189,24 +185,8 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	switch (id->driver_data) {
-	case M10_N3000:
-		cells = m10bmc_pacn3000_subdevs;
-		n_cell = ARRAY_SIZE(m10bmc_pacn3000_subdevs);
-		break;
-	case M10_D5005:
-		cells = m10bmc_d5005_subdevs;
-		n_cell = ARRAY_SIZE(m10bmc_d5005_subdevs);
-		break;
-	case M10_N5010:
-		cells = m10bmc_n5010_subdevs;
-		n_cell = ARRAY_SIZE(m10bmc_n5010_subdevs);
-		break;
-	default:
-		return -ENODEV;
-	}
-
-	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cells, n_cell,
+	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
+				   info->cells, info->n_cells,
 				   NULL, 0, NULL);
 	if (ret)
 		dev_err(dev, "Failed to register sub-devices: %d\n", ret);
@@ -214,10 +194,25 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
 	return ret;
 }
 
+static const struct intel_m10bmc_platform_info m10bmc_spi_n3000 = {
+	.cells = m10bmc_pacn3000_subdevs,
+	.n_cells = ARRAY_SIZE(m10bmc_pacn3000_subdevs),
+};
+
+static const struct intel_m10bmc_platform_info m10bmc_spi_d5005 = {
+	.cells = m10bmc_d5005_subdevs,
+	.n_cells = ARRAY_SIZE(m10bmc_d5005_subdevs),
+};
+
+static const struct intel_m10bmc_platform_info m10bmc_spi_n5010 = {
+	.cells = m10bmc_n5010_subdevs,
+	.n_cells = ARRAY_SIZE(m10bmc_n5010_subdevs),
+};
+
 static const struct spi_device_id m10bmc_spi_id[] = {
-	{ "m10-n3000", M10_N3000 },
-	{ "m10-d5005", M10_D5005 },
-	{ "m10-n5010", M10_N5010 },
+	{ "m10-n3000", (kernel_ulong_t)&m10bmc_spi_n3000 },
+	{ "m10-d5005", (kernel_ulong_t)&m10bmc_spi_d5005 },
+	{ "m10-n5010", (kernel_ulong_t)&m10bmc_spi_n5010 },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, m10bmc_spi_id);
diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
index f0044b14136e..725b51ea4aee 100644
--- a/include/linux/mfd/intel-m10-bmc.h
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -118,14 +118,26 @@
 /* Address of 4KB inverted bit vector containing staging area FLASH count */
 #define STAGING_FLASH_COUNT	0x17ffb000
 
+/**
+ * struct intel_m10bmc_platform_info - Intel MAX 10 BMC platform specific information
+ * @cells: MFD cells
+ * @n_cells: MFD cells ARRAY_SIZE()
+ */
+struct intel_m10bmc_platform_info {
+	struct mfd_cell *cells;
+	int n_cells;
+};
+
 /**
  * struct intel_m10bmc - Intel MAX 10 BMC parent driver data structure
  * @dev: this device
  * @regmap: the regmap used to access registers by m10bmc itself
+ * @info: the platform information for MAX10 BMC
  */
 struct intel_m10bmc {
 	struct device *dev;
 	struct regmap *regmap;
+	const struct intel_m10bmc_platform_info *info;
 };
 
 /*
-- 
2.30.2

[PATCH v2 02/11] mfd: intel-m10-bmc: Rename the local variables

[Ilpo Järvinen]

Local variables directly interact with dev_get_drvdata/dev_set_drvdata
should be named ddata.

Co-developed-by: Tianfei zhang <tianfei.zhang@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
Reviewed-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.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 12c522c16d83..2c26203c4799 100644
--- a/drivers/mfd/intel-m10-bmc.c
+++ b/drivers/mfd/intel-m10-bmc.c
@@ -81,15 +81,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;
 
@@ -106,11 +106,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.30.2

[PATCH v2 03/11] mfd: intel-m10-bmc: Split into core and spi specific parts

[Ilpo Järvinen]

Split the common code from intel-m10-bmc driver into intel-m10-bmc-core
and move the SPI bus parts into an interface specific file.

intel-m10-bmc-core becomes the core MFD functions which can support
multiple bus interface like SPI bus.

Co-developed-by: Tianfei zhang <tianfei.zhang@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
Reviewed-by: Russ Weight <russell.h.weight@intel.com>
Acked-by: Guenter Roeck <linux@roeck-us.net> # hwmon
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 MAINTAINERS                                   |   2 +-
 drivers/fpga/Kconfig                          |   2 +-
 drivers/hwmon/Kconfig                         |   2 +-
 drivers/mfd/Kconfig                           |  30 ++--
 drivers/mfd/Makefile                          |   5 +-
 drivers/mfd/intel-m10-bmc-core.c              | 122 +++++++++++++++++
 .../{intel-m10-bmc.c => intel-m10-bmc-spi.c}  | 128 +++---------------
 include/linux/mfd/intel-m10-bmc.h             |   6 +
 8 files changed, 173 insertions(+), 124 deletions(-)
 create mode 100644 drivers/mfd/intel-m10-bmc-core.c
 rename drivers/mfd/{intel-m10-bmc.c => intel-m10-bmc-spi.c} (59%)

diff --git a/MAINTAINERS b/MAINTAINERS
index cf0f18502372..ddfa4f8b3c80 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10452,7 +10452,7 @@ S:	Maintained
 F:	Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
 F:	Documentation/hwmon/intel-m10-bmc-hwmon.rst
 F:	drivers/hwmon/intel-m10-bmc-hwmon.c
-F:	drivers/mfd/intel-m10-bmc.c
+F:	drivers/mfd/intel-m10-bmc*
 F:	include/linux/mfd/intel-m10-bmc.h
 
 INTEL MENLOW THERMAL DRIVER
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 6ce143dafd04..0a00763b9f28 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -246,7 +246,7 @@ config FPGA_MGR_VERSAL_FPGA
 
 config FPGA_M10_BMC_SEC_UPDATE
 	tristate "Intel MAX10 BMC Secure Update driver"
-	depends on MFD_INTEL_M10_BMC
+	depends on MFD_INTEL_M10_BMC_CORE
 	select FW_LOADER
 	select FW_UPLOAD
 	help
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 7ac3daaf59ce..984a55e0f313 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2321,7 +2321,7 @@ config SENSORS_XGENE
 
 config SENSORS_INTEL_M10_BMC_HWMON
 	tristate "Intel MAX10 BMC Hardware Monitoring"
-	depends on MFD_INTEL_M10_BMC
+	depends on MFD_INTEL_M10_BMC_CORE
 	help
 	  This driver provides support for the hardware monitoring functionality
 	  on Intel MAX10 BMC chip.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 8b93856de432..a09d4ac60dc7 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2219,18 +2219,24 @@ config SGI_MFD_IOC3
 	  If you have an SGI Origin, Octane, or a PCI IOC3 card,
 	  then say Y. Otherwise say N.
 
-config MFD_INTEL_M10_BMC
-	tristate "Intel MAX 10 Board Management Controller"
-	depends on SPI_MASTER
-	select REGMAP_SPI_AVMM
-	select MFD_CORE
-	help
-	  Support for the Intel MAX 10 board management controller using the
-	  SPI interface.
-
-	  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_INTEL_M10_BMC_CORE
+        tristate
+        select MFD_CORE
+        select REGMAP
+        default n
+
+config MFD_INTEL_M10_BMC_SPI
+        tristate "Intel MAX 10 Board Management Controller with SPI"
+        depends on SPI_MASTER
+        select MFD_INTEL_M10_BMC_CORE
+        select REGMAP_SPI_AVMM
+        help
+          Support for the Intel MAX 10 board management controller using the
+          SPI interface.
+
+          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"
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 7ed3ef4a698c..f32276cdd0c2 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -271,7 +271,10 @@ 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
+
+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_ATC260X)	+= atc260x-core.o
 obj-$(CONFIG_MFD_ATC260X_I2C)	+= atc260x-i2c.o
diff --git a/drivers/mfd/intel-m10-bmc-core.c b/drivers/mfd/intel-m10-bmc-core.c
new file mode 100644
index 000000000000..6630b81b10c4
--- /dev/null
+++ b/drivers/mfd/intel-m10-bmc-core.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Intel MAX 10 Board Management Controller chip - common code
+ *
+ * Copyright (C) 2018-2020 Intel Corporation. All rights reserved.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/dev_printk.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/intel-m10-bmc.h>
+#include <linux/module.h>
+
+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_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_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 *ddata = dev_get_drvdata(dev);
+	unsigned int macaddr_low, macaddr_high;
+	int ret;
+
+	ret = m10bmc_sys_read(ddata, M10BMC_MAC_LOW, &macaddr_low);
+	if (ret)
+		return ret;
+
+	ret = m10bmc_sys_read(ddata, M10BMC_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 *ddata = dev_get_drvdata(dev);
+	unsigned int macaddr_high;
+	int ret;
+
+	ret = m10bmc_sys_read(ddata, M10BMC_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,
+};
+
+static const struct attribute_group m10bmc_group = {
+	.attrs = m10bmc_attrs,
+};
+
+const struct attribute_group *m10bmc_dev_groups[] = {
+	&m10bmc_group,
+	NULL,
+};
+EXPORT_SYMBOL_GPL(m10bmc_dev_groups);
+
+int m10bmc_dev_init(struct intel_m10bmc *m10bmc, const struct intel_m10bmc_platform_info *info)
+{
+	int ret;
+
+	m10bmc->info = info;
+	dev_set_drvdata(m10bmc->dev, m10bmc);
+
+	ret = devm_mfd_add_devices(m10bmc->dev, PLATFORM_DEVID_AUTO,
+				   info->cells, info->n_cells,
+				   NULL, 0, NULL);
+	if (ret)
+		dev_err(m10bmc->dev, "Failed to register sub-devices: %d\n", ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(m10bmc_dev_init);
+
+MODULE_DESCRIPTION("Intel MAX 10 BMC core MFD driver");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc-spi.c
similarity index 59%
rename from drivers/mfd/intel-m10-bmc.c
rename to drivers/mfd/intel-m10-bmc-spi.c
index 2c26203c4799..be1d4ddedabb 100644
--- a/drivers/mfd/intel-m10-bmc.c
+++ b/drivers/mfd/intel-m10-bmc-spi.c
@@ -5,29 +5,14 @@
  * Copyright (C) 2018-2020 Intel Corporation. All rights reserved.
  */
 #include <linux/bitfield.h>
+#include <linux/dev_printk.h>
 #include <linux/init.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/intel-m10-bmc.h>
 #include <linux/module.h>
-#include <linux/mutex.h>
 #include <linux/regmap.h>
 #include <linux/spi/spi.h>
 
-static struct mfd_cell m10bmc_d5005_subdevs[] = {
-	{ .name = "d5005bmc-hwmon" },
-	{ .name = "d5005bmc-sec-update" }
-};
-
-static struct mfd_cell m10bmc_pacn3000_subdevs[] = {
-	{ .name = "n3000bmc-hwmon" },
-	{ .name = "n3000bmc-retimer" },
-	{ .name = "n3000bmc-sec-update" },
-};
-
-static struct mfd_cell m10bmc_n5010_subdevs[] = {
-	{ .name = "n5010bmc-hwmon" },
-};
-
 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),
@@ -48,86 +33,6 @@ static struct regmap_config intel_m10bmc_regmap_config = {
 	.max_register = M10BMC_MEM_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_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_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 *ddata = dev_get_drvdata(dev);
-	unsigned int macaddr_low, macaddr_high;
-	int ret;
-
-	ret = m10bmc_sys_read(ddata, M10BMC_MAC_LOW, &macaddr_low);
-	if (ret)
-		return ret;
-
-	ret = m10bmc_sys_read(ddata, M10BMC_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 *ddata = dev_get_drvdata(dev);
-	unsigned int macaddr_high;
-	int ret;
-
-	ret = m10bmc_sys_read(ddata, M10BMC_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 check_m10bmc_version(struct intel_m10bmc *ddata)
 {
 	unsigned int v;
@@ -166,11 +71,9 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
 		return -ENOMEM;
 
 	info = (struct intel_m10bmc_platform_info *)id->driver_data;
-	ddata->info = info;
 	ddata->dev = dev;
 
-	ddata->regmap =
-		devm_regmap_init_spi_avmm(spi, &intel_m10bmc_regmap_config);
+	ddata->regmap = devm_regmap_init_spi_avmm(spi, &intel_m10bmc_regmap_config);
 	if (IS_ERR(ddata->regmap)) {
 		ret = PTR_ERR(ddata->regmap);
 		dev_err(dev, "Failed to allocate regmap: %d\n", ret);
@@ -185,15 +88,24 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
 		return ret;
 	}
 
-	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
-				   info->cells, info->n_cells,
-				   NULL, 0, NULL);
-	if (ret)
-		dev_err(dev, "Failed to register sub-devices: %d\n", ret);
-
-	return ret;
+	return m10bmc_dev_init(ddata, info);
 }
 
+static struct mfd_cell m10bmc_d5005_subdevs[] = {
+	{ .name = "d5005bmc-hwmon" },
+	{ .name = "d5005bmc-sec-update" },
+};
+
+static struct mfd_cell m10bmc_pacn3000_subdevs[] = {
+	{ .name = "n3000bmc-hwmon" },
+	{ .name = "n3000bmc-retimer" },
+	{ .name = "n3000bmc-sec-update" },
+};
+
+static struct mfd_cell m10bmc_n5010_subdevs[] = {
+	{ .name = "n5010bmc-hwmon" },
+};
+
 static const struct intel_m10bmc_platform_info m10bmc_spi_n3000 = {
 	.cells = m10bmc_pacn3000_subdevs,
 	.n_cells = ARRAY_SIZE(m10bmc_pacn3000_subdevs),
@@ -220,14 +132,14 @@ MODULE_DEVICE_TABLE(spi, m10bmc_spi_id);
 static struct spi_driver intel_m10bmc_spi_driver = {
 	.driver = {
 		.name = "intel-m10-bmc",
-		.dev_groups = m10bmc_groups,
+		.dev_groups = m10bmc_dev_groups,
 	},
 	.probe = intel_m10_bmc_spi_probe,
 	.id_table = m10bmc_spi_id,
 };
 module_spi_driver(intel_m10bmc_spi_driver);
 
-MODULE_DESCRIPTION("Intel MAX 10 BMC Device Driver");
+MODULE_DESCRIPTION("Intel MAX 10 BMC SPI bus interface");
 MODULE_AUTHOR("Intel Corporation");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("spi:intel-m10-bmc");
diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
index 725b51ea4aee..82e0820e146e 100644
--- a/include/linux/mfd/intel-m10-bmc.h
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -171,4 +171,10 @@ m10bmc_raw_read(struct intel_m10bmc *m10bmc, unsigned int addr,
 #define m10bmc_sys_read(m10bmc, offset, val) \
 	m10bmc_raw_read(m10bmc, M10BMC_SYS_BASE + (offset), val)
 
+/*
+ * MAX10 BMC Core support
+ */
+int m10bmc_dev_init(struct intel_m10bmc *m10bmc, const struct intel_m10bmc_platform_info *info);
+extern const struct attribute_group *m10bmc_dev_groups[];
+
 #endif /* __MFD_INTEL_M10_BMC_H */
-- 
2.30.2

[PATCH v2 04/11] mfd: intel-m10-bmc: Support multiple CSR register layouts

[Ilpo Järvinen]

There are different addresses for the MAX10 CSR registers. Introducing
a new data structure m10bmc_csr_map for the register definition of
MAX10 CSR.

Provide the csr_map for SPI.

Co-developed-by: Tianfei zhang <tianfei.zhang@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
Reviewed-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 73 +++++++++++++++++--------
 drivers/mfd/intel-m10-bmc-core.c        | 10 ++--
 drivers/mfd/intel-m10-bmc-spi.c         | 23 ++++++++
 include/linux/mfd/intel-m10-bmc.h       | 38 +++++++++++--
 4 files changed, 111 insertions(+), 33 deletions(-)

diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c
index 79d48852825e..dbe8aff95da3 100644
--- a/drivers/fpga/intel-m10-bmc-sec-update.c
+++ b/drivers/fpga/intel-m10-bmc-sec-update.c
@@ -73,16 +73,24 @@ show_root_entry_hash(struct device *dev, u32 exp_magic,
 	return cnt;
 }
 
-#define DEVICE_ATTR_SEC_REH_RO(_name, _magic, _prog_addr, _reh_addr) \
+#define DEVICE_ATTR_SEC_REH_RO(_name)						\
 static ssize_t _name##_root_entry_hash_show(struct device *dev, \
 					    struct device_attribute *attr, \
 					    char *buf) \
-{ return show_root_entry_hash(dev, _magic, _prog_addr, _reh_addr, buf); } \
+{										\
+	struct m10bmc_sec *sec = dev_get_drvdata(dev);				\
+	const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;	\
+										\
+	return show_root_entry_hash(dev, csr_map->_name##_magic,		\
+				    csr_map->_name##_prog_addr,			\
+				    csr_map->_name##_reh_addr,			\
+				    buf);					\
+}										\
 static DEVICE_ATTR_RO(_name##_root_entry_hash)
 
-DEVICE_ATTR_SEC_REH_RO(bmc, BMC_PROG_MAGIC, BMC_PROG_ADDR, BMC_REH_ADDR);
-DEVICE_ATTR_SEC_REH_RO(sr, SR_PROG_MAGIC, SR_PROG_ADDR, SR_REH_ADDR);
-DEVICE_ATTR_SEC_REH_RO(pr, PR_PROG_MAGIC, PR_PROG_ADDR, PR_REH_ADDR);
+DEVICE_ATTR_SEC_REH_RO(bmc);
+DEVICE_ATTR_SEC_REH_RO(sr);
+DEVICE_ATTR_SEC_REH_RO(pr);
 
 #define CSK_BIT_LEN		128U
 #define CSK_32ARRAY_SIZE	DIV_ROUND_UP(CSK_BIT_LEN, 32)
@@ -122,18 +130,25 @@ show_canceled_csk(struct device *dev, u32 addr, char *buf)
 	return bitmap_print_to_pagebuf(1, buf, csk_map, CSK_BIT_LEN);
 }
 
-#define DEVICE_ATTR_SEC_CSK_RO(_name, _addr) \
+#define DEVICE_ATTR_SEC_CSK_RO(_name)						\
 static ssize_t _name##_canceled_csks_show(struct device *dev, \
 					  struct device_attribute *attr, \
 					  char *buf) \
-{ return show_canceled_csk(dev, _addr, buf); } \
+{										\
+	struct m10bmc_sec *sec = dev_get_drvdata(dev);				\
+	const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;	\
+										\
+	return show_canceled_csk(dev,						\
+				 csr_map->_name##_prog_addr + CSK_VEC_OFFSET,	\
+				 buf);						\
+}										\
 static DEVICE_ATTR_RO(_name##_canceled_csks)
 
 #define CSK_VEC_OFFSET 0x34
 
-DEVICE_ATTR_SEC_CSK_RO(bmc, BMC_PROG_ADDR + CSK_VEC_OFFSET);
-DEVICE_ATTR_SEC_CSK_RO(sr, SR_PROG_ADDR + CSK_VEC_OFFSET);
-DEVICE_ATTR_SEC_CSK_RO(pr, PR_PROG_ADDR + CSK_VEC_OFFSET);
+DEVICE_ATTR_SEC_CSK_RO(bmc);
+DEVICE_ATTR_SEC_CSK_RO(sr);
+DEVICE_ATTR_SEC_CSK_RO(pr);
 
 #define FLASH_COUNT_SIZE 4096	/* count stored as inverted bit vector */
 
@@ -141,6 +156,7 @@ static ssize_t flash_count_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
 {
 	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;
 	u8 *flash_buf;
 	int cnt, ret;
@@ -160,12 +176,12 @@ static ssize_t flash_count_show(struct device *dev,
 	if (!flash_buf)
 		return -ENOMEM;
 
-	ret = regmap_bulk_read(sec->m10bmc->regmap, STAGING_FLASH_COUNT,
+	ret = regmap_bulk_read(sec->m10bmc->regmap, csr_map->rsu_update_counter,
 			       flash_buf, FLASH_COUNT_SIZE / stride);
 	if (ret) {
 		dev_err(sec->dev,
 			"failed to read flash count: %x cnt %x: %d\n",
-			STAGING_FLASH_COUNT, FLASH_COUNT_SIZE / stride, ret);
+			csr_map->rsu_update_counter, FLASH_COUNT_SIZE / stride, ret);
 		goto exit_free;
 	}
 	cnt = num_bits - bitmap_weight((unsigned long *)flash_buf, num_bits);
@@ -200,20 +216,22 @@ static const struct attribute_group *m10bmc_sec_attr_groups[] = {
 
 static void log_error_regs(struct m10bmc_sec *sec, u32 doorbell)
 {
+	const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;
 	u32 auth_result;
 
 	dev_err(sec->dev, "RSU error status: 0x%08x\n", doorbell);
 
-	if (!m10bmc_sys_read(sec->m10bmc, M10BMC_AUTH_RESULT, &auth_result))
+	if (!m10bmc_sys_read(sec->m10bmc, csr_map->auth_result, &auth_result))
 		dev_err(sec->dev, "RSU auth result: 0x%08x\n", auth_result);
 }
 
 static enum fw_upload_err rsu_check_idle(struct m10bmc_sec *sec)
 {
+	const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;
 	u32 doorbell;
 	int ret;
 
-	ret = m10bmc_sys_read(sec->m10bmc, M10BMC_DOORBELL, &doorbell);
+	ret = m10bmc_sys_read(sec->m10bmc, csr_map->doorbell, &doorbell);
 	if (ret)
 		return FW_UPLOAD_ERR_RW_ERROR;
 
@@ -246,11 +264,12 @@ static inline bool rsu_start_done(u32 doorbell)
 
 static enum fw_upload_err rsu_update_init(struct m10bmc_sec *sec)
 {
+	const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;
 	u32 doorbell, status;
 	int ret;
 
 	ret = regmap_update_bits(sec->m10bmc->regmap,
-				 M10BMC_SYS_BASE + M10BMC_DOORBELL,
+				 csr_map->base + csr_map->doorbell,
 				 DRBL_RSU_REQUEST | DRBL_HOST_STATUS,
 				 DRBL_RSU_REQUEST |
 				 FIELD_PREP(DRBL_HOST_STATUS,
@@ -259,7 +278,7 @@ static enum fw_upload_err rsu_update_init(struct m10bmc_sec *sec)
 		return FW_UPLOAD_ERR_RW_ERROR;
 
 	ret = regmap_read_poll_timeout(sec->m10bmc->regmap,
-				       M10BMC_SYS_BASE + M10BMC_DOORBELL,
+				       csr_map->base + csr_map->doorbell,
 				       doorbell,
 				       rsu_start_done(doorbell),
 				       NIOS_HANDSHAKE_INTERVAL_US,
@@ -286,11 +305,12 @@ static enum fw_upload_err rsu_update_init(struct m10bmc_sec *sec)
 
 static enum fw_upload_err rsu_prog_ready(struct m10bmc_sec *sec)
 {
+	const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;
 	unsigned long poll_timeout;
 	u32 doorbell, progress;
 	int ret;
 
-	ret = m10bmc_sys_read(sec->m10bmc, M10BMC_DOORBELL, &doorbell);
+	ret = m10bmc_sys_read(sec->m10bmc, csr_map->doorbell, &doorbell);
 	if (ret)
 		return FW_UPLOAD_ERR_RW_ERROR;
 
@@ -300,7 +320,7 @@ static enum fw_upload_err rsu_prog_ready(struct m10bmc_sec *sec)
 		if (time_after(jiffies, poll_timeout))
 			break;
 
-		ret = m10bmc_sys_read(sec->m10bmc, M10BMC_DOORBELL, &doorbell);
+		ret = m10bmc_sys_read(sec->m10bmc, csr_map->doorbell, &doorbell);
 		if (ret)
 			return FW_UPLOAD_ERR_RW_ERROR;
 	}
@@ -319,11 +339,12 @@ static enum fw_upload_err rsu_prog_ready(struct m10bmc_sec *sec)
 
 static enum fw_upload_err rsu_send_data(struct m10bmc_sec *sec)
 {
+	const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;
 	u32 doorbell;
 	int ret;
 
 	ret = regmap_update_bits(sec->m10bmc->regmap,
-				 M10BMC_SYS_BASE + M10BMC_DOORBELL,
+				 csr_map->base + csr_map->doorbell,
 				 DRBL_HOST_STATUS,
 				 FIELD_PREP(DRBL_HOST_STATUS,
 					    HOST_STATUS_WRITE_DONE));
@@ -331,7 +352,7 @@ static enum fw_upload_err rsu_send_data(struct m10bmc_sec *sec)
 		return FW_UPLOAD_ERR_RW_ERROR;
 
 	ret = regmap_read_poll_timeout(sec->m10bmc->regmap,
-				       M10BMC_SYS_BASE + M10BMC_DOORBELL,
+				       csr_map->base + csr_map->doorbell,
 				       doorbell,
 				       rsu_prog(doorbell) != RSU_PROG_READY,
 				       NIOS_HANDSHAKE_INTERVAL_US,
@@ -360,7 +381,9 @@ static enum fw_upload_err rsu_send_data(struct m10bmc_sec *sec)
 
 static int rsu_check_complete(struct m10bmc_sec *sec, u32 *doorbell)
 {
-	if (m10bmc_sys_read(sec->m10bmc, M10BMC_DOORBELL, doorbell))
+	const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;
+
+	if (m10bmc_sys_read(sec->m10bmc, csr_map->doorbell, doorbell))
 		return -EIO;
 
 	switch (rsu_stat(*doorbell)) {
@@ -389,10 +412,11 @@ static int rsu_check_complete(struct m10bmc_sec *sec, u32 *doorbell)
 
 static enum fw_upload_err rsu_cancel(struct m10bmc_sec *sec)
 {
+	const struct m10bmc_csr_map *csr_map = sec->m10bmc->info->csr_map;
 	u32 doorbell;
 	int ret;
 
-	ret = m10bmc_sys_read(sec->m10bmc, M10BMC_DOORBELL, &doorbell);
+	ret = m10bmc_sys_read(sec->m10bmc, csr_map->doorbell, &doorbell);
 	if (ret)
 		return FW_UPLOAD_ERR_RW_ERROR;
 
@@ -400,7 +424,7 @@ static enum fw_upload_err rsu_cancel(struct m10bmc_sec *sec)
 		return FW_UPLOAD_ERR_BUSY;
 
 	ret = regmap_update_bits(sec->m10bmc->regmap,
-				 M10BMC_SYS_BASE + M10BMC_DOORBELL,
+				 csr_map->base + csr_map->doorbell,
 				 DRBL_HOST_STATUS,
 				 FIELD_PREP(DRBL_HOST_STATUS,
 					    HOST_STATUS_ABORT_RSU));
@@ -445,6 +469,7 @@ static enum fw_upload_err m10bmc_sec_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;
 	int ret;
@@ -453,7 +478,7 @@ static enum fw_upload_err m10bmc_sec_write(struct fw_upload *fwl, const u8 *data
 	if (sec->cancel_request)
 		return rsu_cancel(sec);
 
-	ret = m10bmc_sys_read(sec->m10bmc, M10BMC_DOORBELL, &doorbell);
+	ret = m10bmc_sys_read(sec->m10bmc, csr_map->doorbell, &doorbell);
 	if (ret) {
 		return FW_UPLOAD_ERR_RW_ERROR;
 	} else if (rsu_prog(doorbell) != RSU_PROG_READY) {
diff --git a/drivers/mfd/intel-m10-bmc-core.c b/drivers/mfd/intel-m10-bmc-core.c
index 6630b81b10c4..51b78b868235 100644
--- a/drivers/mfd/intel-m10-bmc-core.c
+++ b/drivers/mfd/intel-m10-bmc-core.c
@@ -19,7 +19,7 @@ static ssize_t bmc_version_show(struct device *dev,
 	unsigned int val;
 	int ret;
 
-	ret = m10bmc_sys_read(ddata, M10BMC_BUILD_VER, &val);
+	ret = m10bmc_sys_read(ddata, ddata->info->csr_map->build_version, &val);
 	if (ret)
 		return ret;
 
@@ -34,7 +34,7 @@ static ssize_t bmcfw_version_show(struct device *dev,
 	unsigned int val;
 	int ret;
 
-	ret = m10bmc_sys_read(ddata, NIOS2_FW_VERSION, &val);
+	ret = m10bmc_sys_read(ddata, ddata->info->csr_map->fw_version, &val);
 	if (ret)
 		return ret;
 
@@ -49,11 +49,11 @@ static ssize_t mac_address_show(struct device *dev,
 	unsigned int macaddr_low, macaddr_high;
 	int ret;
 
-	ret = m10bmc_sys_read(ddata, M10BMC_MAC_LOW, &macaddr_low);
+	ret = m10bmc_sys_read(ddata, ddata->info->csr_map->mac_low, &macaddr_low);
 	if (ret)
 		return ret;
 
-	ret = m10bmc_sys_read(ddata, M10BMC_MAC_HIGH, &macaddr_high);
+	ret = m10bmc_sys_read(ddata, ddata->info->csr_map->mac_high, &macaddr_high);
 	if (ret)
 		return ret;
 
@@ -74,7 +74,7 @@ static ssize_t mac_count_show(struct device *dev,
 	unsigned int macaddr_high;
 	int ret;
 
-	ret = m10bmc_sys_read(ddata, M10BMC_MAC_HIGH, &macaddr_high);
+	ret = m10bmc_sys_read(ddata, ddata->info->csr_map->mac_high, &macaddr_high);
 	if (ret)
 		return ret;
 
diff --git a/drivers/mfd/intel-m10-bmc-spi.c b/drivers/mfd/intel-m10-bmc-spi.c
index be1d4ddedabb..611a4ab42717 100644
--- a/drivers/mfd/intel-m10-bmc-spi.c
+++ b/drivers/mfd/intel-m10-bmc-spi.c
@@ -91,6 +91,26 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
 	return m10bmc_dev_init(ddata, info);
 }
 
+static const struct m10bmc_csr_map m10bmc_spi_csr_map = {
+	.base = M10BMC_SYS_BASE,
+	.build_version = M10BMC_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,
+	.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,
+};
+
 static struct mfd_cell m10bmc_d5005_subdevs[] = {
 	{ .name = "d5005bmc-hwmon" },
 	{ .name = "d5005bmc-sec-update" },
@@ -109,16 +129,19 @@ static struct mfd_cell m10bmc_n5010_subdevs[] = {
 static const struct intel_m10bmc_platform_info m10bmc_spi_n3000 = {
 	.cells = m10bmc_pacn3000_subdevs,
 	.n_cells = ARRAY_SIZE(m10bmc_pacn3000_subdevs),
+	.csr_map = &m10bmc_spi_csr_map,
 };
 
 static const struct intel_m10bmc_platform_info m10bmc_spi_d5005 = {
 	.cells = m10bmc_d5005_subdevs,
 	.n_cells = ARRAY_SIZE(m10bmc_d5005_subdevs),
+	.csr_map = &m10bmc_spi_csr_map,
 };
 
 static const struct intel_m10bmc_platform_info m10bmc_spi_n5010 = {
 	.cells = m10bmc_n5010_subdevs,
 	.n_cells = ARRAY_SIZE(m10bmc_n5010_subdevs),
+	.csr_map = &m10bmc_spi_csr_map,
 };
 
 static const struct spi_device_id m10bmc_spi_id[] = {
diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
index 82e0820e146e..91567375f1bf 100644
--- a/include/linux/mfd/intel-m10-bmc.h
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -118,14 +118,39 @@
 /* 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
+ */
+struct m10bmc_csr_map {
+	unsigned int base;
+	unsigned int build_version;
+	unsigned int fw_version;
+	unsigned int mac_low;
+	unsigned int mac_high;
+	unsigned int doorbell;
+	unsigned int auth_result;
+	unsigned int bmc_prog_addr;
+	unsigned int bmc_reh_addr;
+	unsigned int bmc_magic;
+	unsigned int sr_prog_addr;
+	unsigned int sr_reh_addr;
+	unsigned int sr_magic;
+	unsigned int pr_prog_addr;
+	unsigned int pr_reh_addr;
+	unsigned int pr_magic;
+	unsigned int rsu_update_counter;
+};
+
 /**
  * struct intel_m10bmc_platform_info - Intel MAX 10 BMC platform specific information
  * @cells: MFD cells
  * @n_cells: MFD cells ARRAY_SIZE()
+ * @csr_map: the mappings for register definition of MAX10 BMC
  */
 struct intel_m10bmc_platform_info {
 	struct mfd_cell *cells;
 	int n_cells;
+	const struct m10bmc_csr_map *csr_map;
 };
 
 /**
@@ -164,12 +189,17 @@ m10bmc_raw_read(struct intel_m10bmc *m10bmc, unsigned int addr,
  * The base of the system registers could be configured by HW developers, and
  * in HW SPEC, the base is not added to the addresses of the system registers.
  *
- * This macro helps to simplify the accessing of the system registers. And if
+ * This function helps to simplify the accessing of the system registers. And if
  * the base is reconfigured in HW, SW developers could simply change the
- * M10BMC_SYS_BASE accordingly.
+ * csr_map's base accordingly.
  */
-#define m10bmc_sys_read(m10bmc, offset, val) \
-	m10bmc_raw_read(m10bmc, M10BMC_SYS_BASE + (offset), val)
+static inline int m10bmc_sys_read(struct intel_m10bmc *m10bmc, unsigned int offset,
+				  unsigned int *val)
+{
+	const struct m10bmc_csr_map *csr_map = m10bmc->info->csr_map;
+
+	return m10bmc_raw_read(m10bmc, csr_map->base + offset, val);
+}
 
 /*
  * MAX10 BMC Core support
-- 
2.30.2

[PATCH v2 05/11] fpga: intel-m10-bmc: Add flash ops for sec update

[Ilpo Järvinen]

Access to flash staging area is different for N6000 from that of the
SPI interfaced counterparts.

Introduce intel_m10bmc_flash_ops to allow interface specific
differentiations for the flash access path for sec update, add flash
ops for SPI-based MAX10 interface, and make sec update driver to use
the new operations.

Co-developed-by: Tianfei zhang <tianfei.zhang@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
Co-developed-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/fpga/intel-m10-bmc-sec-update.c | 79 ++++++-------------------
 drivers/mfd/intel-m10-bmc-spi.c         | 62 +++++++++++++++++++
 include/linux/mfd/intel-m10-bmc.h       | 14 +++++
 3 files changed, 94 insertions(+), 61 deletions(-)

diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c
index dbe8aff95da3..3bd22d03616a 100644
--- a/drivers/fpga/intel-m10-bmc-sec-update.c
+++ b/drivers/fpga/intel-m10-bmc-sec-update.c
@@ -38,11 +38,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 = sec->m10bmc->flash_ops->read(sec->m10bmc, (u8 *)&magic, prog_addr, sizeof(magic));
 	if (ret)
 		return ret;
 
@@ -50,19 +48,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 = sec->m10bmc->flash_ops->read(sec->m10bmc, 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 +93,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 = sec->m10bmc->flash_ops->read(sec->m10bmc, (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 +141,21 @@ 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 = sec->m10bmc->flash_ops->read(sec->m10bmc, 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);
@@ -470,15 +444,14 @@ static enum fw_upload_err m10bmc_sec_write(struct fw_upload *fwl, const u8 *data
 {
 	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 +459,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->flash_ops->write(m10bmc, 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;
 }
diff --git a/drivers/mfd/intel-m10-bmc-spi.c b/drivers/mfd/intel-m10-bmc-spi.c
index 611a4ab42717..b70e3e87600a 100644
--- a/drivers/mfd/intel-m10-bmc-spi.c
+++ b/drivers/mfd/intel-m10-bmc-spi.c
@@ -33,6 +33,67 @@ static struct regmap_config intel_m10bmc_regmap_config = {
 	.max_register = M10BMC_MEM_END,
 };
 
+static int m10bmc_spi_flash_write(struct intel_m10bmc *m10bmc, const u8 *buf, u32 offset, u32 size)
+{
+	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_spi_flash_read(struct intel_m10bmc *m10bmc, u8 *buf, u32 addr, u32 size)
+{
+	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 const struct intel_m10bmc_flash_ops m10bmc_spi_flash_ops = {
+	.read = m10bmc_spi_flash_read,
+	.write = m10bmc_spi_flash_write,
+};
+
 static int check_m10bmc_version(struct intel_m10bmc *ddata)
 {
 	unsigned int v;
@@ -72,6 +133,7 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
 
 	info = (struct intel_m10bmc_platform_info *)id->driver_data;
 	ddata->dev = dev;
+	ddata->flash_ops = &m10bmc_spi_flash_ops;
 
 	ddata->regmap = devm_regmap_init_spi_avmm(spi, &intel_m10bmc_regmap_config);
 	if (IS_ERR(ddata->regmap)) {
diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
index 91567375f1bf..7f87e898a212 100644
--- a/include/linux/mfd/intel-m10-bmc.h
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -153,16 +153,30 @@ struct intel_m10bmc_platform_info {
 	const struct m10bmc_csr_map *csr_map;
 };
 
+struct intel_m10bmc;
+
+/**
+ * struct intel_m10bmc_flash_ops - device specific operations for flash R/W
+ * @read: read a block of data from flash
+ * @write: write a block of data to flash
+ */
+struct intel_m10bmc_flash_ops {
+	int (*read)(struct intel_m10bmc *m10bmc, u8 *buf, u32 addr, u32 size);
+	int (*write)(struct intel_m10bmc *m10bmc, const u8 *buf, u32 offset, u32 size);
+};
+
 /**
  * struct intel_m10bmc - Intel MAX 10 BMC parent driver data structure
  * @dev: this device
  * @regmap: the regmap used to access registers by m10bmc itself
  * @info: the platform information for MAX10 BMC
+ * @flash_ops: optional device specific operations for flash R/W
  */
 struct intel_m10bmc {
 	struct device *dev;
 	struct regmap *regmap;
 	const struct intel_m10bmc_platform_info *info;
+	const struct intel_m10bmc_flash_ops *flash_ops;
 };
 
 /*
-- 
2.30.2

[PATCH v2 06/11] mfd: intel-m10-bmc: Downscope SPI defines & prefix with M10BMC_SPI

[Ilpo Järvinen]

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_SPI
to make it more obvious these are related to SPI only.

Some bitfield defs are also moved to intel-m10-bmc-core which seems
more appropriate for them.

Reviewed-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/mfd/intel-m10-bmc-core.c  | 11 ++++
 drivers/mfd/intel-m10-bmc-spi.c   | 94 ++++++++++++++++++++++---------
 include/linux/mfd/intel-m10-bmc.h | 47 ----------------
 3 files changed, 78 insertions(+), 74 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 b70e3e87600a..a17a0b50f352 100644
--- a/drivers/mfd/intel-m10-bmc-spi.c
+++ b/drivers/mfd/intel-m10-bmc-spi.c
@@ -13,10 +13,49 @@
 #include <linux/regmap.h>
 #include <linux/spi/spi.h>
 
+#define M10BMC_SPI_LEGACY_BUILD_VER	0x300468
+#define M10BMC_SPI_SYS_BASE		0x300800
+#define M10BMC_SPI_SYS_END		0x300fff
+#define M10BMC_SPI_FLASH_BASE		0x10000000
+#define M10BMC_SPI_FLASH_END		0x1fffffff
+#define M10BMC_SPI_MEM_END		M10BMC_SPI_FLASH_END
+
+#define M10BMC_SPI_STAGING_BASE		0x18000000
+
+/* Register offset of system registers */
+#define NIOS2_FW_VERSION		0x0
+#define M10BMC_SPI_MAC_LOW		0x10
+#define M10BMC_SPI_MAC_HIGH		0x14
+#define M10BMC_SPI_TEST_REG		0x3c
+#define M10BMC_SPI_BUILD_VER		0x68
+#define M10BMC_SPI_VER_LEGACY_INVALID	0xffffffff
+
+/* Secure update doorbell register, in system register region */
+#define M10BMC_SPI_DOORBELL		0x400
+
+/* Authorization Result register, in system register region */
+#define M10BMC_SPI_AUTH_RESULT		0x404
+
+/* Addresses for security related data in FLASH */
+#define M10BMC_SPI_BMC_REH_ADDR		0x17ffc004
+#define M10BMC_SPI_BMC_PROG_ADDR	0x17ffc000
+#define M10BMC_SPI_BMC_PROG_MAGIC	0x5746
+
+#define M10BMC_SPI_SR_REH_ADDR		0x17ffd004
+#define M10BMC_SPI_SR_PROG_ADDR		0x17ffd000
+#define M10BMC_SPI_SR_PROG_MAGIC	0x5253
+
+#define M10BMC_SPI_PR_REH_ADDR		0x17ffe004
+#define M10BMC_SPI_PR_PROG_ADDR		0x17ffe000
+#define M10BMC_SPI_PR_PROG_MAGIC	0x5250
+
+/* Address of 4KB inverted bit vector containing staging area FLASH count */
+#define M10BMC_SPI_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_SPI_LEGACY_BUILD_VER, M10BMC_SPI_LEGACY_BUILD_VER),
+	regmap_reg_range(M10BMC_SPI_SYS_BASE, M10BMC_SPI_SYS_END),
+	regmap_reg_range(M10BMC_SPI_FLASH_BASE, M10BMC_SPI_FLASH_END),
 };
 
 static const struct regmap_access_table m10bmc_access_table = {
@@ -30,7 +69,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_SPI_MEM_END,
 };
 
 static int m10bmc_spi_flash_write(struct intel_m10bmc *m10bmc, const u8 *buf, u32 offset, u32 size)
@@ -45,7 +84,7 @@ static int m10bmc_spi_flash_write(struct intel_m10bmc *m10bmc, const u8 *buf, u3
 	if (WARN_ON_ONCE(stride > sizeof(leftover_tmp)))
 		return -EINVAL;
 
-	ret = regmap_bulk_write(m10bmc->regmap, M10BMC_STAGING_BASE + offset,
+	ret = regmap_bulk_write(m10bmc->regmap, M10BMC_SPI_STAGING_BASE + offset,
 				buf + offset, write_count);
 	if (ret)
 		return ret;
@@ -53,7 +92,8 @@ static int m10bmc_spi_flash_write(struct intel_m10bmc *m10bmc, const u8 *buf, u3
 	/* 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,
+		ret = regmap_write(m10bmc->regmap,
+				   M10BMC_SPI_STAGING_BASE + offset + leftover_offset,
 				   leftover_tmp);
 		if (ret)
 			return ret;
@@ -102,16 +142,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_SPI_LEGACY_BUILD_VER), so its read out value would have
+	 * not been M10BMC_SPI_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_SPI_VER_LEGACY_INVALID.
 	 */
-	ret = m10bmc_raw_read(ddata, M10BMC_LEGACY_BUILD_VER, &v);
+	ret = m10bmc_raw_read(ddata, M10BMC_SPI_LEGACY_BUILD_VER, &v);
 	if (ret)
 		return -ENODEV;
 
-	if (v != M10BMC_VER_LEGACY_INVALID) {
+	if (v != M10BMC_SPI_VER_LEGACY_INVALID) {
 		dev_err(ddata->dev, "bad version M10BMC detected\n");
 		return -ENODEV;
 	}
@@ -154,23 +194,23 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
 }
 
 static const struct m10bmc_csr_map m10bmc_spi_csr_map = {
-	.base = M10BMC_SYS_BASE,
-	.build_version = M10BMC_BUILD_VER,
+	.base = M10BMC_SPI_SYS_BASE,
+	.build_version = M10BMC_SPI_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,
-	.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_SPI_MAC_LOW,
+	.mac_high = M10BMC_SPI_MAC_HIGH,
+	.doorbell = M10BMC_SPI_DOORBELL,
+	.auth_result = M10BMC_SPI_AUTH_RESULT,
+	.bmc_prog_addr = M10BMC_SPI_BMC_PROG_ADDR,
+	.bmc_reh_addr = M10BMC_SPI_BMC_REH_ADDR,
+	.bmc_magic = M10BMC_SPI_BMC_PROG_MAGIC,
+	.sr_prog_addr = M10BMC_SPI_SR_PROG_ADDR,
+	.sr_reh_addr = M10BMC_SPI_SR_REH_ADDR,
+	.sr_magic = M10BMC_SPI_SR_PROG_MAGIC,
+	.pr_prog_addr = M10BMC_SPI_PR_PROG_ADDR,
+	.pr_reh_addr = M10BMC_SPI_PR_REH_ADDR,
+	.pr_magic = M10BMC_SPI_PR_PROG_MAGIC,
+	.rsu_update_counter = M10BMC_SPI_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 7f87e898a212..200425b5b266 100644
--- a/include/linux/mfd/intel-m10-bmc.h
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -9,39 +9,8 @@
 
 #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 +71,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

[PATCH v2 07/11] regmap: indirect: Add indirect regmap support

[Ilpo Järvinen]

Add support for indirect register access via a regmap interface.

Indirect register access is a generic way to access registers indirectly.
One use case is accessing registers on Intel FPGA IPs with e.g. PMCI or
HSSI.

Co-developed-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/base/regmap/Kconfig           |   3 +
 drivers/base/regmap/Makefile          |   1 +
 drivers/base/regmap/regmap-indirect.c | 128 ++++++++++++++++++++++++++
 include/linux/regmap.h                |  55 +++++++++++
 4 files changed, 187 insertions(+)
 create mode 100644 drivers/base/regmap/regmap-indirect.c

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 159bac6c5046..94e5ca5434cf 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -65,3 +65,6 @@ config REGMAP_I3C
 config REGMAP_SPI_AVMM
 	tristate
 	depends on SPI
+
+config REGMAP_INDIRECT
+	tristate
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index 11facb32a027..6221a4740806 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_REGMAP_SCCB) += regmap-sccb.o
 obj-$(CONFIG_REGMAP_I3C) += regmap-i3c.o
 obj-$(CONFIG_REGMAP_SPI_AVMM) += regmap-spi-avmm.o
 obj-$(CONFIG_REGMAP_MDIO) += regmap-mdio.o
+obj-$(CONFIG_REGMAP_INDIRECT) += regmap-indirect.o
diff --git a/drivers/base/regmap/regmap-indirect.c b/drivers/base/regmap/regmap-indirect.c
new file mode 100644
index 000000000000..ac42a36eb907
--- /dev/null
+++ b/drivers/base/regmap/regmap-indirect.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Indirect Register Access.
+ *
+ * Copyright (C) 2020-2022 Intel Corporation, Inc.
+ */
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+
+struct indirect_ctx {
+	void __iomem *base;
+	struct device *dev;
+	const struct regmap_indirect_cfg *indirect_cfg;
+};
+
+static int indirect_bus_idle_cmd(struct indirect_ctx *ctx)
+{
+	unsigned int cmd;
+	int ret;
+
+	writel(ctx->indirect_cfg->idle_cmd, ctx->base + ctx->indirect_cfg->cmd_offset);
+
+	ret = readl_poll_timeout(ctx->base + ctx->indirect_cfg->cmd_offset, cmd,
+				 cmd == ctx->indirect_cfg->idle_cmd,
+				 ctx->indirect_cfg->sleep_us, ctx->indirect_cfg->timeout_us);
+	if (ret)
+		dev_err(ctx->dev, "timed out waiting idle cmd (residual cmd=0x%x)\n", cmd);
+
+	return ret;
+}
+
+static int indirect_bus_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 + ctx->indirect_cfg->cmd_offset);
+	if (cmd != ctx->indirect_cfg->idle_cmd)
+		dev_warn(ctx->dev, "residual cmd 0x%x on read entry\n", cmd);
+
+	writel(reg, ctx->base + ctx->indirect_cfg->addr_offset);
+	writel(ctx->indirect_cfg->read_cmd, ctx->base + ctx->indirect_cfg->cmd_offset);
+
+	ret = readl_poll_timeout(ctx->base + ctx->indirect_cfg->ack_offset, ack,
+				 (ack & ctx->indirect_cfg->ack_mask) == ctx->indirect_cfg->ack_mask,
+				 ctx->indirect_cfg->sleep_us, ctx->indirect_cfg->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 + ctx->indirect_cfg->read_offset);
+
+	if (indirect_bus_idle_cmd(ctx)) {
+		if (!ret)
+			ret = -ETIMEDOUT;
+		goto out;
+	}
+
+	*val = tmpval;
+out:
+	return ret;
+}
+
+static int indirect_bus_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 + ctx->indirect_cfg->cmd_offset);
+	if (cmd != ctx->indirect_cfg->idle_cmd)
+		dev_warn(ctx->dev, "residual cmd 0x%x on write entry\n", cmd);
+
+	writel(val, ctx->base + ctx->indirect_cfg->write_offset);
+	writel(reg, ctx->base + ctx->indirect_cfg->addr_offset);
+	writel(ctx->indirect_cfg->write_cmd, ctx->base + ctx->indirect_cfg->cmd_offset);
+
+	ret = readl_poll_timeout(ctx->base + ctx->indirect_cfg->ack_offset, ack,
+				 (ack & ctx->indirect_cfg->ack_mask) == ctx->indirect_cfg->ack_mask,
+				 ctx->indirect_cfg->sleep_us, ctx->indirect_cfg->timeout_us);
+	if (ret)
+		dev_err(ctx->dev, "write timed out on reg 0x%x ack 0x%x\n", reg, ack);
+
+	if (indirect_bus_idle_cmd(ctx)) {
+		if (!ret)
+			ret = -ETIMEDOUT;
+	}
+
+	return ret;
+}
+
+static const struct regmap_bus indirect_bus = {
+	.reg_write = indirect_bus_reg_write,
+	.reg_read =  indirect_bus_reg_read,
+};
+
+struct regmap *__devm_regmap_init_indirect(struct device *dev,
+					   void __iomem *base,
+					   struct regmap_config *cfg,
+					   struct lock_class_key *lock_key,
+					   const char *lock_name)
+{
+	struct indirect_ctx *ctx;
+
+	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return NULL;
+
+	ctx->base = base;
+	ctx->dev = dev;
+	ctx->indirect_cfg = cfg->indirect_cfg;
+
+	indirect_bus_idle_cmd(ctx);
+
+	return __devm_regmap_init(dev, &indirect_bus, ctx, cfg, lock_key, lock_name);
+}
+EXPORT_SYMBOL_GPL(__devm_regmap_init_indirect);
+
+MODULE_DESCRIPTION("Indirect Register Access");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index ca3434dca3a0..adaa7bca4f60 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -190,6 +190,41 @@ enum regmap_endian {
 	REGMAP_ENDIAN_NATIVE,
 };
 
+/**
+ * struct regmap_indirect_cfg - A configuration for indirect register access
+ *
+ * @cmd_offset: Command register offset
+ * @idle_cmd: Idle command
+ * @read_cmd: Read command
+ * @write_cmd: Write command
+ *
+ * @ack_offset: Command acknowledgment register offset
+ * @ack_mask: Command acknowledgment bit mask
+ *
+ * @addr_offset: Address register offset
+ * @read_offset: Read register offset
+ * @write_offset: Write register offset
+ *
+ * @sleep_us: Command wait sleep (usecs)
+ * @timeout_us: Command timeout (usecs)
+ */
+struct regmap_indirect_cfg {
+	unsigned int cmd_offset;
+	u32 idle_cmd;
+	u32 read_cmd;
+	u32 write_cmd;
+
+	unsigned int ack_offset;
+	u32 ack_mask;
+
+	unsigned int addr_offset;
+	unsigned int read_offset;
+	unsigned int write_offset;
+
+	unsigned long sleep_us;
+	unsigned long timeout_us;
+};
+
 /**
  * struct regmap_range - A register range, used for access related checks
  *                       (readable/writeable/volatile/precious checks)
@@ -431,6 +466,8 @@ struct regmap_config {
 	const struct regmap_range_cfg *ranges;
 	unsigned int num_ranges;
 
+	const struct regmap_indirect_cfg *indirect_cfg;
+
 	bool use_hwlock;
 	bool use_raw_spinlock;
 	unsigned int hwlock_id;
@@ -693,6 +730,12 @@ struct regmap *__devm_regmap_init_spi_avmm(struct spi_device *spi,
 					   const struct regmap_config *config,
 					   struct lock_class_key *lock_key,
 					   const char *lock_name);
+struct regmap *__devm_regmap_init_indirect(struct device *dev,
+					   void __iomem *base,
+					   struct regmap_config *cfg,
+					   struct lock_class_key *lock_key,
+					   const char *lock_name);
+
 /*
  * Wrapper for regmap_init macros to include a unique lockdep key and name
  * for each call. No-op if CONFIG_LOCKDEP is not set.
@@ -1148,6 +1191,18 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
 	__regmap_lockdep_wrapper(__devm_regmap_init_spi_avmm, #config,	\
 				 spi, config)
 
+/**
+ * devm_regmap_init_indirect - create a regmap for indirect register access
+ * @dev: device creating the regmap
+ * @base: __iomem point to base of memory with mailbox
+ * @cfg: regmap_config describing interface
+ *
+ * Return: 0 on success, negative error code otherwise.
+ */
+#define devm_regmap_init_indirect(dev, base, config)			\
+	__regmap_lockdep_wrapper(__devm_regmap_init_indirect, #config,	\
+				 dev, base, config)
+
 int regmap_mmio_attach_clk(struct regmap *map, struct clk *clk);
 void regmap_mmio_detach_clk(struct regmap *map);
 void regmap_exit(struct regmap *map);
-- 
2.30.2

[PATCH v2 08/11] intel-m10-bmc: Add regmap_indirect_cfg for Intel FPGA IPs

[Ilpo Järvinen]

Create the regmap_indirect_cfg with offsets and commands for Intel FPGA
IPs indirect register access.

Reviewed-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 include/linux/mfd/intel-m10-bmc.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
index 200425b5b266..03ba92a68e01 100644
--- a/include/linux/mfd/intel-m10-bmc.h
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -9,6 +9,19 @@
 
 #include <linux/regmap.h>
 
+#define INTEL_M10_REGMAP_INDIRECT_CFG	\
+	.cmd_offset = 0,	\
+	.idle_cmd = 0,		\
+	.read_cmd = BIT(0),	\
+	.write_cmd = BIT(1),	\
+	.ack_offset = 0,	\
+	.ack_mask = BIT(2),	\
+	.addr_offset = 0x4,	\
+	.read_offset = 0x8,	\
+	.write_offset = 0xc,	\
+	.sleep_us = 1,		\
+	.timeout_us = 10000
+
 #define M10BMC_STAGING_SIZE		0x3800000
 
 /* Doorbell register fields */
-- 
2.30.2

[PATCH v2 09/11] mfd: intel-m10-bmc: Add PMCI driver

[Ilpo Järvinen]

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. This
driver leverages the regmap APIs to support Intel specific Indirect
Register Interface for register read/write on PMCI.

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 <tianfei.zhang@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
Co-developed-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 .../ABI/testing/sysfs-driver-intel-m10-bmc    |   8 +-
 drivers/mfd/Kconfig                           |  12 ++
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/intel-m10-bmc-pmci.c              | 152 ++++++++++++++++++
 4 files changed, 169 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 <yilun.xu@intel.com>
@@ -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@intel.com>
@@ -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@intel.com>
@@ -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@intel.com>
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..8df79056b0b1
--- /dev/null
+++ b/drivers/mfd/intel-m10-bmc-pmci.c
@@ -0,0 +1,152 @@
+// 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, 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 M10BMC_PMCI_BMC_REH_ADDR	0x7ffc004
+#define M10BMC_PMCI_BMC_PROG_ADDR	0x7ffc000
+#define M10BMC_PMCI_BMC_PROG_MAGIC	0x5746
+
+#define M10BMC_PMCI_SR_REH_ADDR		0x7ffd004
+#define M10BMC_PMCI_SR_PROG_ADDR	0x7ffd000
+#define M10BMC_PMCI_SR_PROG_MAGIC	0x5253
+
+#define M10BMC_PMCI_PR_REH_ADDR		0x7ffe004
+#define M10BMC_PMCI_PR_PROG_ADDR	0x7ffe000
+#define M10BMC_PMCI_PR_PROG_MAGIC	0x5250
+
+#define M10BMC_PMCI_STAGING_FLASH_COUNT	0x7ff5000
+
+struct m10bmc_pmci_device {
+	void __iomem *base;
+	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 m10bmc_pmci_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 = &m10bmc_pmci_access_table,
+	.rd_table = &m10bmc_pmci_access_table,
+	.max_register = M10BMC_PMCI_SYS_END,
+	.indirect_cfg = &indirect_cfg,
+};
+
+static struct mfd_cell m10bmc_pmci_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 = M10BMC_PMCI_BMC_PROG_ADDR,
+	.bmc_reh_addr = M10BMC_PMCI_BMC_REH_ADDR,
+	.bmc_magic = M10BMC_PMCI_BMC_PROG_MAGIC,
+	.sr_prog_addr = M10BMC_PMCI_SR_PROG_ADDR,
+	.sr_reh_addr = M10BMC_PMCI_SR_REH_ADDR,
+	.sr_magic = M10BMC_PMCI_SR_PROG_MAGIC,
+	.pr_prog_addr = M10BMC_PMCI_PR_PROG_ADDR,
+	.pr_reh_addr = M10BMC_PMCI_PR_REH_ADDR,
+	.pr_magic = M10BMC_PMCI_PR_PROG_MAGIC,
+	.rsu_update_counter = M10BMC_PMCI_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_pmci_csr_map,
+};
+
+static int m10bmc_pmci_probe(struct dfl_device *ddev)
+{
+	struct device *dev = &ddev->dev;
+	struct m10bmc_pmci_device *pmci;
+
+	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);
+
+	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_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

[PATCH v2 10/11] fpga: m10bmc-sec: Add support for N6000

[Ilpo Järvinen]

Add support for PMCI-based flash access path and N6000 sec update
support.

Co-developed-by: Tianfei zhang <tianfei.zhang@intel.com>
Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
Co-developed-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/fpga/intel-m10-bmc-sec-update.c |   3 +
 drivers/mfd/intel-m10-bmc-pmci.c        | 209 ++++++++++++++++++++++++
 2 files changed, 212 insertions(+)

diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c
index 3bd22d03616a..fa5141f3504b 100644
--- a/drivers/fpga/intel-m10-bmc-sec-update.c
+++ b/drivers/fpga/intel-m10-bmc-sec-update.c
@@ -590,6 +590,9 @@ static const struct platform_device_id intel_m10bmc_sec_ids[] = {
 	{
 		.name = "d5005bmc-sec-update",
 	},
+	{
+		.name = "n6000bmc-sec-update",
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(platform, intel_m10bmc_sec_ids);
diff --git a/drivers/mfd/intel-m10-bmc-pmci.c b/drivers/mfd/intel-m10-bmc-pmci.c
index 8df79056b0b1..7ac77ed15194 100644
--- a/drivers/mfd/intel-m10-bmc-pmci.c
+++ b/drivers/mfd/intel-m10-bmc-pmci.c
@@ -7,9 +7,11 @@
  *
  */
 
+#include <linux/bitfield.h>
 #include <linux/dfl.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/intel-m10-bmc.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
 
@@ -45,9 +47,203 @@
 
 #define M10BMC_PMCI_STAGING_FLASH_COUNT	0x7ff5000
 
+#define M10BMC_PMCI_FLASH_CTRL			0x40
+#define M10BMC_PMCI_FLASH_WR_MODE		BIT(0)
+#define M10BMC_PMCI_FLASH_RD_MODE		BIT(1)
+#define M10BMC_PMCI_FLASH_BUSY			BIT(2)
+#define M10BMC_PMCI_FLASH_FIFO_SPACE		GENMASK(13, 4)
+#define M10BMC_PMCI_FLASH_READ_COUNT		GENMASK(25, 16)
+
+#define M10BMC_PMCI_FLASH_INT_US		1
+#define M10BMC_PMCI_FLASH_TIMEOUT_US		10000
+
+#define M10BMC_PMCI_FLASH_ADDR			0x44
+#define M10BMC_PMCI_FLASH_FIFO			0x800
+#define M10BMC_PMCI_READ_BLOCK_SIZE		0x800
+#define M10BMC_PMCI_FIFO_MAX_BYTES		0x800
+#define M10BMC_PMCI_FIFO_WORD_SIZE		4
+#define M10BMC_PMCI_FIFO_MAX_WORDS		(M10BMC_PMCI_FIFO_MAX_BYTES / \
+						 M10BMC_PMCI_FIFO_WORD_SIZE)
+
+#define M10BMC_PMCI_FLASH_MUX_CTRL	0x1d0
+#define FLASH_MUX_SELECTION		GENMASK(2, 0)
+#define FLASH_MUX_IDLE			0
+#define FLASH_MUX_NIOS			1
+#define FLASH_MUX_HOST			2
+#define FLASH_MUX_PFL			4
+#define get_flash_mux(mux)		FIELD_GET(FLASH_MUX_SELECTION, mux)
+
+#define FLASH_NIOS_REQUEST		BIT(4)
+#define FLASH_HOST_REQUEST		BIT(5)
+
+#define M10_FLASH_INT_US		1
+#define M10_FLASH_TIMEOUT_US		10000
+
 struct m10bmc_pmci_device {
 	void __iomem *base;
 	struct intel_m10bmc m10bmc;
+	struct mutex flash_mutex;	/* Prevent concurrent flash burst reads */
+};
+
+static void pmci_write_fifo(void __iomem *base, const u32 *buf, size_t count)
+{
+	while (count--)
+		writel(*buf++, base);
+}
+
+static void pmci_read_fifo(void __iomem *base, u32 *buf, size_t count)
+{
+	while (count--)
+		*buf++ = readl(base);
+}
+
+static u32 pmci_get_write_space(struct m10bmc_pmci_device *pmci)
+{
+	u32 val;
+	int ret;
+
+	ret = read_poll_timeout(readl, val,
+				FIELD_GET(M10BMC_PMCI_FLASH_FIFO_SPACE, val) ==
+				M10BMC_PMCI_FIFO_MAX_WORDS,
+				M10BMC_PMCI_FLASH_INT_US, M10BMC_PMCI_FLASH_TIMEOUT_US,
+				false, pmci->base + M10BMC_PMCI_FLASH_CTRL);
+	if (ret == -ETIMEDOUT)
+		return 0;
+
+	return FIELD_GET(M10BMC_PMCI_FLASH_FIFO_SPACE, val) * M10BMC_PMCI_FIFO_WORD_SIZE;
+}
+
+static int pmci_flash_bulk_write(struct intel_m10bmc *m10bmc, const u8 *buf, u32 size)
+{
+	struct m10bmc_pmci_device *pmci = container_of(m10bmc, struct m10bmc_pmci_device, m10bmc);
+	u32 blk_size, offset = 0, write_count;
+
+	while (size) {
+		blk_size = min(pmci_get_write_space(pmci), size);
+		if (blk_size == 0) {
+			dev_err(m10bmc->dev, "get FIFO available size fail\n");
+			return -EIO;
+		}
+
+		if (size < M10BMC_PMCI_FIFO_WORD_SIZE)
+			break;
+
+		write_count = blk_size / M10BMC_PMCI_FIFO_WORD_SIZE;
+		pmci_write_fifo(pmci->base + M10BMC_PMCI_FLASH_FIFO,
+				(u32 *)(buf + offset), write_count);
+
+		size -= blk_size;
+		offset += blk_size;
+	}
+
+	/* Handle remainder (less than M10BMC_PMCI_FIFO_WORD_SIZE bytes) */
+	if (size) {
+		u32 tmp = 0;
+
+		memcpy(&tmp, buf + offset, size);
+		pmci_write_fifo(pmci->base + M10BMC_PMCI_FLASH_FIFO, &tmp, 1);
+	}
+
+	return 0;
+}
+
+static int pmci_flash_bulk_read(struct intel_m10bmc *m10bmc, u8 *buf, u32 addr, u32 size)
+{
+	struct m10bmc_pmci_device *pmci = container_of(m10bmc, struct m10bmc_pmci_device, m10bmc);
+	u32 blk_size, offset = 0, val, full_read_count, read_count;
+	int ret;
+
+	while (size) {
+		blk_size = min_t(u32, size, M10BMC_PMCI_READ_BLOCK_SIZE);
+		full_read_count = blk_size / M10BMC_PMCI_FIFO_WORD_SIZE;
+
+		read_count = full_read_count;
+		if (full_read_count * M10BMC_PMCI_FIFO_WORD_SIZE < blk_size)
+			read_count++;
+
+		writel(addr + offset, pmci->base + M10BMC_PMCI_FLASH_ADDR);
+		writel(FIELD_PREP(M10BMC_PMCI_FLASH_READ_COUNT, read_count) |
+		       M10BMC_PMCI_FLASH_RD_MODE,
+		       pmci->base + M10BMC_PMCI_FLASH_CTRL);
+
+		ret = readl_poll_timeout((pmci->base + M10BMC_PMCI_FLASH_CTRL), val,
+					 !(val & M10BMC_PMCI_FLASH_BUSY),
+					 M10BMC_PMCI_FLASH_INT_US, M10BMC_PMCI_FLASH_TIMEOUT_US);
+		if (ret) {
+			dev_err(m10bmc->dev, "read timed out on reading flash 0x%xn", val);
+			return ret;
+		}
+
+		if (size < M10BMC_PMCI_FIFO_WORD_SIZE)
+			break;
+
+		pmci_read_fifo(pmci->base + M10BMC_PMCI_FLASH_FIFO,
+			       (u32 *)(buf + offset), full_read_count);
+
+		size -= blk_size;
+		offset += blk_size;
+
+		writel(0, pmci->base + M10BMC_PMCI_FLASH_CTRL);
+	}
+
+	/* Handle remainder (less than M10BMC_PMCI_FIFO_WORD_SIZE bytes) */
+	if (size) {
+		u32 tmp;
+
+		pmci_read_fifo(pmci->base + M10BMC_PMCI_FLASH_FIFO, &tmp, 1);
+		memcpy(buf + offset, &tmp, size);
+	}
+
+	return 0;
+}
+
+static int m10bmc_pmci_set_flash_host_mux(struct intel_m10bmc *m10bmc, bool request)
+{
+	u32 ctrl;
+	int ret;
+
+	ret = regmap_update_bits(m10bmc->regmap, M10BMC_PMCI_FLASH_MUX_CTRL,
+				 FLASH_HOST_REQUEST,
+				 FIELD_PREP(FLASH_HOST_REQUEST, request));
+	if (ret)
+		return ret;
+
+	return regmap_read_poll_timeout(m10bmc->regmap,
+					M10BMC_PMCI_FLASH_MUX_CTRL, ctrl,
+					request ? (get_flash_mux(ctrl) == FLASH_MUX_HOST) :
+						  (get_flash_mux(ctrl) != FLASH_MUX_HOST),
+					M10_FLASH_INT_US, M10_FLASH_TIMEOUT_US);
+}
+
+static int m10bmc_pmci_flash_write(struct intel_m10bmc *m10bmc, const u8 *buf, u32 offset, u32 size)
+{
+	return pmci_flash_bulk_write(m10bmc, buf + offset, size);
+}
+
+static int m10bmc_pmci_flash_read(struct intel_m10bmc *m10bmc, u8 *buf, u32 addr, u32 size)
+{
+	struct m10bmc_pmci_device *pmci = container_of(m10bmc, struct m10bmc_pmci_device, m10bmc);
+	int ret, ret2;
+
+	mutex_lock(&pmci->flash_mutex);
+	ret = m10bmc_pmci_set_flash_host_mux(m10bmc, true);
+	if (ret)
+		goto read_fail;
+
+	ret = pmci_flash_bulk_read(m10bmc, buf, addr, size);
+
+read_fail:
+	ret2 = m10bmc_pmci_set_flash_host_mux(m10bmc, false);
+	mutex_unlock(&pmci->flash_mutex);
+
+	if (ret)
+		return ret;
+	return ret2;
+}
+
+static const struct intel_m10bmc_flash_ops m10bmc_pmci_flash_ops = {
+	.read = m10bmc_pmci_flash_read,
+	.write = m10bmc_pmci_flash_write,
 };
 
 static const struct regmap_range m10bmc_pmci_regmap_range[] = {
@@ -75,6 +271,7 @@ static struct regmap_config m10bmc_pmci_regmap_config = {
 
 static struct mfd_cell m10bmc_pmci_n6000_bmc_subdevs[] = {
 	{ .name = "n6000bmc-hwmon" },
+	{ .name = "n6000bmc-sec-update" },
 };
 
 static const struct m10bmc_csr_map m10bmc_pmci_csr_map = {
@@ -112,6 +309,9 @@ static int m10bmc_pmci_probe(struct dfl_device *ddev)
 	if (!pmci)
 		return -ENOMEM;
 
+	mutex_init(&pmci->flash_mutex);
+	pmci->m10bmc.flash_ops = &m10bmc_pmci_flash_ops;
+
 	pmci->m10bmc.dev = dev;
 
 	pmci->base = devm_ioremap_resource(dev, &ddev->mmio_res);
@@ -128,6 +328,14 @@ static int m10bmc_pmci_probe(struct dfl_device *ddev)
 	return m10bmc_dev_init(&pmci->m10bmc, &m10bmc_pmci_n6000);
 }
 
+static void m10bmc_pmci_remove(struct dfl_device *ddev)
+{
+	struct intel_m10bmc *m10bmc = dev_get_drvdata(&ddev->dev);
+	struct m10bmc_pmci_device *pmci = container_of(m10bmc, struct m10bmc_pmci_device, m10bmc);
+
+	mutex_destroy(&pmci->flash_mutex);
+}
+
 #define FME_FEATURE_ID_M10BMC_PMCI	0x12
 
 static const struct dfl_device_id m10bmc_pmci_ids[] = {
@@ -143,6 +351,7 @@ static struct dfl_driver m10bmc_pmci_driver = {
 	},
 	.id_table = m10bmc_pmci_ids,
 	.probe    = m10bmc_pmci_probe,
+	.remove   = m10bmc_pmci_remove,
 };
 
 module_dfl_driver(m10bmc_pmci_driver);
-- 
2.30.2

[PATCH v2 11/11] mfd: intel-m10-bmc: Change MODULE_LICENSE() to GPL

[Ilpo Järvinen]

"GPL v2" should not be used as MODULE_LICENSE(). "GPL" is enough, see
commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL
v2" bogosity") for more details.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/mfd/intel-m10-bmc-core.c | 2 +-
 drivers/mfd/intel-m10-bmc-spi.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/intel-m10-bmc-core.c b/drivers/mfd/intel-m10-bmc-core.c
index 50a4ec758bdb..3b9e866b2bcf 100644
--- a/drivers/mfd/intel-m10-bmc-core.c
+++ b/drivers/mfd/intel-m10-bmc-core.c
@@ -130,4 +130,4 @@ EXPORT_SYMBOL_GPL(m10bmc_dev_init);
 
 MODULE_DESCRIPTION("Intel MAX 10 BMC core MFD driver");
 MODULE_AUTHOR("Intel Corporation");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/intel-m10-bmc-spi.c b/drivers/mfd/intel-m10-bmc-spi.c
index a17a0b50f352..1173f35cfcaa 100644
--- a/drivers/mfd/intel-m10-bmc-spi.c
+++ b/drivers/mfd/intel-m10-bmc-spi.c
@@ -266,5 +266,5 @@ module_spi_driver(intel_m10bmc_spi_driver);
 
 MODULE_DESCRIPTION("Intel MAX 10 BMC SPI bus interface");
 MODULE_AUTHOR("Intel Corporation");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
 MODULE_ALIAS("spi:intel-m10-bmc");
-- 
2.30.2

Re: [PATCH v2 07/11] regmap: indirect: Add indirect regmap support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2180 bytes --]

On Thu, Nov 17, 2022 at 02:05:11PM +0200, Ilpo Järvinen wrote:
> Add support for indirect register access via a regmap interface.
> 
> Indirect register access is a generic way to access registers indirectly.
> One use case is accessing registers on Intel FPGA IPs with e.g. PMCI or
> HSSI.

I can't tell from this changelog what exactly you're trying to
implement here...

> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Indirect Register Access.
> + *
> + * Copyright (C) 2020-2022 Intel Corporation, Inc.
> + */
> +#include <linux/debugfs.h>

Please make the entire comment a C++ one so things look more
intentional.

> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/seq_file.h>

I can't see what seq_file.h is used for, which is probably good
TBH since the interfaces it offers don't look like things I'd
expect a regmap bus to use.

> +static int indirect_bus_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 + ctx->indirect_cfg->cmd_offset);
> +	if (cmd != ctx->indirect_cfg->idle_cmd)
> +		dev_warn(ctx->dev, "residual cmd 0x%x on read entry\n", cmd);
> +
> +	writel(reg, ctx->base + ctx->indirect_cfg->addr_offset);
> +	writel(ctx->indirect_cfg->read_cmd, ctx->base + ctx->indirect_cfg->cmd_offset);
> +
> +	ret = readl_poll_timeout(ctx->base + ctx->indirect_cfg->ack_offset, ack,
> +				 (ack & ctx->indirect_cfg->ack_mask) == ctx->indirect_cfg->ack_mask,
> +				 ctx->indirect_cfg->sleep_us, ctx->indirect_cfg->timeout_us);

This all looks very specific to one particular implementation,
requiring a particular set of memory mapped registers and
operations - things like the initial read of the command for
example.  It's not clear to me how much reuse this is likely to
see outside of the one driver you're trying to add - if you want
to implement something device specific you can just provide
the custom operations in the device's regmap configuration rather
than having to provide a bus.  Why add a bus?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 07/11] regmap: indirect: Add indirect regmap support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3085 bytes --]

On Thu, 17 Nov 2022, Mark Brown wrote:

> On Thu, Nov 17, 2022 at 02:05:11PM +0200, Ilpo Järvinen wrote:
> > Add support for indirect register access via a regmap interface.
> > 
> > Indirect register access is a generic way to access registers indirectly.
> > One use case is accessing registers on Intel FPGA IPs with e.g. PMCI or
> > HSSI.
> 
> I can't tell from this changelog what exactly you're trying to
> implement here...
>
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Indirect Register Access.
> > + *
> > + * Copyright (C) 2020-2022 Intel Corporation, Inc.
> > + */
> > +#include <linux/debugfs.h>
> 
> Please make the entire comment a C++ one so things look more
> intentional.

Eh, all/most SPDX-License-Identifier lines are done like this one?!?

> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/regmap.h>
> > +#include <linux/seq_file.h>
> 
> I can't see what seq_file.h is used for, which is probably good
> TBH since the interfaces it offers don't look like things I'd
> expect a regmap bus to use.

Yeah, it seems it was not even the only useless header. I'll clean 
them up.

> > +static int indirect_bus_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 + ctx->indirect_cfg->cmd_offset);
> > +	if (cmd != ctx->indirect_cfg->idle_cmd)
> > +		dev_warn(ctx->dev, "residual cmd 0x%x on read entry\n", cmd);
> > +
> > +	writel(reg, ctx->base + ctx->indirect_cfg->addr_offset);
> > +	writel(ctx->indirect_cfg->read_cmd, ctx->base + ctx->indirect_cfg->cmd_offset);
> > +
> > +	ret = readl_poll_timeout(ctx->base + ctx->indirect_cfg->ack_offset, ack,
> > +				 (ack & ctx->indirect_cfg->ack_mask) == ctx->indirect_cfg->ack_mask,
> > +				 ctx->indirect_cfg->sleep_us, ctx->indirect_cfg->timeout_us);
> 
> This all looks very specific to one particular implementation,
> requiring a particular set of memory mapped registers and
> operations - things like the initial read of the command for
> example. It's not clear to me how much reuse this is likely to
> see outside of the one driver you're trying to add - if you want
> to implement something device specific you can just provide
> the custom operations in the device's regmap configuration rather
> than having to provide a bus.  Why add a bus?

The point of not doing it in a particular driver is that the users will 
be spread around more than into a single driver. This is a generic 
mechanism for accessing registers of IPs on Intel FPGA. The point being 
that IPs can use this common mechanism rather than each coming up their 
own way.

Mark Brown objected earlier naming it something related to Intel FPGAs [1]
but I certainly know it still fixes the operations like you note even if 
the offsets and values are now "customizable" (they weren't in the 
earliest versions of this patch).

[1] https://lore.kernel.org/all/YqB9O8HhZV2tXo8g@sirena.org.uk/T/#m75d4abdfd00f05866d837246ddc357a8af53cf99

-- 
 i.

Re: [PATCH v2 07/11] regmap: indirect: Add indirect regmap support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2754 bytes --]

On Thu, Nov 17, 2022 at 04:35:23PM +0200, Ilpo Järvinen wrote:
> On Thu, 17 Nov 2022, Mark Brown wrote:
> > On Thu, Nov 17, 2022 at 02:05:11PM +0200, Ilpo Järvinen wrote:

> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Indirect Register Access.
> > > + *
> > > + * Copyright (C) 2020-2022 Intel Corporation, Inc.
> > > + */
> > > +#include <linux/debugfs.h>

> > Please make the entire comment a C++ one so things look more
> > intentional.

> Eh, all/most SPDX-License-Identifier lines are done like this one?!?

The SPDX header has to be a C++ comment, please make the rest of
this a C++ comment.

> > > +	ret = readl_poll_timeout(ctx->base + ctx->indirect_cfg->ack_offset, ack,
> > > +				 (ack & ctx->indirect_cfg->ack_mask) == ctx->indirect_cfg->ack_mask,
> > > +				 ctx->indirect_cfg->sleep_us, ctx->indirect_cfg->timeout_us);

> > This all looks very specific to one particular implementation,
> > requiring a particular set of memory mapped registers and
> > operations - things like the initial read of the command for
> > example. It's not clear to me how much reuse this is likely to
> > see outside of the one driver you're trying to add - if you want
> > to implement something device specific you can just provide
> > the custom operations in the device's regmap configuration rather
> > than having to provide a bus.  Why add a bus?

> The point of not doing it in a particular driver is that the users will 
> be spread around more than into a single driver. This is a generic 
> mechanism for accessing registers of IPs on Intel FPGA. The point being 
> that IPs can use this common mechanism rather than each coming up their 
> own way.

You're saying that this is generic but it's really not looking
very generic at all, like I say there's a bunch of assumptions in
the code that look entirely specific to your implementation here.
Any abstraction and reusability seems extremely unclear, I'm not
seeing what this is doing that is diffrent to the driver using
this providing it's own register read and write operations.

> Mark Brown objected earlier naming it something related to Intel FPGAs [1]
> but I certainly know it still fixes the operations like you note even if 
> the offsets and values are now "customizable" (they weren't in the 
> earliest versions of this patch).

> [1] https://lore.kernel.org/all/YqB9O8HhZV2tXo8g@sirena.org.uk/T/#m75d4abdfd00f05866d837246ddc357a8af53cf99

No, what I'm objecting to there is pretty much the same thing I'm
saying here - this doesn't seem like it's a particularly generic
implementation and I'm really not clear that there'd be anything
meaningful left by the time the implementation assumptions are
removed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 07/11] regmap: indirect: Add indirect regmap support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3541 bytes --]

On Thu, 17 Nov 2022, Mark Brown wrote:

> On Thu, Nov 17, 2022 at 04:35:23PM +0200, Ilpo Järvinen wrote:
> > On Thu, 17 Nov 2022, Mark Brown wrote:
> > > On Thu, Nov 17, 2022 at 02:05:11PM +0200, Ilpo Järvinen wrote:

> > > > +	ret = readl_poll_timeout(ctx->base + ctx->indirect_cfg->ack_offset, ack,
> > > > +				 (ack & ctx->indirect_cfg->ack_mask) == ctx->indirect_cfg->ack_mask,
> > > > +				 ctx->indirect_cfg->sleep_us, ctx->indirect_cfg->timeout_us);
> 
> > > This all looks very specific to one particular implementation,
> > > requiring a particular set of memory mapped registers and
> > > operations - things like the initial read of the command for
> > > example. It's not clear to me how much reuse this is likely to
> > > see outside of the one driver you're trying to add - if you want
> > > to implement something device specific you can just provide
> > > the custom operations in the device's regmap configuration rather
> > > than having to provide a bus.  Why add a bus?
> 
> > The point of not doing it in a particular driver is that the users will 
> > be spread around more than into a single driver. This is a generic 
> > mechanism for accessing registers of IPs on Intel FPGA. The point being 
> > that IPs can use this common mechanism rather than each coming up their 
> > own way.
> 
> You're saying that this is generic but it's really not looking
> very generic at all, like I say there's a bunch of assumptions in
> the code that look entirely specific to your implementation here.
> Any abstraction and reusability seems extremely unclear, I'm not
> seeing what this is doing that is diffrent to the driver using
> this providing it's own register read and write operations.
>
> > Mark Brown objected earlier naming it something related to Intel FPGAs [1]
> > but I certainly know it still fixes the operations like you note even if 
> > the offsets and values are now "customizable" (they weren't in the 
> > earliest versions of this patch).
> 
> > [1] https://lore.kernel.org/all/YqB9O8HhZV2tXo8g@sirena.org.uk/T/#m75d4abdfd00f05866d837246ddc357a8af53cf99
> 
> No, what I'm objecting to there is pretty much the same thing I'm
> saying here - this doesn't seem like it's a particularly generic
> implementation and I'm really not clear that there'd be anything
> meaningful left by the time the implementation assumptions are
> removed.

That's probably because it sounds to me you're trying to extend its 
genericness beyond the domain where it's generic. That is, you're looking 
for genericness outside of IPs (that have their own driver each) in Intel 
FPGA domain.

Whether that is "generic" enough to reside in drivers/base/regmap can
of course be debated but lets say I put it into drivers/mfd/ along with 
the code currently using it. By doing that, we'll postpone this discussion 
to the point when the first driver using it outside of drivers/mfd/ comes 
by. At that point, having the indirect code in drivers/mfd/ is shown to 
be a wrong choice.

It's of course nothing that couldn't be fixed by patches moving the code 
around to some more preferred location. And that location likely turns out 
to be drivers/base/regmap, no? Or do you have a better place for it in 
that case?

Please also keep in mind that we're talking about an FPGA device here, a 
device that is capable of implementing other devices that fall under 
various drivers/xx/. Obviously each would have a driver of their own so
there is no as strong only single device/driver mapping here as you might 
be thinking.


-- 
 i.

Re: [PATCH v2 07/11] regmap: indirect: Add indirect regmap support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]

On Fri, Nov 18, 2022 at 02:49:45PM +0200, Ilpo Järvinen wrote:
> On Thu, 17 Nov 2022, Mark Brown wrote:

> > No, what I'm objecting to there is pretty much the same thing I'm
> > saying here - this doesn't seem like it's a particularly generic
> > implementation and I'm really not clear that there'd be anything
> > meaningful left by the time the implementation assumptions are
> > removed.

> That's probably because it sounds to me you're trying to extend its 
> genericness beyond the domain where it's generic. That is, you're looking 
> for genericness outside of IPs (that have their own driver each) in Intel 
> FPGA domain.

This just says it's adding "indirect regmap support" - there's
nothing here saying that it's some Intel specific thing but it's
quite specific to some IPs.  Perhaps you have some name for this
interface?  You're only adding one user here which isn't helping
make the case that this is something generic.

> Please also keep in mind that we're talking about an FPGA device here, a 
> device that is capable of implementing other devices that fall under 
> various drivers/xx/. Obviously each would have a driver of their own so
> there is no as strong only single device/driver mapping here as you might 
> be thinking.

I can't tell what you're trying to say here.  Are you saying that
this is somehow baked into some FPGA design so that it's memory
mapped with only a few registers showing to the rest of the
system rather than just having a substantial memory mapped
window like is typically used for FPGAs, but someohow this
register window stuff is implemented in the soft IP so people are
just throwng vaugely similar interfaces into a random host mapped
register layout?

Whatever's going on here this clearly isn't a generic
implementation of an indirect register map.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 07/11] regmap: indirect: Add indirect regmap support

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3413 bytes --]

On Fri, 18 Nov 2022, Mark Brown wrote:

> On Fri, Nov 18, 2022 at 02:49:45PM +0200, Ilpo Järvinen wrote:
> > On Thu, 17 Nov 2022, Mark Brown wrote:
> 
> > > No, what I'm objecting to there is pretty much the same thing I'm
> > > saying here - this doesn't seem like it's a particularly generic
> > > implementation and I'm really not clear that there'd be anything
> > > meaningful left by the time the implementation assumptions are
> > > removed.
> 
> > That's probably because it sounds to me you're trying to extend its 
> > genericness beyond the domain where it's generic. That is, you're looking 
> > for genericness outside of IPs (that have their own driver each) in Intel 
> > FPGA domain.
> 
> This just says it's adding "indirect regmap support" - there's
> nothing here saying that it's some Intel specific thing but it's
> quite specific to some IPs. 

Yeah, but it's that way mainly because of your earlier comments. :-)

I tried to make it more "generic" to the extent possible because of your 
concern related to genericness and I therefore intentionally put the Intel 
specific numbers into the other change.

Previously you were against saying it clearly that it's Intel FPGA 
specific when Matthew proposed changing the name to not sound something 
too generic. If you're ok with that now, I'm happy to make such change.

> Perhaps you have some name for this
> interface?  You're only adding one user here which isn't helping
> make the case that this is something generic.
>
> > Please also keep in mind that we're talking about an FPGA device here, a 
> > device that is capable of implementing other devices that fall under 
> > various drivers/xx/. Obviously each would have a driver of their own so
> > there is no as strong only single device/driver mapping here as you might 
> > be thinking.
> 
> I can't tell what you're trying to say here.  Are you saying that
> this is somehow baked into some FPGA design so that it's memory
> mapped with only a few registers showing to the rest of the
> system rather than just having a substantial memory mapped
> window like is typically used for FPGAs, but someohow this
> register window stuff is implemented in the soft IP so people are
> just throwng vaugely similar interfaces into a random host mapped
> register layout?

What I tried to say the users are not expected to be nicely confined into 
drivers/mfd/ (and a single driver in there).

You didn't answer at all my question about where to place the code?
I'm repeating it with the context below since you cut it off:


That's probably because it sounds to me you're trying to extend its 
genericness beyond the domain where it's generic. That is, you're looking 
for genericness outside of IPs (that have their own driver each) in Intel 
FPGA domain.

Whether that is "generic" enough to reside in drivers/base/regmap can
of course be debated but lets say I put it into drivers/mfd/ along with 
the code currently using it. By doing that, we'll postpone this discussion 
to the point when the first driver using it outside of drivers/mfd/ comes 
by. At that point, having the indirect code in drivers/mfd/ is shown to 
be a wrong choice.

It's of course nothing that couldn't be fixed by patches moving the code 
around to some more preferred location. And that location likely turns out 
to be drivers/base/regmap, no? Or do you have a better place for it in 
that case?


-- 
 i.

Re: [PATCH v2 00/11] intel-m10-bmc: Split BMC to core and SPI parts & add PMCI+N6000 support

[Xu Yilun]

On 2022-11-17 at 14:05:04 +0200, Ilpo Järvinen wrote:
> 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. This series also introduced regmap for
> indirect register access generic to Intel FPGA IPs.
> 
> 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.
> 
> I'm not entirely sure if I did properly address Yilun's comment on
> placement of the flash_ops related code. To me the original way which
> keeps them in mfd/intel-m10-bmc-{spi,pmci}.c still seems to the best
> way. If that is still not acceptable, I propose that I'll move just the
> flash ops functions into intel-m10-bmc-sec-update-{spi,pmci}.c and

I don't think the split of intel-m10-bmc-sec-update-{spi, pmci}.c is
needed. The mfd subdev is a platform device actually, it doesn't have to
know the bus type.

> create sec related platform info struct to select the correct flash
> ops. This would effectively be the "double split" approach I suggested
> earlier (both mfd and sec have their own per interface splits, to me
> the double split looks overkill but it would meet Yilun's goal of
> keeping sec related code within the sec driver).

Yes, this is still my preference.

My idea is, the secure update driver could be told by mfd core whether
there is a bypass channel for flash bulk read/write. If yes, use it. If
no, just direct access to flash staging areas as it previously does.

This is like:

struct intel_m10bmc {
        struct device *dev;
        struct regmap *regmap;
	...
+       const struct intel_m10bmc_flash_bulk_ops *flash_bulk_ops;
}

In intel-m10-bmc-pmci.c,

  +static const struct intel_m10bmc_flash_bulk_ops m10bmc_pmci_flash_bulk_ops = {
  +       .read = m10bmc_pmci_flash_bulk_read,
  +       .write = m10bmc_pmci_flash_bulk_write,
  };

In intel-m10-bmc-spi.c, no need to have flash_bulk_ops. 

In intel-m10-bmc-sec-update.c,

  Check if flash_bulk_ops is available, if yes,

    set_flash_host_mux(aquire)   /* I assume flash mux is dedicated for sec-update dev */
    sec->m10bmc->flash_bulk_ops->write()
    set_flash_host_mux(release)

  if no:
    follow previous direct access.

Thanks,
Yilun

> 
> The patch set is based on top of commit dfd10332596e ("fpga:
> m10bmc-sec: Fix kconfig dependencies") to avoid triggering a Kconfig
> conflict.
> 
> 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 (11):
>   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: Add flash ops for sec update
>   mfd: intel-m10-bmc: Downscope SPI defines & prefix with M10BMC_SPI
>   regmap: indirect: Add indirect regmap support
>   intel-m10-bmc: Add regmap_indirect_cfg for Intel FPGA IPs
>   mfd: intel-m10-bmc: Add PMCI driver
>   fpga: m10bmc-sec: Add support for N6000
>   mfd: intel-m10-bmc: Change MODULE_LICENSE() to GPL
> 
>  .../ABI/testing/sysfs-driver-intel-m10-bmc    |   8 +-
>  MAINTAINERS                                   |   2 +-
>  drivers/base/regmap/Kconfig                   |   3 +
>  drivers/base/regmap/Makefile                  |   1 +
>  drivers/base/regmap/regmap-indirect.c         | 128 +++++++
>  drivers/fpga/Kconfig                          |   2 +-
>  drivers/fpga/intel-m10-bmc-sec-update.c       | 149 ++++----
>  drivers/hwmon/Kconfig                         |   2 +-
>  drivers/mfd/Kconfig                           |  34 +-
>  drivers/mfd/Makefile                          |   6 +-
>  drivers/mfd/intel-m10-bmc-core.c              | 133 +++++++
>  drivers/mfd/intel-m10-bmc-pmci.c              | 361 ++++++++++++++++++
>  drivers/mfd/intel-m10-bmc-spi.c               | 270 +++++++++++++
>  drivers/mfd/intel-m10-bmc.c                   | 238 ------------
>  include/linux/mfd/intel-m10-bmc.h             | 122 +++---
>  include/linux/regmap.h                        |  55 +++
>  16 files changed, 1131 insertions(+), 383 deletions(-)
>  create mode 100644 drivers/base/regmap/regmap-indirect.c
>  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
> 

Re: [PATCH v2 07/11] regmap: indirect: Add indirect regmap support

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 3593 bytes --]

On Mon, Nov 21, 2022 at 03:37:40PM +0200, Ilpo Järvinen wrote:

> Previously you were against saying it clearly that it's Intel FPGA 
> specific when Matthew proposed changing the name to not sound something 
> too generic. If you're ok with that now, I'm happy to make such change.

Saying it's for some Intel FPGA just makes it sound like it should only
live within the driver for that FPGA.  The issue with there being only
one user:

> > Perhaps you have some name for this
> > interface?  You're only adding one user here which isn't helping
> > make the case that this is something generic.

is part of it, as is the fact that the naming is so very generic.  Even
"Intel FPGA" seems to be heading to the generic side, this is presumably
some specific thing rather than just something that everyone using a
FPGA from Intel is going to need.  The issue with this being overly
specific isn't just the name, it's the code as well.

> > I can't tell what you're trying to say here.  Are you saying that
> > this is somehow baked into some FPGA design so that it's memory
> > mapped with only a few registers showing to the rest of the
> > system rather than just having a substantial memory mapped
> > window like is typically used for FPGAs, but someohow this
> > register window stuff is implemented in the soft IP so people are
> > just throwng vaugely similar interfaces into a random host mapped
> > register layout?
> 
> What I tried to say the users are not expected to be nicely confined into 
> drivers/mfd/ (and a single driver in there).

So this interface is part of the physical IP surrounding the actual
programmable bit of a FPGA or something?  That doesn't seem entirely
right though given the fact that the registers are apparently one of the
things that gets moved around a lot.  I still have no idea what this
hardware actually looks like or what this code is trying to represent,
especially given the very few things that you are trying to
parameterise.  It's really not obvious there's even any point in trying
to share this code at the abstraction level you've gone for.

Do you have any examples of even a second user that isn't this one MFD
which you can share?

> You didn't answer at all my question about where to place the code?
> I'm repeating it with the context below since you cut it off:

I keep telling you to either make this so that it's actually generic or
just have register get/set operations in the regmap for the device using
it.  As things stand with the code you've sent there's a bunch of things
like the way it's doing direct MMIO (which means it only works on top of
memory mapped devices) and the absolute requirement for an idle command
and a wait for completion which clearly look like this is device specific.

> Whether that is "generic" enough to reside in drivers/base/regmap can
> of course be debated but lets say I put it into drivers/mfd/ along with 
> the code currently using it. By doing that, we'll postpone this discussion 
> to the point when the first driver using it outside of drivers/mfd/ comes 
> by. At that point, having the indirect code in drivers/mfd/ is shown to 
> be a wrong choice.

> It's of course nothing that couldn't be fixed by patches moving the code 
> around to some more preferred location. And that location likely turns out 
> to be drivers/base/regmap, no? Or do you have a better place for it in 
> that case?

If you think this should be shared via regmap then make it shareable.
That needs more work than just repainting the name.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 11/11] mfd: intel-m10-bmc: Change MODULE_LICENSE() to GPL

[Greg KH]

On Thu, Nov 17, 2022 at 02:05:15PM +0200, Ilpo Järvinen wrote:
> "GPL v2" should not be used as MODULE_LICENSE(). "GPL" is enough, see
> commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE "GPL" vs. "GPL
> v2" bogosity") for more details.

Same issue here, this is not needed at all, so please just leave it
alone or people will want to start to send this type of change out for
all old drivers.  That's not needed or wanted by anyone.

thanks,

greg k-h