linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support
@ 2020-08-05  8:00 Xu Yilun
  2020-08-05  8:00 ` [PATCH v3 1/2] regmap: add Intel SPI Slave to AVMM Bus Bridge support Xu Yilun
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Xu Yilun @ 2020-08-05  8:00 UTC (permalink / raw)
  To: broonie, lee.jones, linux-kernel
  Cc: trix, yilun.xu, matthew.gerlach, russell.h.weight, lgoncalv, hao.wu

This patchset adds the regmap-spi-avmm to support the Intel SPI Slave to
AVMM Bus Bridge (spi-avmm) IP block. It also implements the usercase - the
driver of Intel Max10 BMC chip which integrates this IP block.

Patch #1 implements the regmap-spi-avmm.
Patch #2 implements the mfd driver of Intel Max10 BMC chip.

Main changes from v1:
- Split out the regmap-spi-avmm module out of intel-m10-bmc module.

Main changes from v2:
- Refactor the code of regmap-spi-avmm.
   - Rewrites the rx flow and simplifies the implementation, collapse some
     function calls.
   - Add bounds checking every time we fill trans_buf & phy_buf.
   - Try to configure spi mode on regmap init
   - delete regmap_bus.reg_write/reg_read callbacks.
   - Squash the bug fixing patch.
- Add the sub devices in mfd_cell for Max10 bmc driver.
- Improve comments and some minor fixes.

Xu Yilun (2):
  regmap: add Intel SPI Slave to AVMM Bus Bridge support
  mfd: intel-m10-bmc: add Max10 BMC chip support for Intel FPGA PAC

 .../ABI/testing/sysfs-driver-intel-m10-bmc         |  15 +
 drivers/base/regmap/Kconfig                        |   6 +-
 drivers/base/regmap/Makefile                       |   1 +
 drivers/base/regmap/regmap-spi-avmm.c              | 735 +++++++++++++++++++++
 drivers/mfd/Kconfig                                |  13 +
 drivers/mfd/Makefile                               |   2 +
 drivers/mfd/intel-m10-bmc.c                        | 169 +++++
 include/linux/mfd/intel-m10-bmc.h                  |  57 ++
 include/linux/regmap.h                             |  36 +
 9 files changed, 1033 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
 create mode 100644 drivers/base/regmap/regmap-spi-avmm.c
 create mode 100644 drivers/mfd/intel-m10-bmc.c
 create mode 100644 include/linux/mfd/intel-m10-bmc.h

-- 
2.7.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/2] regmap: add Intel SPI Slave to AVMM Bus Bridge support
  2020-08-05  8:00 [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support Xu Yilun
@ 2020-08-05  8:00 ` Xu Yilun
  2020-08-05  8:00 ` [PATCH v3 2/2] mfd: intel-m10-bmc: add Max10 BMC chip support for Intel FPGA PAC Xu Yilun
  2020-08-17  8:24 ` [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support Xu Yilun
  2 siblings, 0 replies; 8+ messages in thread
From: Xu Yilun @ 2020-08-05  8:00 UTC (permalink / raw)
  To: broonie, lee.jones, linux-kernel
  Cc: trix, yilun.xu, matthew.gerlach, russell.h.weight, lgoncalv, hao.wu

This patch add support for regmap APIs that are intended to be used by
the drivers of some SPI slave chips which integrate the "SPI slave to
Avalon Master Bridge" (spi-avmm) IP.

The spi-avmm IP acts as a bridge to convert encoded streams of bytes
from the host to the chip's internal register read/write on Avalon bus.
The driver implements the register read/write operations for a generic
SPI master to access the sub devices behind spi-avmm bridge.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Reviewed-by: Tom Rix <trix@redhat.com>
Reviewed-by: Luis Claudio R. Goncalves <lgoncalv@redhat.com>
---
v2: Split out the regmap-spi-avmm as an independent module.
v3: rewrite the rx flow, now the driver reads one word and parse it
     immediately into trans_buf. This simplifies the rx handling.
    collapse some function calls.
    Add bounds checking every time we fill trans_buf & phy_buf.
    Try to configure spi mode on regmap init.
    delete regmap_bus.reg_write/reg_read callbacks.
    squash the bug fixing patch.
    improve comments and minor fixes.
---
 drivers/base/regmap/Kconfig           |   6 +-
 drivers/base/regmap/Makefile          |   1 +
 drivers/base/regmap/regmap-spi-avmm.c | 735 ++++++++++++++++++++++++++++++++++
 include/linux/regmap.h                |  36 ++
 4 files changed, 777 insertions(+), 1 deletion(-)
 create mode 100644 drivers/base/regmap/regmap-spi-avmm.c

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 1d1d26b..bcb90d8 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -4,7 +4,7 @@
 # subsystems should select the appropriate symbols.
 
 config REGMAP
-	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ || REGMAP_SOUNDWIRE || REGMAP_SCCB || REGMAP_I3C)
+	default y if (REGMAP_I2C || REGMAP_SPI || REGMAP_SPMI || REGMAP_W1 || REGMAP_AC97 || REGMAP_MMIO || REGMAP_IRQ || REGMAP_SOUNDWIRE || REGMAP_SCCB || REGMAP_I3C || REGMAP_SPI_AVMM)
 	select IRQ_DOMAIN if REGMAP_IRQ
 	bool
 
@@ -53,3 +53,7 @@ config REGMAP_SCCB
 config REGMAP_I3C
 	tristate
 	depends on I3C
+
+config REGMAP_SPI_AVMM
+	tristate
+	depends on SPI
diff --git a/drivers/base/regmap/Makefile b/drivers/base/regmap/Makefile
index ff6c7d8..ac1b69e 100644
--- a/drivers/base/regmap/Makefile
+++ b/drivers/base/regmap/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_REGMAP_W1) += regmap-w1.o
 obj-$(CONFIG_REGMAP_SOUNDWIRE) += regmap-sdw.o
 obj-$(CONFIG_REGMAP_SCCB) += regmap-sccb.o
 obj-$(CONFIG_REGMAP_I3C) += regmap-i3c.o
+obj-$(CONFIG_REGMAP_SPI_AVMM) += regmap-spi-avmm.o
diff --git a/drivers/base/regmap/regmap-spi-avmm.c b/drivers/base/regmap/regmap-spi-avmm.c
new file mode 100644
index 0000000..7aec297
--- /dev/null
+++ b/drivers/base/regmap/regmap-spi-avmm.c
@@ -0,0 +1,735 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Register map access API - SPI AVMM support
+//
+// Copyright (C) 2018-2020 Intel Corporation. All rights reserved.
+
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+/*
+ * This driver implements the regmap operations for a generic SPI
+ * master to access the registers of the spi slave chip which has an
+ * Avalone bus in it.
+ *
+ * The "SPI slave to Avalon Master Bridge" (spi-avmm) IP should be integrated
+ * in the spi slave chip. The IP acts as a bridge to convert encoded streams of
+ * bytes from the host to the internal register read/write on Avalon bus. In
+ * order to issue register access requests to the slave chip, the host should
+ * send formatted bytes that conform to the transfer protocol.
+ * The transfer protocol contains 3 layers: transaction layer, packet layer
+ * and physical layer.
+ *
+ * Reference Documents could be found at:
+ * https://www.intel.com/content/www/us/en/programmable/documentation/sfo1400787952932.html
+ *
+ * Chapter "SPI Slave/JTAG to Avalon Master Bridge Cores" is a general
+ * introduction to the protocol.
+ *
+ * Chapter "Avalon Packets to Transactions Converter Core" describes
+ * the transaction layer.
+ *
+ * Chapter "Avalon-ST Bytes to Packets and Packets to Bytes Converter Cores"
+ * describes the packet layer.
+ *
+ * Chapter "Avalon-ST Serial Peripheral Interface Core" describes the
+ * physical layer.
+ *
+ *
+ * When host issues a regmap read/write, the driver will transform the request
+ * to byte stream layer by layer. It formats the register addr, value and
+ * length to the transaction layer request, then converts the request to packet
+ * layer bytes stream and then to physical layer bytes stream. Finally the
+ * driver sends the formatted byte stream over SPI bus to the slave chip.
+ *
+ * The spi-avmm IP on the slave chip decodes the byte stream and initiates
+ * register read/write on its internal Avalon bus, and then encodes the
+ * response to byte stream and sends back to host.
+ *
+ * The driver receives the byte stream, reverses the 3 layers transformation,
+ * and finally gets the response value (read out data for register read,
+ * successful written size for register write).
+ */
+
+#define PKT_SOP			0x7a
+#define PKT_EOP			0x7b
+#define PKT_CHANNEL		0x7c
+#define PKT_ESC			0x7d
+
+#define PHY_IDLE		0x4a
+#define PHY_ESC			0x4d
+
+#define TRANS_CODE_WRITE	0x0
+#define TRANS_CODE_SEQ_WRITE	0x4
+#define TRANS_CODE_READ		0x10
+#define TRANS_CODE_SEQ_READ	0x14
+#define TRANS_CODE_NO_TRANS	0x7f
+
+#define SPI_AVMM_XFER_TIMEOUT	(msecs_to_jiffies(200))
+
+/* slave's register addr is 32 bits */
+#define SPI_AVMM_REG_SIZE		4UL
+/* slave's register value is 32 bits */
+#define SPI_AVMM_VAL_SIZE		4UL
+
+/*
+ * max rx size could be larger. But considering the buffer consuming,
+ * it is proper that we limit 1KB xfer at max.
+ */
+#define MAX_READ_CNT		256UL
+#define MAX_WRITE_CNT		1UL
+
+struct trans_req_header {
+	u8 code;
+	u8 rsvd;
+	__be16 size;
+	__be32 addr;
+} __packed;
+
+struct trans_resp_header {
+	u8 r_code;
+	u8 rsvd;
+	__be16 size;
+} __packed;
+
+#define TRANS_REQ_HD_SIZE	(sizeof(struct trans_req_header))
+#define TRANS_RESP_HD_SIZE	(sizeof(struct trans_resp_header))
+
+/*
+ * In transaction layer,
+ * the write request format is: Transaction request header + data
+ * the read request format is: Transaction request header
+ * the write response format is: Transaction response header
+ * the read response format is: pure data, no Transaction response header
+ */
+#define TRANS_WR_TX_SIZE(n)	(TRANS_REQ_HD_SIZE + SPI_AVMM_VAL_SIZE * (n))
+#define TRANS_RD_TX_SIZE	TRANS_REQ_HD_SIZE
+#define TRANS_TX_MAX		TRANS_WR_TX_SIZE(MAX_WRITE_CNT)
+
+#define TRANS_RD_RX_SIZE(n)	(SPI_AVMM_VAL_SIZE * (n))
+#define TRANS_WR_RX_SIZE	TRANS_RESP_HD_SIZE
+#define TRANS_RX_MAX		TRANS_RD_RX_SIZE(MAX_READ_CNT)
+
+/* tx & rx share one transaction layer buffer */
+#define TRANS_BUF_SIZE		((TRANS_TX_MAX > TRANS_RX_MAX) ?	\
+				 TRANS_TX_MAX : TRANS_RX_MAX)
+
+/*
+ * In tx phase, the host prepares all the phy layer bytes of a request in the
+ * phy buffer and sends them in a batch.
+ *
+ * The packet layer and physical layer defines several special chars for
+ * various purpose, when a transaction layer byte hits one of these special
+ * chars, it should be escaped. The escape rule is, "Escape char first,
+ * following the byte XOR'ed with 0x20".
+ *
+ * This macro defines the max possible length of the phy data. In the worst
+ * case, all transaction layer bytes need to be escaped (so the data length
+ * doubles), plus 4 special chars (SOP, CHANNEL, CHANNEL_NUM, EOP). Finally
+ * we should make sure the length is aligned to SPI BPW.
+ */
+#define PHY_TX_MAX		ALIGN(2 * TRANS_TX_MAX + 4, 4)
+
+/*
+ * Unlike tx, phy rx is affected by possible PHY_IDLE bytes from slave, the max
+ * length of the rx bit stream is unpredictable. So the driver reads the words
+ * one by one, and parses each word immediately into transaction layer buffer.
+ * Only one word length of phy buffer is used for rx.
+ */
+#define PHY_BUF_SIZE		PHY_TX_MAX
+
+/**
+ * struct spi_avmm_bridge - SPI slave to AVMM bus master bridge
+ *
+ * @spi: spi slave associated with this bridge.
+ * @word_len: bytes of word for spi transfer.
+ * @trans_len: length of valid data in trans_buf.
+ * @phy_len: length of valid data in phy_buf.
+ * @trans_buf: the bridge buffer for transaction layer data.
+ * @phy_buf: the bridge buffer for physical layer data.
+ * @swap_words: the word swapping cb for phy data. NULL if not needed.
+ *
+ * As a device's registers are implemented on the AVMM bus address space, it
+ * requires the driver to issue formatted requests to spi slave to AVMM bus
+ * master bridge to perform register access.
+ */
+struct spi_avmm_bridge {
+	struct spi_device *spi;
+	unsigned char word_len;
+	unsigned int trans_len;
+	unsigned int phy_len;
+	/* bridge buffer used in translation between protocol layers */
+	char trans_buf[TRANS_BUF_SIZE];
+	char phy_buf[PHY_BUF_SIZE];
+	void (*swap_words)(char *buf, unsigned int len);
+};
+
+static void br_swap_words_32(char *buf, unsigned int len)
+{
+	u32 *p = (u32 *)buf;
+	unsigned int count;
+
+	count = len / 4;
+	while (count--) {
+		*p = swab32p(p);
+		p++;
+	}
+}
+
+/*
+ * Format transaction layer data in br->trans_buf according to the register
+ * access request, Store valid transaction layer data length in br->trans_len.
+ */
+static int br_trans_tx_prepare(struct spi_avmm_bridge *br, bool is_read, u32 reg,
+			       u32 *wr_val, u32 count)
+{
+	struct trans_req_header *header;
+	unsigned int trans_len;
+	u8 code;
+	__le32 *data;
+	int i;
+
+	if (is_read) {
+		if (count == 1)
+			code = TRANS_CODE_READ;
+		else
+			code = TRANS_CODE_SEQ_READ;
+	} else {
+		if (count == 1)
+			code = TRANS_CODE_WRITE;
+		else
+			code = TRANS_CODE_SEQ_WRITE;
+	}
+
+	header = (struct trans_req_header *)br->trans_buf;
+	header->code = code;
+	header->rsvd = 0;
+	header->size = cpu_to_be16((u16)count * SPI_AVMM_VAL_SIZE);
+	header->addr = cpu_to_be32(reg);
+
+	trans_len = TRANS_REQ_HD_SIZE;
+
+	if (!is_read) {
+		trans_len += SPI_AVMM_VAL_SIZE * count;
+		if (trans_len > sizeof(br->trans_buf))
+			return -ENOMEM;
+
+		data = (__le32 *)(br->trans_buf + TRANS_REQ_HD_SIZE);
+
+		for (i = 0; i < count; i++)
+			*data++ = cpu_to_le32(*wr_val++);
+	}
+
+	/* Store valid trans data length for next layer */
+	br->trans_len = trans_len;
+
+	return 0;
+}
+
+/*
+ * Convert transaction layer data (in br->trans_buf) to phy layer data, store
+ * them in br->phy_buf. Store valid phy layer data length in br->phy_len.
+ */
+static int br_pkt_phy_tx_prepare(struct spi_avmm_bridge *br)
+{
+	bool need_esc = false, eop_inserted = false;
+	char *tb, *tb_end, *pb, *pb_limit;
+
+	tb = br->trans_buf;
+	tb_end = tb + br->trans_len;
+	pb = br->phy_buf;
+	pb_limit = pb + ARRAY_SIZE(br->phy_buf);
+
+	*pb++ = PKT_SOP;
+
+	/*
+	 * The driver doesn't support multiple channels so the channel number
+	 * is always 0.
+	 */
+	*pb++ = PKT_CHANNEL;
+	*pb++ = 0x0;
+
+	for (; pb < pb_limit && tb < tb_end; pb++) {
+		if (need_esc) {
+			*pb = *tb++ ^ 0x20;
+			need_esc = false;
+			continue;
+		}
+
+		/* EOP should be inserted before the last valid char */
+		if (tb == tb_end - 1 && !eop_inserted) {
+			*pb = PKT_EOP;
+			eop_inserted = true;
+			continue;
+		}
+
+		/*
+		 * insert an ESCAPE char if the data value equals any special
+		 * char.
+		 */
+		switch (*tb) {
+		case PKT_SOP:
+		case PKT_EOP:
+		case PKT_CHANNEL:
+		case PKT_ESC:
+			*pb = PKT_ESC;
+			need_esc = true;
+			break;
+		case PHY_IDLE:
+		case PHY_ESC:
+			*pb = PHY_ESC;
+			need_esc = true;
+			break;
+		default:
+			*pb = *tb++;
+			break;
+		}
+	}
+
+	/* The phy buffer is used out but transaction layer data remains */
+	if (tb < tb_end)
+		return -ENOMEM;
+
+	/* Store valid phy data length for spi transfer */
+	br->phy_len = pb - br->phy_buf;
+
+	return 0;
+}
+
+/*
+ * tx_buf len should be aligned with SPI's BPW. Spare bytes should be padded
+ * with PHY_IDLE, then the slave will just drop them.
+ *
+ * The driver will not simply pad 4a at the tail. The concern is that driver
+ * will not store MISO data during tx phase, if the driver pads 4a at the tail,
+ * it is possible that if the slave is fast enough to response at the padding
+ * time. As a result these rx bytes are lost. In the following case, 7a,7c,00
+ * will lost.
+ * MOSI ...|7a|7c|00|10| |00|00|04|02| |4b|7d|5a|7b| |40|4a|4a|4a| |XX|XX|...
+ * MISO ...|4a|4a|4a|4a| |4a|4a|4a|4a| |4a|4a|4a|4a| |4a|7a|7c|00| |78|56|...
+ *
+ * So the driver moves EOP and bytes after EOP to the end of the aligned size,
+ * then fill the hole with PHY_IDLE. As following:
+ * before pad ...|7a|7c|00|10| |00|00|04|02| |4b|7d|5a|7b| |40|
+ * after pad  ...|7a|7c|00|10| |00|00|04|02| |4b|7d|5a|4a| |4a|4a|7b|40|
+ * Then if the slave will not get the entire packet before the tx phase is
+ * over, it can't responsed to anything either.
+ */
+static int br_phy_buf_pad(struct spi_avmm_bridge *br)
+{
+	char *pb = &br->phy_buf[br->phy_len - 1], *dst_pb;
+	unsigned int aligned_len;
+
+	aligned_len = ALIGN(br->phy_len, br->word_len);
+	if (aligned_len > sizeof(br->phy_buf))
+		return -ENOMEM;
+
+	if (aligned_len == br->phy_len)
+		return 0;
+
+	dst_pb = &br->phy_buf[aligned_len - 1];
+
+	/* move EOP and bytes after EOP to the end of aligned size */
+	for (; pb > br->phy_buf; pb--, dst_pb--) {
+		*dst_pb = *pb;
+
+		if (*pb == PKT_EOP)
+			break;
+	}
+
+	/* fill the hole with PHY_IDLEs */
+	while (pb < dst_pb)
+		*pb++ = PHY_IDLE;
+
+	/* update the phy data length */
+	br->phy_len = aligned_len;
+
+	return 0;
+}
+
+/*
+ * In tx phase, the slave only returns PHY_IDLE (0x4a). So the driver will
+ * ignore rx in tx phase.
+ */
+static int br_do_tx(struct spi_avmm_bridge *br)
+{
+	/* reorder words for spi transfer */
+	if (br->swap_words)
+		br->swap_words(br->phy_buf, br->phy_len);
+
+	/* send all data in phy_buf  */
+	return spi_write(br->spi, br->phy_buf, br->phy_len);
+}
+
+/*
+ * This function read the rx byte stream from SPI word by word and convert
+ * them to transaction layer data in br->trans_buf. It also stores the length
+ * of rx transaction layer data in br->trans_len
+ *
+ * The slave may send an unknown number of PHY_IDLEs in rx phase, so we cannot
+ * prepare a fixed length buffer to receive all of the rx data in a batch. We
+ * have to read word by word and convert them to transaction layer data at
+ * once.
+ */
+static int br_do_rx_and_pkt_phy_parse(struct spi_avmm_bridge *br)
+{
+	bool eop_found = false, channel_found = false, esc_found = false;
+	bool valid_word = false, last_try = false;
+	struct device *dev = &br->spi->dev;
+	char *pb, *tb_limit, *tb = NULL;
+	unsigned long poll_timeout;
+	int ret, i;
+
+	tb_limit = br->trans_buf + ARRAY_SIZE(br->trans_buf);
+	pb = br->phy_buf;
+	poll_timeout = jiffies + SPI_AVMM_XFER_TIMEOUT;
+	while (tb < tb_limit) {
+		ret = spi_read(br->spi, pb, br->word_len);
+		if (ret)
+			return ret;
+
+		/* reorder the word back */
+		if (br->swap_words)
+			br->swap_words(pb, br->word_len);
+
+		valid_word = false;
+		for (i = 0; i < br->word_len; i++) {
+			/* drop everything before first SOP */
+			if (!tb && pb[i] != PKT_SOP)
+				continue;
+
+			/* drop PHY_IDLE */
+			if (pb[i] == PHY_IDLE)
+				continue;
+
+			valid_word = true;
+
+			/*
+			 * We don't support multiple channels, so error out if
+			 * a non-zero channel number is found.
+			 */
+			if (channel_found) {
+				if (pb[i] != 0) {
+					dev_err(dev, "%s channel num != 0\n",
+						__func__);
+					return -EFAULT;
+				}
+
+				channel_found = false;
+				continue;
+			}
+
+			switch (pb[i]) {
+			case PKT_SOP:
+				/*
+				 * reset the parsing if a second SOP appears.
+				 */
+				tb = br->trans_buf;
+				eop_found = false;
+				channel_found = false;
+				esc_found = false;
+				break;
+			case PKT_EOP:
+				/*
+				 * No special char is expected after ESC char.
+				 * No special char (except ESC & PHY_IDLE) is
+				 * expected after EOP char.
+				 *
+				 * The special chars are all dropped.
+				 */
+				if (esc_found || eop_found)
+					return -EFAULT;
+
+				eop_found = true;
+				break;
+			case PKT_CHANNEL:
+				if (esc_found || eop_found)
+					return -EFAULT;
+
+				channel_found = true;
+				break;
+			case PKT_ESC:
+			case PHY_ESC:
+				if (esc_found)
+					return -EFAULT;
+
+				esc_found = true;
+				break;
+			default:
+				/* Record the normal byte in trans_buf. */
+				if (esc_found) {
+					*tb++ = pb[i] ^ 0x20;
+					esc_found = false;
+				} else {
+					*tb++ = pb[i];
+				}
+
+				/*
+				 * We get the last normal byte after EOP, it is
+				 * time we finish. Normally the function should
+				 * return here.
+				 */
+				if (eop_found) {
+					br->trans_len = tb - br->trans_buf;
+					return 0;
+				}
+			}
+		}
+
+		if (valid_word) {
+			/* update poll timeout when we get valid word */
+			poll_timeout = jiffies + SPI_AVMM_XFER_TIMEOUT;
+			last_try = false;
+		} else {
+			/*
+			 * We timeout when rx keeps invalid for some time. But
+			 * it is possible we are scheduled out for long time
+			 * after a spi_read. So when we are scheduled in, a SW
+			 * timeout happens. But actually HW may have worked fine and
+			 * has been ready long time ago. So we need to do an extra
+			 * read, if we get a valid word then we could continue rx,
+			 * otherwise real a HW issue happens.
+			 */
+			if (last_try)
+				return -ETIMEDOUT;
+
+			if (time_after(jiffies, poll_timeout))
+				last_try = true;
+		}
+	}
+
+	/*
+	 * We have used out all transfer layer buffer but cannot find the end
+	 * of the byte stream.
+	 */
+	dev_err(dev, "%s transfer buffer is full but rx doesn't end\n",
+		__func__);
+
+	return -EFAULT;
+}
+
+/*
+ * For read transactions, the avmm bus will directly return register values
+ * without transaction response header.
+ */
+static int br_rd_trans_rx_parse(struct spi_avmm_bridge *br,
+				u32 *val, unsigned int expected_count)
+{
+	unsigned int i, trans_len = br->trans_len;
+	__le32 *data;
+
+	if (expected_count * SPI_AVMM_VAL_SIZE != trans_len)
+		return -EFAULT;
+
+	data = (__le32 *)br->trans_buf;
+	for (i = 0; i < expected_count; i++)
+		*val++ = le32_to_cpu(*data++);
+
+	return 0;
+}
+
+/*
+ * For write transactions, the slave will return a transaction response
+ * header.
+ */
+static int br_wr_trans_rx_parse(struct spi_avmm_bridge *br,
+				unsigned int expected_count)
+{
+	unsigned int trans_len = br->trans_len;
+	struct trans_resp_header *resp;
+	u8 code;
+	u16 val_len;
+
+	if (trans_len != TRANS_RESP_HD_SIZE)
+		return -EFAULT;
+
+	resp = (struct trans_resp_header *)br->trans_buf;
+
+	code = resp->r_code ^ 0x80;
+	val_len = be16_to_cpu(resp->size);
+	if (!val_len || val_len != expected_count * SPI_AVMM_VAL_SIZE)
+		return -EFAULT;
+
+	/* error out if the trans code doesn't align with the val size */
+	if ((val_len == SPI_AVMM_VAL_SIZE && code != TRANS_CODE_WRITE) ||
+	    (val_len > SPI_AVMM_VAL_SIZE && code != TRANS_CODE_SEQ_WRITE))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int do_reg_access(void *context, bool is_read, unsigned int reg,
+			 unsigned int *value, unsigned int count)
+{
+	struct spi_avmm_bridge *br = context;
+	int ret;
+
+	/* invalidate bridge buffers first */
+	br->trans_len = 0;
+	br->phy_len = 0;
+
+	ret = br_trans_tx_prepare(br, is_read, reg, value, count);
+	if (ret)
+		return ret;
+
+	ret = br_pkt_phy_tx_prepare(br);
+	if (ret)
+		return ret;
+
+	if (br->word_len != 1) {
+		ret = br_phy_buf_pad(br);
+		if (ret)
+			return ret;
+	}
+
+	ret = br_do_tx(br);
+	if (ret)
+		return ret;
+
+	ret = br_do_rx_and_pkt_phy_parse(br);
+	if (ret)
+		return ret;
+
+	if (is_read)
+		return br_rd_trans_rx_parse(br, value, count);
+	else
+		return br_wr_trans_rx_parse(br, count);
+}
+
+static int regmap_spi_avmm_gather_write(void *context,
+					const void *reg_buf, size_t reg_len,
+					const void *val_buf, size_t val_len)
+{
+	if (reg_len != SPI_AVMM_REG_SIZE)
+		return -EINVAL;
+
+	if (!IS_ALIGNED(val_len, SPI_AVMM_VAL_SIZE))
+		return -EINVAL;
+
+	return do_reg_access(context, false, *(u32 *)reg_buf, (u32 *)val_buf,
+			     val_len / SPI_AVMM_VAL_SIZE);
+}
+
+static int regmap_spi_avmm_write(void *context, const void *data, size_t bytes)
+{
+	if (bytes < SPI_AVMM_REG_SIZE + SPI_AVMM_VAL_SIZE)
+		return -EINVAL;
+
+	return regmap_spi_avmm_gather_write(context, data, SPI_AVMM_REG_SIZE,
+					    data + SPI_AVMM_REG_SIZE,
+					    bytes - SPI_AVMM_REG_SIZE);
+}
+
+static int regmap_spi_avmm_read(void *context,
+				const void *reg_buf, size_t reg_len,
+				void *val_buf, size_t val_len)
+{
+	if (reg_len != SPI_AVMM_REG_SIZE)
+		return -EINVAL;
+
+	if (!IS_ALIGNED(val_len, SPI_AVMM_VAL_SIZE))
+		return -EINVAL;
+
+	return do_reg_access(context, true, *(u32 *)reg_buf, val_buf,
+			     (val_len / SPI_AVMM_VAL_SIZE));
+}
+
+static struct spi_avmm_bridge *
+spi_avmm_bridge_ctx_gen(struct spi_device *spi)
+{
+	struct spi_avmm_bridge *br;
+
+	if (!spi)
+		return ERR_PTR(-ENODEV);
+
+	/* Only support BPW == 8 or 32 now. Try 32 BPW first. */
+	spi->mode = SPI_MODE_1;
+	spi->bits_per_word = 32;
+	if (spi_setup(spi)) {
+		spi->bits_per_word = 8;
+		if (spi_setup(spi))
+			return ERR_PTR(-EINVAL);
+	}
+
+	br = kzalloc(sizeof(*br), GFP_KERNEL);
+	if (!br)
+		return ERR_PTR(-ENOMEM);
+
+	br->spi = spi;
+	br->word_len = spi->bits_per_word / 8;
+	if (br->word_len == 4) {
+		/*
+		 * The protocol requires little endian byte order but MSB
+		 * first. So driver needs to swap the byte order word by word
+		 * if word length > 1.
+		 */
+		br->swap_words = br_swap_words_32;
+	}
+
+	return br;
+}
+
+static void spi_avmm_bridge_ctx_free(void *context)
+{
+	kfree(context);
+}
+
+static const struct regmap_bus regmap_spi_avmm_bus = {
+	.write = regmap_spi_avmm_write,
+	.gather_write = regmap_spi_avmm_gather_write,
+	.read = regmap_spi_avmm_read,
+	.reg_format_endian_default = REGMAP_ENDIAN_NATIVE,
+	.val_format_endian_default = REGMAP_ENDIAN_NATIVE,
+	.max_raw_read = SPI_AVMM_VAL_SIZE * MAX_READ_CNT,
+	.max_raw_write = SPI_AVMM_VAL_SIZE * MAX_WRITE_CNT,
+	.free_context = spi_avmm_bridge_ctx_free,
+};
+
+struct regmap *__regmap_init_spi_avmm(struct spi_device *spi,
+				      const struct regmap_config *config,
+				      struct lock_class_key *lock_key,
+				      const char *lock_name)
+{
+	struct spi_avmm_bridge *bridge;
+	struct regmap *map;
+
+	bridge = spi_avmm_bridge_ctx_gen(spi);
+	if (IS_ERR(bridge))
+		return ERR_CAST(bridge);
+
+	map = __regmap_init(&spi->dev, &regmap_spi_avmm_bus,
+			    bridge, config, lock_key, lock_name);
+	if (IS_ERR(map)) {
+		spi_avmm_bridge_ctx_free(bridge);
+		return ERR_CAST(map);
+	}
+
+	return map;
+}
+EXPORT_SYMBOL_GPL(__regmap_init_spi_avmm);
+
+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 spi_avmm_bridge *bridge;
+	struct regmap *map;
+
+	bridge = spi_avmm_bridge_ctx_gen(spi);
+	if (IS_ERR(bridge))
+		return ERR_CAST(bridge);
+
+	map = __devm_regmap_init(&spi->dev, &regmap_spi_avmm_bus,
+				 bridge, config, lock_key, lock_name);
+	if (IS_ERR(map)) {
+		spi_avmm_bridge_ctx_free(bridge);
+		return ERR_CAST(map);
+	}
+
+	return map;
+}
+EXPORT_SYMBOL_GPL(__devm_regmap_init_spi_avmm);
+
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 1970ed5..d865d8f 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -567,6 +567,10 @@ struct regmap *__regmap_init_sdw(struct sdw_slave *sdw,
 				 const struct regmap_config *config,
 				 struct lock_class_key *lock_key,
 				 const char *lock_name);
+struct regmap *__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(struct device *dev,
 				  const struct regmap_bus *bus,
@@ -620,6 +624,10 @@ struct regmap *__devm_regmap_init_i3c(struct i3c_device *i3c,
 				 const struct regmap_config *config,
 				 struct lock_class_key *lock_key,
 				 const char *lock_name);
+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);
 /*
  * Wrapper for regmap_init macros to include a unique lockdep key and name
  * for each call. No-op if CONFIG_LOCKDEP is not set.
@@ -806,6 +814,19 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
 	__regmap_lockdep_wrapper(__regmap_init_sdw, #config,		\
 				sdw, config)
 
+/**
+ * regmap_init_spi_avmm() - Initialize register map for Intel SPI Slave
+ * to AVMM Bus Bridge
+ *
+ * @spi: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a struct regmap.
+ */
+#define regmap_init_spi_avmm(spi, config)					\
+	__regmap_lockdep_wrapper(__regmap_init_spi_avmm, #config,		\
+				 spi, config)
 
 /**
  * devm_regmap_init() - Initialise managed register map
@@ -993,6 +1014,21 @@ bool regmap_ac97_default_volatile(struct device *dev, unsigned int reg);
 	__regmap_lockdep_wrapper(__devm_regmap_init_i3c, #config,	\
 				i3c, config)
 
+/**
+ * devm_regmap_init_spi_avmm() - Initialize register map for Intel SPI Slave
+ * to AVMM Bus Bridge
+ *
+ * @spi: Device that will be interacted with
+ * @config: Configuration for register map
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to a struct regmap.  The map will be automatically freed by the
+ * device management code.
+ */
+#define devm_regmap_init_spi_avmm(spi, config)				\
+	__regmap_lockdep_wrapper(__devm_regmap_init_spi_avmm, #config,	\
+				 spi, 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.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/2] mfd: intel-m10-bmc: add Max10 BMC chip support for Intel FPGA PAC
  2020-08-05  8:00 [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support Xu Yilun
  2020-08-05  8:00 ` [PATCH v3 1/2] regmap: add Intel SPI Slave to AVMM Bus Bridge support Xu Yilun
@ 2020-08-05  8:00 ` Xu Yilun
  2020-08-17  8:24 ` [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support Xu Yilun
  2 siblings, 0 replies; 8+ messages in thread
From: Xu Yilun @ 2020-08-05  8:00 UTC (permalink / raw)
  To: broonie, lee.jones, linux-kernel
  Cc: trix, yilun.xu, matthew.gerlach, russell.h.weight, lgoncalv, hao.wu

This patch implements the basic functions of the BMC chip for some Intel
FPGA PCIe Acceleration Cards (PAC). The BMC is implemented using the
intel max10 CPLD.

This BMC chip is connected to FPGA by a SPI bus. To provide direct
register access from FPGA, the "SPI slave to Avalon Master Bridge"
(spi-avmm) IP is integrated in the chip. It converts encoded streams of
bytes from the host to the internal register read/write on Avalon bus.
So This driver uses the regmap-spi-avmm for register accessing.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Signed-off-by: Wu Hao <hao.wu@intel.com>
Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Reviewed-by: Tom Rix <trix@redhat.com>
---
v2: Split out the regmap-spi-avmm part.
    Rename the file intel-m10-bmc-main.c to intel-m10-bmc.c, cause
     there is only one source file left for this module now.
v3: add the sub devices in mfd_cell.
    some minor fixes.
---
 .../ABI/testing/sysfs-driver-intel-m10-bmc         |  15 ++
 drivers/mfd/Kconfig                                |  13 ++
 drivers/mfd/Makefile                               |   2 +
 drivers/mfd/intel-m10-bmc.c                        | 169 +++++++++++++++++++++
 include/linux/mfd/intel-m10-bmc.h                  |  57 +++++++
 5 files changed, 256 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
 create mode 100644 drivers/mfd/intel-m10-bmc.c
 create mode 100644 include/linux/mfd/intel-m10-bmc.h

diff --git a/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
new file mode 100644
index 0000000..979a2d6
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
@@ -0,0 +1,15 @@
+What:		/sys/bus/spi/devices/.../bmc_version
+Date:		June 2020
+KernelVersion:	5.10
+Contact:	Xu Yilun <yilun.xu@intel.com>
+Description:	Read only. Returns the hardware build version of Intel
+		MAX10 BMC chip.
+		Format: "0x%x".
+
+What:		/sys/bus/spi/devices/.../bmcfw_version
+Date:		June 2020
+KernelVersion:	5.10
+Contact:	Xu Yilun <yilun.xu@intel.com>
+Description:	Read only. Returns the firmware version of Intel MAX10
+		BMC chip.
+		Format: "0x%x".
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 33df083..57745f5 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2118,5 +2118,18 @@ 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 MAX10 Board Management Controller"
+	depends on SPI_MASTER
+	select REGMAP_SPI_AVMM
+	select MFD_CORE
+	help
+	  Support for the Intel MAX10 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.
+
 endmenu
 endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index a60e5f8..dd2cc7b 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -264,3 +264,5 @@ obj-$(CONFIG_MFD_STMFX) 	+= stmfx.o
 obj-$(CONFIG_MFD_KHADAS_MCU) 	+= khadas-mcu.o
 
 obj-$(CONFIG_SGI_MFD_IOC3)	+= ioc3.o
+
+obj-$(CONFIG_MFD_INTEL_M10_BMC)   += intel-m10-bmc.o
diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc.c
new file mode 100644
index 0000000..0dfd73a
--- /dev/null
+++ b/drivers/mfd/intel-m10-bmc.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Intel Max10 Board Management Controller chip
+ *
+ * Copyright (C) 2018-2020 Intel Corporation. All rights reserved.
+ *
+ */
+#include <linux/bitfield.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/intel-m10-bmc.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+enum m10bmc_type {
+	M10_N3000,
+};
+
+static struct mfd_cell m10bmc_pacn3000_subdevs[] = {
+	{
+		.name = "n3000bmc-hwmon",
+	},
+	{
+		.name = "n3000bmc-pkvl",
+	},
+	{
+		.name = "m10bmc-secure",
+	},
+
+};
+
+static struct regmap_config intel_m10bmc_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = M10BMC_MEM_END,
+};
+
+static ssize_t bmc_version_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct intel_m10bmc *m10bmc = dev_get_drvdata(dev);
+	unsigned int val;
+	int ret;
+
+	ret = m10bmc_sys_read(m10bmc, 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 *max10 = dev_get_drvdata(dev);
+	unsigned int val;
+	int ret;
+
+	ret = m10bmc_sys_read(max10, NIOS2_FW_VERSION, &val);
+	if (ret)
+		return ret;
+
+	return sprintf(buf, "0x%x\n", val);
+}
+static DEVICE_ATTR_RO(bmcfw_version);
+
+static struct attribute *m10bmc_attrs[] = {
+	&dev_attr_bmc_version.attr,
+	&dev_attr_bmcfw_version.attr,
+	NULL,
+};
+
+static struct attribute_group m10bmc_attr_group = {
+	.attrs = m10bmc_attrs,
+};
+
+static const struct attribute_group *m10bmc_dev_groups[] = {
+	&m10bmc_attr_group,
+	NULL
+};
+
+static int check_m10bmc_version(struct intel_m10bmc *m10bmc)
+{
+	unsigned int v;
+
+	if (m10bmc_raw_read(m10bmc, M10BMC_LEGACY_SYS_BASE + M10BMC_BUILD_VER,
+			    &v))
+		return -ENODEV;
+
+	if (v != 0xffffffff) {
+		dev_err(m10bmc->dev, "bad version M10BMC detected\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int intel_m10_bmc_spi_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+	struct device *dev = &spi->dev;
+	struct mfd_cell *cells;
+	struct intel_m10bmc *m10bmc;
+	int ret, n_cell;
+
+	m10bmc = devm_kzalloc(dev, sizeof(*m10bmc), GFP_KERNEL);
+	if (!m10bmc)
+		return -ENOMEM;
+
+	m10bmc->dev = dev;
+
+	m10bmc->regmap =
+		devm_regmap_init_spi_avmm(spi, &intel_m10bmc_regmap_config);
+	if (IS_ERR(m10bmc->regmap)) {
+		ret = PTR_ERR(m10bmc->regmap);
+		dev_err(dev, "Failed to allocate regmap: %d\n", ret);
+		return ret;
+	}
+
+	spi_set_drvdata(spi, m10bmc);
+
+	ret = check_m10bmc_version(m10bmc);
+	if (ret) {
+		dev_err(dev, "Failed to identify m10bmc hardware\n");
+		return ret;
+	}
+
+	switch (id->driver_data) {
+	case M10_N3000:
+		cells = m10bmc_pacn3000_subdevs;
+		n_cell = ARRAY_SIZE(m10bmc_pacn3000_subdevs);
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, cells, n_cell,
+				   NULL, 0, NULL);
+	if (ret)
+		dev_err(dev, "Failed to register sub-devices: %d\n", ret);
+
+	return ret;
+}
+
+static const struct spi_device_id m10bmc_spi_id[] = {
+	{ "m10-n3000", M10_N3000 },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, m10bmc_spi_id);
+
+static struct spi_driver intel_m10bmc_spi_driver = {
+	.driver = {
+		.name = "intel-m10-bmc",
+		.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 Max10 BMC Device Driver");
+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
new file mode 100644
index 0000000..4979756
--- /dev/null
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Driver Header File for Intel Max10 Board Management Controller chip.
+ *
+ * Copyright (C) 2018-2020 Intel Corporation, Inc.
+ *
+ */
+#ifndef __INTEL_M10_BMC_H
+#define __INTEL_M10_BMC_H
+
+#include <linux/regmap.h>
+
+#define M10BMC_LEGACY_SYS_BASE	0x300400
+#define M10BMC_SYS_BASE		0x300800
+#define M10BMC_MEM_END		0x200000fc
+
+/* Register offset of system registers */
+#define NIOS2_FW_VERSION	0x0
+#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)
+
+/**
+ * struct intel_m10bmc - Intel Max10 BMC MFD device private data structure
+ * @dev: this device
+ * @regmap: the regmap used to access registers by m10bmc itself
+ */
+struct intel_m10bmc {
+	struct device *dev;
+	struct regmap *regmap;
+};
+
+/*
+ * register access helper functions.
+ *
+ * m10bmc_raw_read - read m10bmc register per addr
+ * m10bmc_sys_read - read m10bmc system register per offset
+ */
+static inline int
+m10bmc_raw_read(struct intel_m10bmc *m10bmc, unsigned int addr,
+		unsigned int *val)
+{
+	int ret;
+
+	ret = regmap_read(m10bmc->regmap, addr, val);
+	if (ret)
+		dev_err(m10bmc->dev, "fail to read raw reg %x: %d\n",
+			addr, ret);
+
+	return ret;
+}
+
+#define m10bmc_sys_read(m10bmc, offset, val) \
+	m10bmc_raw_read(m10bmc, M10BMC_SYS_BASE + (offset), val)
+
+#endif /* __INTEL_M10_BMC_H */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support
  2020-08-05  8:00 [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support Xu Yilun
  2020-08-05  8:00 ` [PATCH v3 1/2] regmap: add Intel SPI Slave to AVMM Bus Bridge support Xu Yilun
  2020-08-05  8:00 ` [PATCH v3 2/2] mfd: intel-m10-bmc: add Max10 BMC chip support for Intel FPGA PAC Xu Yilun
@ 2020-08-17  8:24 ` Xu Yilun
  2020-08-17  9:12   ` Lee Jones
  2 siblings, 1 reply; 8+ messages in thread
From: Xu Yilun @ 2020-08-17  8:24 UTC (permalink / raw)
  To: broonie, lee.jones, linux-kernel
  Cc: trix, matthew.gerlach, russell.h.weight, lgoncalv, hao.wu

Hi Brown & jones:

I tried to refacor the regmap code and add comments in this patchset. I
made big changes to the rx flow and remove some tricky parts in it.

Would it be more understandable than last version? I'm expecting your
comments on it when you have time, thanks in advance.

Yilun

On Wed, Aug 05, 2020 at 04:00:54PM +0800, Xu Yilun wrote:
> This patchset adds the regmap-spi-avmm to support the Intel SPI Slave to
> AVMM Bus Bridge (spi-avmm) IP block. It also implements the usercase - the
> driver of Intel Max10 BMC chip which integrates this IP block.
> 
> Patch #1 implements the regmap-spi-avmm.
> Patch #2 implements the mfd driver of Intel Max10 BMC chip.
> 
> Main changes from v1:
> - Split out the regmap-spi-avmm module out of intel-m10-bmc module.
> 
> Main changes from v2:
> - Refactor the code of regmap-spi-avmm.
>    - Rewrites the rx flow and simplifies the implementation, collapse some
>      function calls.
>    - Add bounds checking every time we fill trans_buf & phy_buf.
>    - Try to configure spi mode on regmap init
>    - delete regmap_bus.reg_write/reg_read callbacks.
>    - Squash the bug fixing patch.
> - Add the sub devices in mfd_cell for Max10 bmc driver.
> - Improve comments and some minor fixes.
> 
> Xu Yilun (2):
>   regmap: add Intel SPI Slave to AVMM Bus Bridge support
>   mfd: intel-m10-bmc: add Max10 BMC chip support for Intel FPGA PAC
> 
>  .../ABI/testing/sysfs-driver-intel-m10-bmc         |  15 +
>  drivers/base/regmap/Kconfig                        |   6 +-
>  drivers/base/regmap/Makefile                       |   1 +
>  drivers/base/regmap/regmap-spi-avmm.c              | 735 +++++++++++++++++++++
>  drivers/mfd/Kconfig                                |  13 +
>  drivers/mfd/Makefile                               |   2 +
>  drivers/mfd/intel-m10-bmc.c                        | 169 +++++
>  include/linux/mfd/intel-m10-bmc.h                  |  57 ++
>  include/linux/regmap.h                             |  36 +
>  9 files changed, 1033 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
>  create mode 100644 drivers/base/regmap/regmap-spi-avmm.c
>  create mode 100644 drivers/mfd/intel-m10-bmc.c
>  create mode 100644 include/linux/mfd/intel-m10-bmc.h
> 
> -- 
> 2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support
  2020-08-17  8:24 ` [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support Xu Yilun
@ 2020-08-17  9:12   ` Lee Jones
  2020-08-18  8:36     ` Xu Yilun
  0 siblings, 1 reply; 8+ messages in thread
From: Lee Jones @ 2020-08-17  9:12 UTC (permalink / raw)
  To: Xu Yilun
  Cc: broonie, linux-kernel, trix, matthew.gerlach, russell.h.weight,
	lgoncalv, hao.wu

On Mon, 17 Aug 2020, Xu Yilun wrote:

> Hi Brown & jones:
> 
> I tried to refacor the regmap code and add comments in this patchset. I
> made big changes to the rx flow and remove some tricky parts in it.
> 
> Would it be more understandable than last version? I'm expecting your
> comments on it when you have time, thanks in advance.

Just resubmit please.  We can review the code itself.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support
  2020-08-17  9:12   ` Lee Jones
@ 2020-08-18  8:36     ` Xu Yilun
  2020-08-18 14:40       ` Tom Rix
  0 siblings, 1 reply; 8+ messages in thread
From: Xu Yilun @ 2020-08-18  8:36 UTC (permalink / raw)
  To: Lee Jones
  Cc: broonie, linux-kernel, trix, matthew.gerlach, russell.h.weight,
	lgoncalv, hao.wu

On Mon, Aug 17, 2020 at 10:12:52AM +0100, Lee Jones wrote:
> On Mon, 17 Aug 2020, Xu Yilun wrote:
> 
> > Hi Brown & jones:
> > 
> > I tried to refacor the regmap code and add comments in this patchset. I
> > made big changes to the rx flow and remove some tricky parts in it.
> > 
> > Would it be more understandable than last version? I'm expecting your
> > comments on it when you have time, thanks in advance.
> 
> Just resubmit please.  We can review the code itself.

Ok. I'll rebase it to 5.9-rc1 and resubmit it.

Thanks,
Yilun

> 
> -- 
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support
  2020-08-18  8:36     ` Xu Yilun
@ 2020-08-18 14:40       ` Tom Rix
  2020-08-18 15:07         ` Xu Yilun
  0 siblings, 1 reply; 8+ messages in thread
From: Tom Rix @ 2020-08-18 14:40 UTC (permalink / raw)
  To: Xu Yilun, Lee Jones
  Cc: broonie, linux-kernel, matthew.gerlach, russell.h.weight,
	lgoncalv, hao.wu

Yilun,

I was looking at the tx side a bit and think the padding function could be moved into the pkt/phy function.  The pky/phy function already is looking for the eop's so reuse it and remove the search for eop and exchange the loops that do char moving and padding to mem* functions.  The logic is something like.                                                          
if (tb == tb_end - 1 && !eop_inserted) {
        *pb = PKT_EOP;

        p_eop = pb;

...

    dst_pb = &br->phy_buf[aligned_len];
    size_t s = pb - p_eop + 1;
    /* move EOP and bytes after EOP to the end of aligned size */
    memmove(dst_pb - s, p_eop, s);
    /* fill the hole with PHY_IDLEs */
    memset(p_eop, PHY_IDLE, aligned_len - br->phy_len);
    /* update the phy data length */

Tom

 

On 8/18/20 1:36 AM, Xu Yilun wrote:
> On Mon, Aug 17, 2020 at 10:12:52AM +0100, Lee Jones wrote:
>> On Mon, 17 Aug 2020, Xu Yilun wrote:
>>
>>> Hi Brown & jones:
>>>
>>> I tried to refacor the regmap code and add comments in this patchset. I
>>> made big changes to the rx flow and remove some tricky parts in it.
>>>
>>> Would it be more understandable than last version? I'm expecting your
>>> comments on it when you have time, thanks in advance.
>> Just resubmit please.  We can review the code itself.
> Ok. I'll rebase it to 5.9-rc1 and resubmit it.
>
> Thanks,
> Yilun
>
>> -- 
>> Lee Jones [李琼斯]
>> Senior Technical Lead - Developer Services
>> Linaro.org │ Open source software for Arm SoCs
>> Follow Linaro: Facebook | Twitter | Blog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support
  2020-08-18 14:40       ` Tom Rix
@ 2020-08-18 15:07         ` Xu Yilun
  0 siblings, 0 replies; 8+ messages in thread
From: Xu Yilun @ 2020-08-18 15:07 UTC (permalink / raw)
  To: Tom Rix
  Cc: Lee Jones, broonie, linux-kernel, matthew.gerlach,
	russell.h.weight, lgoncalv, hao.wu

I think that's a good optimization. I'll include this change and send a
v4.

Thanks.

On Tue, Aug 18, 2020 at 07:40:45AM -0700, Tom Rix wrote:
> Yilun,
> 
> I was looking at the tx side a bit and think the padding function could be moved into the pkt/phy function.  The pky/phy function already is looking for the eop's so reuse it and remove the search for eop and exchange the loops that do char moving and padding to mem* functions.  The logic is something like.                                                          
> if (tb == tb_end - 1 && !eop_inserted) {
>         *pb = PKT_EOP;
> 
>         p_eop = pb;
> 
> ...
> 
>     dst_pb = &br->phy_buf[aligned_len];
>     size_t s = pb - p_eop + 1;
>     /* move EOP and bytes after EOP to the end of aligned size */
>     memmove(dst_pb - s, p_eop, s);
>     /* fill the hole with PHY_IDLEs */
>     memset(p_eop, PHY_IDLE, aligned_len - br->phy_len);
>     /* update the phy data length */
> 
> Tom
> 
>  
> 
> On 8/18/20 1:36 AM, Xu Yilun wrote:
> > On Mon, Aug 17, 2020 at 10:12:52AM +0100, Lee Jones wrote:
> >> On Mon, 17 Aug 2020, Xu Yilun wrote:
> >>
> >>> Hi Brown & jones:
> >>>
> >>> I tried to refacor the regmap code and add comments in this patchset. I
> >>> made big changes to the rx flow and remove some tricky parts in it.
> >>>
> >>> Would it be more understandable than last version? I'm expecting your
> >>> comments on it when you have time, thanks in advance.
> >> Just resubmit please.  We can review the code itself.
> > Ok. I'll rebase it to 5.9-rc1 and resubmit it.
> >
> > Thanks,
> > Yilun
> >
> >> -- 
> >> Lee Jones [李琼斯]
> >> Senior Technical Lead - Developer Services
> >> Linaro.org │ Open source software for Arm SoCs
> >> Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-08-18 15:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-05  8:00 [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support Xu Yilun
2020-08-05  8:00 ` [PATCH v3 1/2] regmap: add Intel SPI Slave to AVMM Bus Bridge support Xu Yilun
2020-08-05  8:00 ` [PATCH v3 2/2] mfd: intel-m10-bmc: add Max10 BMC chip support for Intel FPGA PAC Xu Yilun
2020-08-17  8:24 ` [PATCH v3 0/2] add regmap-spi-avmm & Intel Max10 BMC chip support Xu Yilun
2020-08-17  9:12   ` Lee Jones
2020-08-18  8:36     ` Xu Yilun
2020-08-18 14:40       ` Tom Rix
2020-08-18 15:07         ` Xu Yilun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).