[PATCH RFC SPI for-next 0/2] spi: microchip: pci1xxxx: Load SPI driver for SPI endpoint of PCI1XXXX switch

[PATCH RFC SPI for-next 0/2] spi: microchip: pci1xxxx: Load SPI driver for SPI endpoint of PCI1XXXX switch

[Tharun Kumar P]

Microchip PCI1XXXX is an unmanaged PCIe3.1a switch for consumer, industrial, and
automotive applications. This switch has multiple downstream ports. One of the
switch's downstream ports is a multifunction endpoint; one of those functions
supports SPI functionality. This series of patches provides the SPI controller
driver for the SPI function of the multifunction PCIe endpoint of the switch.

Tharun Kumar P (2):
  spi: microchip: pci1xxxx: Add driver for SPI controller of PCI1XXXX
    PCIe switch
  spi: microchip: pci1xxxx: Add suspend and resume support for PCI1XXXX
    SPI driver

 drivers/spi/Kconfig        |   9 +
 drivers/spi/Makefile       |   1 +
 drivers/spi/spi-pci1xxxx.c | 453 +++++++++++++++++++++++++++++++++++++
 3 files changed, 463 insertions(+)
 create mode 100644 drivers/spi/spi-pci1xxxx.c

-- 
2.25.1

[PATCH RFC SPI for-next 1/2] spi: microchip: pci1xxxx: Add driver for SPI controller of PCI1XXXX PCIe switch

[Tharun Kumar P]

Microchip pci1xxxx is a PCIe switch with a multi-function endpoint on one of its
downstream ports. SPI is one of the functions in the multi-function endpoint. This
function has 2 SPI masters, operates at a maximum frequency of 30 MHz and supports
7 client devices per master. This patch adds complete functionality to the SPI
function except for suspend and resume.

Signed-off-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>

---
 drivers/spi/Kconfig        |   9 +
 drivers/spi/Makefile       |   1 +
 drivers/spi/spi-pci1xxxx.c | 378 +++++++++++++++++++++++++++++++++++++
 3 files changed, 388 insertions(+)
 create mode 100644 drivers/spi/spi-pci1xxxx.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 9987c3f2bd1c..08ca811440f9 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -701,6 +701,15 @@ config SPI_ORION
 	  This enables using the SPI master controller on the Orion
 	  and MVEBU chips.
 
+config SPI_PCI1XXXX
+	tristate "PCI1XXXX SPI Bus support"
+	depends on PCI
+	help
+	  Say "yes" to Enable the SPI Bus support for the PCI1xxxx card
+	  This is a PCI to SPI Bus driver
+	  This driver can be built as module. If so, the module will be
+	  called as spi-pci1xxxx.
+
 config SPI_PIC32
 	tristate "Microchip PIC32 series SPI"
 	depends on MACH_PIC32 || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 15d2f3835e45..b75fa3988f96 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -93,6 +93,7 @@ obj-$(CONFIG_SPI_OMAP_100K)		+= spi-omap-100k.o
 obj-$(CONFIG_SPI_OMAP24XX)		+= spi-omap2-mcspi.o
 obj-$(CONFIG_SPI_TI_QSPI)		+= spi-ti-qspi.o
 obj-$(CONFIG_SPI_ORION)			+= spi-orion.o
+obj-$(CONFIG_SPI_PCI1XXXX)		+= spi-pci1xxxx.o
 obj-$(CONFIG_SPI_PIC32)			+= spi-pic32.o
 obj-$(CONFIG_SPI_PIC32_SQI)		+= spi-pic32-sqi.o
 obj-$(CONFIG_SPI_PL022)			+= spi-pl022.o
diff --git a/drivers/spi/spi-pci1xxxx.c b/drivers/spi/spi-pci1xxxx.c
new file mode 100644
index 000000000000..0b6ba8108d19
--- /dev/null
+++ b/drivers/spi/spi-pci1xxxx.c
@@ -0,0 +1,378 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * PCI1xxxx SPI driver
+ * Copyright (C) 2022 Microchip Technology Inc.
+ * Authors: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
+ *          Kumaravel Thiagarajan <Kumaravel.Thiagarajan@microchip.com>
+ */
+
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/spi/spi.h>
+
+#define DRV_NAME "spi-pci1xxxx"
+
+#define	SYS_FREQ_DEFAULT		(62500000)
+
+#define	PCI1XXXX_SPI_MAX_CLOCK_HZ	(30000000)
+#define	PCI1XXXX_SPI_CLK_20MHZ		(20000000)
+#define	PCI1XXXX_SPI_CLK_15MHZ		(15000000)
+#define	PCI1XXXX_SPI_CLK_12MHZ		(12000000)
+#define	PCI1XXXX_SPI_CLK_10MHZ		(10000000)
+#define	PCI1XXXX_SPI_MIN_CLOCK_HZ	(2000000)
+
+#define	PCI1XXXX_SPI_BUFFER_SIZE	(320)
+
+#define	SPI_MST_CTL_DEVSEL_MASK		(GENMASK(27, 25))
+#define	SPI_MST_CTL_CMD_LEN_MASK	(GENMASK(16, 8))
+#define	SPI_MST_CTL_SPEED_MASK		(GENMASK(7, 5))
+#define	SPI_MSI_VECTOR_SEL_MASK		(GENMASK(4, 4))
+
+#define	SPI_MST_CTL_FORCE_CE		(BIT(4))
+#define	SPI_MST_CTL_MODE_SEL		(BIT(2))
+#define	SPI_MST_CTL_GO			(BIT(0))
+
+#define	SPI_MST1_ADDR_BASE		(0x800)
+
+/* x refers to SPI Host Controller HW instance id in the below macros - 0 or 1 */
+
+#define	SPI_MST_CMD_BUF_OFFSET(x)		(((x) * SPI_MST1_ADDR_BASE) + 0x00)
+#define	SPI_MST_RSP_BUF_OFFSET(x)		(((x) * SPI_MST1_ADDR_BASE) + 0x200)
+#define	SPI_MST_CTL_REG_OFFSET(x)		(((x) * SPI_MST1_ADDR_BASE) + 0x400)
+#define	SPI_MST_EVENT_REG_OFFSET(x)		(((x) * SPI_MST1_ADDR_BASE) + 0x420)
+#define	SPI_MST_EVENT_MASK_REG_OFFSET(x)	(((x) * SPI_MST1_ADDR_BASE) + 0x424)
+#define	SPI_MST_PAD_CTL_REG_OFFSET(x)		(((x) * SPI_MST1_ADDR_BASE) + 0x460)
+#define	SPIALERT_MST_DB_REG_OFFSET(x)		(((x) * SPI_MST1_ADDR_BASE) + 0x464)
+#define	SPIALERT_MST_VAL_REG_OFFSET(x)		(((x) * SPI_MST1_ADDR_BASE) + 0x468)
+#define	SPI_PCI_CTRL_REG_OFFSET(x)		(((x) * SPI_MST1_ADDR_BASE) + 0x480)
+
+#define PCI1XXXX_IRQ_FLAGS			(IRQF_NO_SUSPEND | IRQF_TRIGGER_NONE)
+#define SPI_MAX_DATA_LEN			320
+
+#define PCI1XXXX_SPI_TIMEOUT			(msecs_to_jiffies(100))
+
+#define SPI_INTR		BIT(8)
+#define SPI_FORCE_CE		BIT(4)
+
+#define SPI_CHIP_SEL_COUNT 7
+#define VENDOR_ID_MCHP 0x1055
+
+struct pci1xxxx_spi_internal {
+	u8 hw_inst;
+	int irq;
+	struct completion spi_xfer_done;
+	struct spi_master *spi_host;
+	struct pci1xxxx_spi *parent;
+	struct {
+		unsigned int dev_sel : 3;
+		unsigned int msi_vector_sel : 1;
+	} prev_val;
+};
+
+struct pci1xxxx_spi {
+	struct pci_dev *dev;
+	u8 total_hw_instances;
+	void __iomem *reg_base;
+	struct pci1xxxx_spi_internal *spi_int[];
+};
+
+static const struct pci_device_id pci1xxxx_spi_pci_id_table[] = {
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa004, PCI_ANY_ID, 0x0001), 0, 0, 0x02},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa004, PCI_ANY_ID, 0x0002), 0, 0, 0x01},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa004, PCI_ANY_ID, 0x0003), 0, 0, 0x11},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa004, PCI_ANY_ID, PCI_ANY_ID), 0, 0, 0x01},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa014, PCI_ANY_ID, 0x0001), 0, 0, 0x02},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa014, PCI_ANY_ID, 0x0002), 0, 0, 0x01},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa014, PCI_ANY_ID, 0x0003), 0, 0, 0x11},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa014, PCI_ANY_ID, PCI_ANY_ID), 0, 0, 0x01},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa024, PCI_ANY_ID, 0x0001), 0, 0, 0x02},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa024, PCI_ANY_ID, 0x0002), 0, 0, 0x01},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa024, PCI_ANY_ID, 0x0003), 0, 0, 0x11},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa024, PCI_ANY_ID, PCI_ANY_ID), 0, 0, 0x01},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa034, PCI_ANY_ID, 0x0001), 0, 0, 0x02},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa034, PCI_ANY_ID, 0x0002), 0, 0, 0x01},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa034, PCI_ANY_ID, 0x0003), 0, 0, 0x11},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa034, PCI_ANY_ID, PCI_ANY_ID), 0, 0, 0x01},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa044, PCI_ANY_ID, 0x0001), 0, 0, 0x02},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa044, PCI_ANY_ID, 0x0002), 0, 0, 0x01},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa044, PCI_ANY_ID, 0x0003), 0, 0, 0x11},
+	{ PCI_DEVICE_SUB(VENDOR_ID_MCHP, 0xa044, PCI_ANY_ID, PCI_ANY_ID), 0, 0, 0x01},
+	{ 0, }
+};
+
+MODULE_DEVICE_TABLE(pci, pci1xxxx_spi_pci_id_table);
+
+static void pci1xxxx_spi_set_cs(struct spi_device *spi, bool enable)
+{
+	struct pci1xxxx_spi_internal *p = spi_controller_get_devdata(spi->controller);
+	struct pci1xxxx_spi *par = p->parent;
+	u32 regval;
+
+	/* Set the DEV_SEL bits of the SPI_MST_CTL_REG */
+	regval = readl(par->reg_base + SPI_MST_CTL_REG_OFFSET(p->hw_inst));
+	if (enable) {
+		regval &= ~SPI_MST_CTL_DEVSEL_MASK;
+		regval |= (spi->chip_select << 25);
+		writel(regval,
+		       par->reg_base + SPI_MST_CTL_REG_OFFSET(p->hw_inst));
+	}
+}
+
+static u8 pci1xxxx_get_clock_div(u32 hz)
+{
+	u8 val = 0;
+
+	if (hz >= PCI1XXXX_SPI_MAX_CLOCK_HZ)
+		val = 2;
+	else if ((hz < PCI1XXXX_SPI_MAX_CLOCK_HZ) && (hz >= PCI1XXXX_SPI_CLK_20MHZ))
+		val = 3;
+	else if ((hz < PCI1XXXX_SPI_CLK_20MHZ) && (hz >= PCI1XXXX_SPI_CLK_15MHZ))
+		val = 4;
+	else if ((hz < PCI1XXXX_SPI_CLK_15MHZ) && (hz >= PCI1XXXX_SPI_CLK_12MHZ))
+		val = 5;
+	else if ((hz < PCI1XXXX_SPI_CLK_12MHZ) && (hz >= PCI1XXXX_SPI_CLK_10MHZ))
+		val = 6;
+	else if ((hz < PCI1XXXX_SPI_CLK_10MHZ) && (hz >= PCI1XXXX_SPI_MIN_CLOCK_HZ))
+		val = 7;
+	else
+		val = 2;
+
+	return val;
+}
+
+static int pci1xxxx_spi_transfer_one(struct spi_controller *spi_ctlr,
+				     struct spi_device *spi, struct spi_transfer *xfer)
+{
+	struct pci1xxxx_spi_internal *p = spi_controller_get_devdata(spi_ctlr);
+	int mode, len, loop_iter, transfer_len;
+	struct pci1xxxx_spi *par = p->parent;
+	unsigned long bytes_transfered;
+	unsigned long bytes_recvd;
+	unsigned long loop_count;
+	u8 *rx_buf, result;
+	const u8 *tx_buf;
+	u32 regval;
+	u8 clkdiv;
+
+	mode = spi->mode;
+	clkdiv = pci1xxxx_get_clock_div(xfer->speed_hz);
+	tx_buf = xfer->tx_buf;
+	rx_buf = xfer->rx_buf;
+	transfer_len = xfer->len;
+	regval = readl(par->reg_base + SPI_MST_EVENT_REG_OFFSET(p->hw_inst));
+	writel(regval, par->reg_base + SPI_MST_EVENT_REG_OFFSET(p->hw_inst));
+
+	if (tx_buf) {
+		bytes_transfered = 0;
+		bytes_recvd = 0;
+		loop_count = transfer_len / SPI_MAX_DATA_LEN;
+		if (transfer_len % SPI_MAX_DATA_LEN != 0)
+			loop_count += 1;
+
+		for (loop_iter = 0; loop_iter < loop_count; loop_iter++) {
+			len = SPI_MAX_DATA_LEN;
+			if ((transfer_len % SPI_MAX_DATA_LEN != 0) &&
+			    (loop_iter == loop_count - 1))
+				len = transfer_len % SPI_MAX_DATA_LEN;
+
+			reinit_completion(&p->spi_xfer_done);
+			memcpy_toio(par->reg_base + SPI_MST_CMD_BUF_OFFSET(p->hw_inst),
+				    &tx_buf[bytes_transfered], len);
+			bytes_transfered += len;
+			regval = readl(par->reg_base +
+				       SPI_MST_CTL_REG_OFFSET(p->hw_inst));
+			regval &= ~(SPI_MST_CTL_MODE_SEL | SPI_MST_CTL_CMD_LEN_MASK |
+				    SPI_MST_CTL_SPEED_MASK);
+
+			if (mode == SPI_MODE_3)
+				regval |= SPI_MST_CTL_MODE_SEL;
+			else
+				regval &= ~SPI_MST_CTL_MODE_SEL;
+
+			regval |= ((clkdiv << 5) | SPI_FORCE_CE | (len << 8));
+			regval &= ~SPI_MST_CTL_CMD_LEN_MASK;
+			writel(regval, par->reg_base +
+			       SPI_MST_CTL_REG_OFFSET(p->hw_inst));
+			regval = readl(par->reg_base +
+				       SPI_MST_CTL_REG_OFFSET(p->hw_inst));
+			regval |= SPI_MST_CTL_GO;
+			writel(regval, par->reg_base +
+			       SPI_MST_CTL_REG_OFFSET(p->hw_inst));
+
+			/* Wait for DMA_TERM interrupt */
+			result = wait_for_completion_timeout(&p->spi_xfer_done,
+							     PCI1XXXX_SPI_TIMEOUT);
+			if (!result)
+				return -ETIMEDOUT;
+
+			if (rx_buf) {
+				memcpy_fromio(&rx_buf[bytes_recvd], par->reg_base +
+					      SPI_MST_RSP_BUF_OFFSET(p->hw_inst), len);
+				bytes_recvd += len;
+			}
+		}
+	}
+
+	regval = readl(par->reg_base + SPI_MST_CTL_REG_OFFSET(p->hw_inst));
+	regval &= ~SPI_FORCE_CE;
+	writel(regval, par->reg_base + SPI_MST_CTL_REG_OFFSET(p->hw_inst));
+
+	return 0;
+}
+
+static irqreturn_t pci1xxxx_spi_isr(int irq, void *dev)
+{
+	struct pci1xxxx_spi_internal *p = dev;
+	irqreturn_t spi_int_fired = IRQ_NONE;
+	u32 regval;
+
+	/* Clear the SPI GO_BIT Interrupt */
+	regval = readl(p->parent->reg_base + SPI_MST_EVENT_REG_OFFSET(p->hw_inst));
+	if (regval & SPI_INTR) {
+		/* Clear xfer_done */
+		complete(&p->spi_xfer_done);
+		spi_int_fired = IRQ_HANDLED;
+	}
+
+	writel(regval, p->parent->reg_base + SPI_MST_EVENT_REG_OFFSET(p->hw_inst));
+
+	return spi_int_fired;
+}
+
+static int pci1xxxx_spi_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
+{
+	u8 hw_inst_cnt, iter, start, only_sec_inst;
+	struct pci1xxxx_spi_internal *spi_sub_ptr;
+	struct device *dev = &pdev->dev;
+	struct pci1xxxx_spi *spi_bus;
+	struct spi_master *spi_host;
+	u32 regval;
+	int ret;
+
+	hw_inst_cnt = ent->driver_data & 0x0f;
+	start = (ent->driver_data & 0xf0) >> 4;
+	(start == 1) ? (only_sec_inst = 1) : (only_sec_inst = 0);
+	spi_bus = devm_kzalloc(&pdev->dev,
+			       struct_size(spi_bus, spi_int, hw_inst_cnt),
+			       GFP_KERNEL);
+	if (!spi_bus)
+		return -ENOMEM;
+
+	spi_bus->dev = pdev;
+	spi_bus->total_hw_instances = hw_inst_cnt;
+	pci_set_master(pdev);
+
+	for (iter = 0; iter < hw_inst_cnt; iter++) {
+		spi_bus->spi_int[iter] = devm_kzalloc(&pdev->dev,
+						      sizeof(struct pci1xxxx_spi_internal),
+						      GFP_KERNEL);
+		spi_sub_ptr = spi_bus->spi_int[iter];
+		spi_sub_ptr->spi_host = devm_spi_alloc_master(dev, sizeof(struct spi_master));
+		if (!spi_sub_ptr->spi_host)
+			return -ENOMEM;
+
+		spi_sub_ptr->parent = spi_bus;
+
+		if (!iter) {
+			ret = pcim_enable_device(pdev);
+			if (ret)
+				return -ENOMEM;
+
+			ret = pci_request_regions(pdev, DRV_NAME);
+			if (ret)
+				return -ENOMEM;
+
+			spi_bus->reg_base = pcim_iomap(pdev, 0, pci_resource_len(pdev, 0));
+			if (!spi_bus->reg_base) {
+				ret = -EINVAL;
+				goto error;
+			}
+
+			ret = pci_alloc_irq_vectors(pdev, hw_inst_cnt, hw_inst_cnt,
+						    PCI_IRQ_ALL_TYPES);
+			if (ret < 0) {
+				dev_err(&pdev->dev, "Error allocating MSI vectors\n");
+				goto error;
+			}
+
+			spi_sub_ptr->irq = pci_irq_vector(pdev, 0);
+
+			ret = devm_request_irq(&pdev->dev, spi_sub_ptr->irq,
+					       pci1xxxx_spi_isr, PCI1XXXX_IRQ_FLAGS,
+					       pci_name(pdev), spi_sub_ptr);
+			if (ret < 0) {
+				dev_err(&pdev->dev, "Unable to request irq : %d",
+					spi_sub_ptr->irq);
+				ret = -ENODEV;
+				goto error;
+			}
+
+			/* This register is only applicable for 1st instance */
+			regval = readl(spi_bus->reg_base + SPI_PCI_CTRL_REG_OFFSET(0));
+			if (!only_sec_inst)
+				regval |= (BIT(4));
+			else
+				regval &= ~(BIT(4));
+
+			writel(regval, spi_bus->reg_base + SPI_PCI_CTRL_REG_OFFSET(0));
+		}
+
+		spi_sub_ptr->hw_inst = start++;
+
+		if (iter == 1) {
+			spi_sub_ptr->irq = pci_irq_vector(pdev, iter);
+			ret = devm_request_irq(&pdev->dev, spi_sub_ptr->irq,
+					       pci1xxxx_spi_isr, PCI1XXXX_IRQ_FLAGS,
+					       pci_name(pdev), spi_sub_ptr);
+			if (ret < 0) {
+				dev_err(&pdev->dev, "Unable to request irq : %d",
+					spi_sub_ptr->irq);
+				ret = -ENODEV;
+				goto error;
+			}
+		}
+
+		init_completion(&spi_sub_ptr->spi_xfer_done);
+		spi_host = spi_sub_ptr->spi_host;
+		spi_host->num_chipselect = SPI_CHIP_SEL_COUNT;
+		spi_host->mode_bits = SPI_MODE_0 | SPI_MODE_3 | SPI_RX_DUAL |
+				      SPI_TX_DUAL | SPI_LOOP;
+		spi_host->transfer_one = pci1xxxx_spi_transfer_one;
+		spi_host->set_cs = pci1xxxx_spi_set_cs;
+		spi_host->bits_per_word_mask = SPI_BPW_MASK(8);
+		spi_host->max_speed_hz = PCI1XXXX_SPI_MAX_CLOCK_HZ;
+		spi_host->min_speed_hz = PCI1XXXX_SPI_MIN_CLOCK_HZ;
+		spi_master_set_devdata(spi_host, spi_sub_ptr);
+		ret = devm_spi_register_master(dev, spi_host);
+		if (ret)
+			goto error;
+
+		/* Initialize Interrupts - SPI_INT */
+		regval = readl(spi_bus->reg_base +
+			       SPI_MST_EVENT_MASK_REG_OFFSET(spi_sub_ptr->hw_inst));
+		regval &= ~SPI_INTR;
+		writel(regval, spi_bus->reg_base +
+		       SPI_MST_EVENT_MASK_REG_OFFSET(spi_sub_ptr->hw_inst));
+	}
+	pci_set_drvdata(pdev, spi_bus);
+
+	return 0;
+
+error:
+	pci_release_regions(pdev);
+	return ret;
+}
+
+static struct pci_driver pci1xxxx_spi_driver = {
+	.name		= DRV_NAME,
+	.id_table	= pci1xxxx_spi_pci_id_table,
+	.probe		= pci1xxxx_spi_probe,
+};
+
+module_pci_driver(pci1xxxx_spi_driver);
+
+MODULE_DESCRIPTION("Microchip Technology Inc. pci1xxxx SPI bus driver");
+MODULE_AUTHOR("Tharun Kumar P");
+MODULE_AUTHOR("Kumaravel Thiagarajan");
+MODULE_LICENSE("GPL v2");
-- 
2.25.1

[PATCH RFC SPI for-next 2/2] spi: microchip: pci1xxxx: Add suspend and resume support for PCI1XXXX SPI driver

[Tharun Kumar P]

Implement suspend, resume callbacks, store config at suspend and restore
config at time of resume

Signed-off-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>

---
 drivers/spi/spi-pci1xxxx.c | 75 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/drivers/spi/spi-pci1xxxx.c b/drivers/spi/spi-pci1xxxx.c
index 0b6ba8108d19..072549231e11 100644
--- a/drivers/spi/spi-pci1xxxx.c
+++ b/drivers/spi/spi-pci1xxxx.c
@@ -58,6 +58,9 @@
 #define SPI_CHIP_SEL_COUNT 7
 #define VENDOR_ID_MCHP 0x1055
 
+#define SPI_SUSPEND_CONFIG 0x101
+#define SPI_RESUME_CONFIG 0x303
+
 struct pci1xxxx_spi_internal {
 	u8 hw_inst;
 	int irq;
@@ -364,10 +367,82 @@ static int pci1xxxx_spi_probe(struct pci_dev *pdev, const struct pci_device_id *
 	return ret;
 }
 
+static void store_restore_config(struct pci1xxxx_spi *spi_ptr,
+				 struct pci1xxxx_spi_internal *spi_sub_ptr,
+				 u8 inst, bool store)
+{
+	u32 regval;
+
+	if (store) {
+		regval = readl(spi_ptr->reg_base +
+			       SPI_MST_CTL_REG_OFFSET(spi_sub_ptr->hw_inst));
+		regval &= SPI_MST_CTL_DEVSEL_MASK;
+		spi_sub_ptr->prev_val.dev_sel = (regval >> 25) & 7;
+		regval = readl(spi_ptr->reg_base +
+			       SPI_PCI_CTRL_REG_OFFSET(spi_sub_ptr->hw_inst));
+		regval &= SPI_MSI_VECTOR_SEL_MASK;
+		spi_sub_ptr->prev_val.msi_vector_sel = (regval >> 4) & 1;
+	} else {
+		regval = readl(spi_ptr->reg_base + SPI_MST_CTL_REG_OFFSET(inst));
+		regval &= ~SPI_MST_CTL_DEVSEL_MASK;
+		regval |= (spi_sub_ptr->prev_val.dev_sel << 25);
+		writel(regval,
+		       spi_ptr->reg_base + SPI_MST_CTL_REG_OFFSET(inst));
+		writel((spi_sub_ptr->prev_val.msi_vector_sel << 4),
+			spi_ptr->reg_base + SPI_PCI_CTRL_REG_OFFSET(inst));
+	}
+}
+
+static int pci1xxxx_spi_resume(struct device *dev)
+{
+	struct pci1xxxx_spi *spi_ptr = dev_get_drvdata(dev);
+	struct pci1xxxx_spi_internal *spi_sub_ptr;
+	u32 regval = SPI_RESUME_CONFIG;
+	u8 iter;
+
+	for (iter = 0; iter < spi_ptr->total_hw_instances; iter++) {
+		spi_sub_ptr = spi_ptr->spi_int[iter];
+		spi_master_resume(spi_sub_ptr->spi_host);
+		writel(regval, spi_ptr->reg_base +
+		       SPI_MST_EVENT_MASK_REG_OFFSET(iter));
+
+		/* Restore config at resume */
+		store_restore_config(spi_ptr, spi_sub_ptr, iter, 0);
+	}
+
+	return 0;
+}
+
+static int pci1xxxx_spi_suspend(struct device *dev)
+{
+	struct pci1xxxx_spi *spi_ptr = dev_get_drvdata(dev);
+	struct pci1xxxx_spi_internal *spi_sub_ptr;
+	u32 reg1 = SPI_SUSPEND_CONFIG;
+	u8 iter;
+
+	for (iter = 0; iter < spi_ptr->total_hw_instances; iter++) {
+		spi_sub_ptr = spi_ptr->spi_int[iter];
+
+		/* Store existing config before suspend */
+		store_restore_config(spi_ptr, spi_sub_ptr, iter, 1);
+		spi_master_suspend(spi_sub_ptr->spi_host);
+		writel(reg1, spi_ptr->reg_base +
+		       SPI_MST_EVENT_MASK_REG_OFFSET(iter));
+	}
+
+	return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(spi_pm_ops, pci1xxxx_spi_suspend,
+				pci1xxxx_spi_resume);
+
 static struct pci_driver pci1xxxx_spi_driver = {
 	.name		= DRV_NAME,
 	.id_table	= pci1xxxx_spi_pci_id_table,
 	.probe		= pci1xxxx_spi_probe,
+	.driver		=	{
+		.pm = pm_sleep_ptr(&spi_pm_ops),
+	},
 };
 
 module_pci_driver(pci1xxxx_spi_driver);
-- 
2.25.1

Re: [PATCH RFC SPI for-next 1/2] spi: microchip: pci1xxxx: Add driver for SPI controller of PCI1XXXX PCIe switch

[Mark Brown]

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

On Wed, Sep 28, 2022 at 09:13:35AM +0530, Tharun Kumar P wrote:

> @@ -0,0 +1,378 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * PCI1xxxx SPI driver
> + * Copyright (C) 2022 Microchip Technology Inc.
> + * Authors: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
> + *          Kumaravel Thiagarajan <Kumaravel.Thiagarajan@microchip.com>
> + */

Please make the above a single C++ style comment block so things look
more intentional.

> +static void pci1xxxx_spi_set_cs(struct spi_device *spi, bool enable)
> +{
> +	struct pci1xxxx_spi_internal *p = spi_controller_get_devdata(spi->controller);
> +	struct pci1xxxx_spi *par = p->parent;
> +	u32 regval;
> +
> +	/* Set the DEV_SEL bits of the SPI_MST_CTL_REG */
> +	regval = readl(par->reg_base + SPI_MST_CTL_REG_OFFSET(p->hw_inst));
> +	if (enable) {
> +		regval &= ~SPI_MST_CTL_DEVSEL_MASK;
> +		regval |= (spi->chip_select << 25);
> +		writel(regval,
> +		       par->reg_base + SPI_MST_CTL_REG_OFFSET(p->hw_inst));
> +	}
> +}

This does nothing if the chip select is to be disabled, that's clearly
not going to work.

> +static int pci1xxxx_spi_transfer_one(struct spi_controller *spi_ctlr,
> +				     struct spi_device *spi, struct spi_transfer *xfer)
> +{

> +	if (tx_buf) {

> +			if (rx_buf) {

The driver should set SPI_CONTROLLER_MUST_TX since it needs to transmit
data in order to recieve.

> +static int pci1xxxx_spi_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> +{
> +	u8 hw_inst_cnt, iter, start, only_sec_inst;
> +	struct pci1xxxx_spi_internal *spi_sub_ptr;
> +	struct device *dev = &pdev->dev;
> +	struct pci1xxxx_spi *spi_bus;
> +	struct spi_master *spi_host;
> +	u32 regval;
> +	int ret;
> +
> +	hw_inst_cnt = ent->driver_data & 0x0f;
> +	start = (ent->driver_data & 0xf0) >> 4;
> +	(start == 1) ? (only_sec_inst = 1) : (only_sec_inst = 0);

Please write normal conditional statements to improve legibility.

> +
> +			ret = devm_request_irq(&pdev->dev, spi_sub_ptr->irq,
> +					       pci1xxxx_spi_isr, PCI1XXXX_IRQ_FLAGS,
> +					       pci_name(pdev), spi_sub_ptr);
> +			if (ret < 0) {
> +				dev_err(&pdev->dev, "Unable to request irq : %d",
> +					spi_sub_ptr->irq);
> +				ret = -ENODEV;
> +				goto error;
> +			}

Are you sure the device is fully set up and ready for interrupts at this
point, and that the freeing of the driver will work fine with devm?

> +		init_completion(&spi_sub_ptr->spi_xfer_done);

I note that the completion that the interrupt handler uses isn't
allocated yet for example...

> +		/* Initialize Interrupts - SPI_INT */
> +		regval = readl(spi_bus->reg_base +
> +			       SPI_MST_EVENT_MASK_REG_OFFSET(spi_sub_ptr->hw_inst));
> +		regval &= ~SPI_INTR;
> +		writel(regval, spi_bus->reg_base +
> +		       SPI_MST_EVENT_MASK_REG_OFFSET(spi_sub_ptr->hw_inst));

...and we do some interrupt mask management later.

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

Re: [PATCH RFC SPI for-next 2/2] spi: microchip: pci1xxxx: Add suspend and resume support for PCI1XXXX SPI driver

[Mark Brown]

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

On Wed, Sep 28, 2022 at 09:13:36AM +0530, Tharun Kumar P wrote:

> +	for (iter = 0; iter < spi_ptr->total_hw_instances; iter++) {
> +		spi_sub_ptr = spi_ptr->spi_int[iter];
> +
> +		/* Store existing config before suspend */
> +		store_restore_config(spi_ptr, spi_sub_ptr, iter, 1);
> +		spi_master_suspend(spi_sub_ptr->spi_host);
> +		writel(reg1, spi_ptr->reg_base +
> +		       SPI_MST_EVENT_MASK_REG_OFFSET(iter));
> +	}

This saves the register configuration before suspending the device,
meaning there may be in progress transfers changing the device while the
save is going on.

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

Re: [PATCH RFC SPI for-next 1/2] spi: microchip: pci1xxxx: Add driver for SPI controller of PCI1XXXX PCIe switch

[Tharunkumar.Pasumarthi]

On Wed, 2022-09-28 at 12:26 +0100, Mark Brown wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Please make the above a single C++ style comment block so things look more 
> intentional.

Okay.

> This does nothing if the chip select is to be disabled, that's clearly not
> going to work. 

Okay. I will handle disable case of chip select.

> The driver should set SPI_CONTROLLER_TX since it needs to transmit data inorer
> to receive

Okay.

> Please write normal conditional statements to improve legibility.

Okay.

> Are you sure the device is fully set up and ready for interrupts at this 
> point, and that the freeing of the driver will work fine with devm?

Okay, I will move init_completion and interrupt mask management before
devm_request_irq API. 


Thanks,
Tharun Kumar P

Re: [PATCH RFC SPI for-next 2/2] spi: microchip: pci1xxxx: Add suspend and resume support for PCI1XXXX SPI driver

[Tharunkumar.Pasumarthi]

On Wed, 2022-09-28 at 12:30 +0100, Mark Brown wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> This saves the register configuration before suspending the device, meaning 
> there may be in progress transfers changing the device while the save is going
> on

I will add logic to wait for the completion of existing transfer at beginning of
suspend callback to handle this. 


Thanks,
Tharun Kumar P