[PATCH net-next v4 01/10] net: wwan: tmi: Add PCIe core

Yanchao Yang posted 10 patches 1 year, 8 months ago
[PATCH net-next v4 01/10] net: wwan: tmi: Add PCIe core
Posted by Yanchao Yang 1 year, 8 months ago
Registers the TMI device driver with the kernel. Set up all the fundamental
configurations for the device: PCIe layer, Modem Host Cross Core Interface
(MHCCIF), Reset Generation Unit (RGU), modem common control operations and
build infrastructure.

* PCIe layer code implements driver probe and removal, MSI-X interrupt
initialization and de-initialization, and the way of resetting the device.
* MHCCIF provides interrupt channels to communicate events such as handshake,
PM and port enumeration.
* RGU provides interrupt channels to generate notifications from the device
so that the TMI driver could get the device reset.
* Modem common control operations provide the basic read/write functions of
the device's hardware registers, mask/unmask/get/clear functions of the
device's interrupt registers and inquiry functions of the device's status.

Signed-off-by: Yanchao Yang <yanchao.yang@mediatek.com>
Signed-off-by: Ting Wang <ting.wang@mediatek.com>
---
 drivers/net/wwan/Kconfig                 |  14 +
 drivers/net/wwan/Makefile                |   1 +
 drivers/net/wwan/mediatek/Makefile       |   8 +
 drivers/net/wwan/mediatek/mtk_dev.h      | 203 ++++++
 drivers/net/wwan/mediatek/pcie/mtk_pci.c | 887 +++++++++++++++++++++++
 drivers/net/wwan/mediatek/pcie/mtk_pci.h | 144 ++++
 drivers/net/wwan/mediatek/pcie/mtk_reg.h |  69 ++
 7 files changed, 1326 insertions(+)
 create mode 100644 drivers/net/wwan/mediatek/Makefile
 create mode 100644 drivers/net/wwan/mediatek/mtk_dev.h
 create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.c
 create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.h
 create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_reg.h

diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
index 410b0245114e..085562aa8682 100644
--- a/drivers/net/wwan/Kconfig
+++ b/drivers/net/wwan/Kconfig
@@ -120,6 +120,20 @@ config MTK_T7XX
 
 	  If unsure, say N.
 
+config MTK_TMI
+	tristate "TMI Driver for Mediatek T-series Device"
+	depends on (PCI && ACPI)
+	select RELAY if WWAN_DEBUGFS
+	help
+	  This driver enables Mediatek T-series WWAN Device communication.
+	  The TMI is intended for t8xx T-series modem chips. Currently, t7xx
+	  is not supported.
+
+	  If you have one of those Mediatek T-series WWAN Modules and wish to
+	  use it in Linux say Y/M here.
+
+	  If unsure, say N.
+
 endif # WWAN
 
 endmenu
diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
index 3960c0ae2445..198d8074851f 100644
--- a/drivers/net/wwan/Makefile
+++ b/drivers/net/wwan/Makefile
@@ -14,3 +14,4 @@ obj-$(CONFIG_QCOM_BAM_DMUX) += qcom_bam_dmux.o
 obj-$(CONFIG_RPMSG_WWAN_CTRL) += rpmsg_wwan_ctrl.o
 obj-$(CONFIG_IOSM) += iosm/
 obj-$(CONFIG_MTK_T7XX) += t7xx/
+obj-$(CONFIG_MTK_TMI) += mediatek/
diff --git a/drivers/net/wwan/mediatek/Makefile b/drivers/net/wwan/mediatek/Makefile
new file mode 100644
index 000000000000..b74192eee432
--- /dev/null
+++ b/drivers/net/wwan/mediatek/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: BSD-3-Clause-Clear
+
+MODULE_NAME := mtk_tmi
+
+mtk_tmi-y = \
+	pcie/mtk_pci.o
+
+obj-$(CONFIG_MTK_TMI) += mtk_tmi.o
diff --git a/drivers/net/wwan/mediatek/mtk_dev.h b/drivers/net/wwan/mediatek/mtk_dev.h
new file mode 100644
index 000000000000..acda9ccf05f1
--- /dev/null
+++ b/drivers/net/wwan/mediatek/mtk_dev.h
@@ -0,0 +1,203 @@
+/* SPDX-License-Identifier: BSD-3-Clause-Clear
+ *
+ * Copyright (c) 2022, MediaTek Inc.
+ */
+
+#ifndef __MTK_DEV_H__
+#define __MTK_DEV_H__
+
+#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
+
+#define MTK_DEV_STR_LEN 16
+
+enum mtk_irq_src {
+	MTK_IRQ_SRC_MIN,
+	MTK_IRQ_SRC_MHCCIF,
+	MTK_IRQ_SRC_SAP_RGU,
+	MTK_IRQ_SRC_DPMAIF,
+	MTK_IRQ_SRC_DPMAIF2,
+	MTK_IRQ_SRC_CLDMA0,
+	MTK_IRQ_SRC_CLDMA1,
+	MTK_IRQ_SRC_CLDMA2,
+	MTK_IRQ_SRC_CLDMA3,
+	MTK_IRQ_SRC_PM_LOCK,
+	MTK_IRQ_SRC_DPMAIF3,
+	MTK_IRQ_SRC_MAX
+};
+
+enum mtk_user_id {
+	MTK_USER_HW,
+	MTK_USER_CTRL,
+	MTK_USER_DPMAIF,
+	MTK_USER_PM,
+	MTK_USER_EXCEPT,
+	MTK_USER_MAX
+};
+
+enum mtk_l1ss_grp {
+	L1SS_PM,
+	L1SS_EXT_EVT,
+};
+
+#define L1SS_BIT_L1(grp)     BIT(((grp) << 2) + 1)
+#define L1SS_BIT_L1_1(grp)   BIT(((grp) << 2) + 2)
+#define L1SS_BIT_L1_2(grp)   BIT(((grp) << 2) + 3)
+
+struct mtk_md_dev;
+
+/**
+ * struct mtk_hw_ops - The HW layer operations provided to transaction layer.
+ * @read32:         Callback to read 32-bit register.
+ *                  Read value from MD. For PCIe, it's BAR 2/3 MMIO read.
+ * @write32:        Callback to write 32-bit register.
+ *                  Write value to MD. For PCIe, it's BAR 2/3 MMIO write.
+ * @get_dev_state:  Callback to get the device's state.
+ * @ack_dev_state:  Callback to acknowledge device state.
+ * @get_irq_id:     Callback to get the irq id specific IP on a chip.
+ * @get_virq_id:     Callback to get the system virtual IRQ.
+ * @register_irq:   Callback to register callback function to specific hardware IP.
+ * @unregister_irq: Callback to unregister callback function to specific hardware IP.
+ * @mask_irq:       Callback to mask the interrupt of specific hardware IP.
+ * @unmask_irq:     Callback to unmask the interrupt of specific hardware IP.
+ * @clear_irq:      Callback to clear the interrupt of specific hardware IP.
+ * @register_ext_evt:Callback to register HW Layer external event.
+ * @unregister_ext_evt:Callback to unregister HW Layer external event.
+ * @mask_ext_evt:   Callback to mask HW Layer external event.
+ * @unmask_ext_evt: Callback to unmask HW Layer external event.
+ * @clear_ext_evt:  Callback to clear HW Layer external event status.
+ * @send_ext_evt:   Callback to send HW Layer external event.
+ * @get_ext_evt_status:Callback to get HW Layer external event status.
+ * @get_hp_status:  Callback to get link hotplug status.
+ */
+struct mtk_hw_ops {
+	u32 (*read32)(struct mtk_md_dev *mdev, u64 addr);
+	void (*write32)(struct mtk_md_dev *mdev, u64 addr, u32 val);
+	u32 (*get_dev_state)(struct mtk_md_dev *mdev);
+	void (*ack_dev_state)(struct mtk_md_dev *mdev, u32 state);
+	int (*get_irq_id)(struct mtk_md_dev *mdev, enum mtk_irq_src irq_src);
+	int (*get_virq_id)(struct mtk_md_dev *mdev, int irq_id);
+	int (*register_irq)(struct mtk_md_dev *mdev, int irq_id,
+			    int (*irq_cb)(int irq_id, void *data), void *data);
+	int (*unregister_irq)(struct mtk_md_dev *mdev, int irq_id);
+	int (*mask_irq)(struct mtk_md_dev *mdev, int irq_id);
+	int (*unmask_irq)(struct mtk_md_dev *mdev, int irq_id);
+	int (*clear_irq)(struct mtk_md_dev *mdev, int irq_id);
+	int (*register_ext_evt)(struct mtk_md_dev *mdev, u32 chs,
+				int (*evt_cb)(u32 status, void *data), void *data);
+	int (*unregister_ext_evt)(struct mtk_md_dev *mdev, u32 chs);
+	void (*mask_ext_evt)(struct mtk_md_dev *mdev, u32 chs);
+	void (*unmask_ext_evt)(struct mtk_md_dev *mdev, u32 chs);
+	void (*clear_ext_evt)(struct mtk_md_dev *mdev, u32 chs);
+	int (*send_ext_evt)(struct mtk_md_dev *mdev, u32 ch);
+	u32 (*get_ext_evt_status)(struct mtk_md_dev *mdev);
+	int (*get_hp_status)(struct mtk_md_dev *mdev);
+};
+
+struct mtk_md_dev {
+	struct device *dev;
+	const struct mtk_hw_ops *hw_ops;
+	void *hw_priv;
+	u32 hw_ver;
+	int msi_nvecs;
+	char dev_str[MTK_DEV_STR_LEN];
+};
+
+static inline u32 mtk_hw_read32(struct mtk_md_dev *mdev, u64 addr)
+{
+	return mdev->hw_ops->read32(mdev, addr);
+}
+
+static inline void mtk_hw_write32(struct mtk_md_dev *mdev, u64 addr, u32 val)
+{
+	mdev->hw_ops->write32(mdev, addr, val);
+}
+
+static inline u32 mtk_hw_get_dev_state(struct mtk_md_dev *mdev)
+{
+	return mdev->hw_ops->get_dev_state(mdev);
+}
+
+static inline void mtk_hw_ack_dev_state(struct mtk_md_dev *mdev, u32 state)
+{
+	mdev->hw_ops->ack_dev_state(mdev, state);
+}
+
+static inline int mtk_hw_get_irq_id(struct mtk_md_dev *mdev, enum mtk_irq_src irq_src)
+{
+	return mdev->hw_ops->get_irq_id(mdev, irq_src);
+}
+
+static inline int mtk_hw_get_virq_id(struct mtk_md_dev *mdev, int irq_id)
+{
+	return mdev->hw_ops->get_virq_id(mdev, irq_id);
+}
+
+static inline int mtk_hw_register_irq(struct mtk_md_dev *mdev, int irq_id,
+				      int (*irq_cb)(int irq_id, void *data), void *data)
+{
+	return mdev->hw_ops->register_irq(mdev, irq_id, irq_cb, data);
+}
+
+static inline int mtk_hw_unregister_irq(struct mtk_md_dev *mdev, int irq_id)
+{
+	return mdev->hw_ops->unregister_irq(mdev, irq_id);
+}
+
+static inline int mtk_hw_mask_irq(struct mtk_md_dev *mdev, int irq_id)
+{
+	return mdev->hw_ops->mask_irq(mdev, irq_id);
+}
+
+static inline int mtk_hw_unmask_irq(struct mtk_md_dev *mdev, int irq_id)
+{
+	return mdev->hw_ops->unmask_irq(mdev, irq_id);
+}
+
+static inline int mtk_hw_clear_irq(struct mtk_md_dev *mdev, int irq_id)
+{
+	return mdev->hw_ops->clear_irq(mdev, irq_id);
+}
+
+static inline int mtk_hw_register_ext_evt(struct mtk_md_dev *mdev, u32 chs,
+					  int (*evt_cb)(u32 status, void *data), void *data)
+{
+	return mdev->hw_ops->register_ext_evt(mdev, chs, evt_cb, data);
+}
+
+static inline int mtk_hw_unregister_ext_evt(struct mtk_md_dev *mdev, u32 chs)
+{
+	return mdev->hw_ops->unregister_ext_evt(mdev, chs);
+}
+
+static inline void mtk_hw_mask_ext_evt(struct mtk_md_dev *mdev, u32 chs)
+{
+	mdev->hw_ops->mask_ext_evt(mdev, chs);
+}
+
+static inline void mtk_hw_unmask_ext_evt(struct mtk_md_dev *mdev, u32 chs)
+{
+	mdev->hw_ops->unmask_ext_evt(mdev, chs);
+}
+
+static inline void mtk_hw_clear_ext_evt(struct mtk_md_dev *mdev, u32 chs)
+{
+	mdev->hw_ops->clear_ext_evt(mdev, chs);
+}
+
+static inline int mtk_hw_send_ext_evt(struct mtk_md_dev *mdev, u32 ch)
+{
+	return mdev->hw_ops->send_ext_evt(mdev, ch);
+}
+
+static inline u32 mtk_hw_get_ext_evt_status(struct mtk_md_dev *mdev)
+{
+	return mdev->hw_ops->get_ext_evt_status(mdev);
+}
+
+static inline int mtk_hw_get_hp_status(struct mtk_md_dev *mdev)
+{
+	return mdev->hw_ops->get_hp_status(mdev);
+}
+
+#endif /* __MTK_DEV_H__ */
diff --git a/drivers/net/wwan/mediatek/pcie/mtk_pci.c b/drivers/net/wwan/mediatek/pcie/mtk_pci.c
new file mode 100644
index 000000000000..f8590d7319a8
--- /dev/null
+++ b/drivers/net/wwan/mediatek/pcie/mtk_pci.c
@@ -0,0 +1,887 @@
+// SPDX-License-Identifier: BSD-3-Clause-Clear
+/*
+ * Copyright (c) 2022, MediaTek Inc.
+ */
+
+#include <linux/acpi.h>
+#include <linux/aer.h>
+#include <linux/bitfield.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "mtk_pci.h"
+#include "mtk_reg.h"
+
+#define MTK_PCI_TRANSPARENT_ATR_SIZE	(0x3F)
+
+static u32 mtk_pci_mac_read32(struct mtk_pci_priv *priv, u64 addr)
+{
+	return ioread32(priv->mac_reg_base + addr);
+}
+
+static void mtk_pci_mac_write32(struct mtk_pci_priv *priv, u64 addr, u32 val)
+{
+	iowrite32(val, priv->mac_reg_base + addr);
+}
+
+static void mtk_pci_set_msix_merged(struct mtk_pci_priv *priv, int irq_cnt)
+{
+	mtk_pci_mac_write32(priv, REG_PCIE_CFG_MSIX, ffs(irq_cnt) * 2 - 1);
+}
+
+static int mtk_pci_setup_atr(struct mtk_md_dev *mdev, struct mtk_atr_cfg *cfg)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	u32 addr, val, size_h, size_l;
+	int atr_size, pos, offset;
+
+	if (cfg->transparent) {
+		atr_size = MTK_PCI_TRANSPARENT_ATR_SIZE; /* No address conversion is performed */
+	} else {
+		if (cfg->src_addr & (cfg->size - 1)) {
+			dev_err(mdev->dev, "Invalid atr src addr is not aligned to size\n");
+			return -EFAULT;
+		}
+		if (cfg->trsl_addr & (cfg->size - 1)) {
+			dev_err(mdev->dev,
+				"Invalid atr trsl addr is not aligned to size, %llx, %llx\n",
+				cfg->trsl_addr,
+				cfg->size - 1);
+			return -EFAULT;
+		}
+
+		size_l = FIELD_GET(GENMASK_ULL(31, 0), cfg->size);
+		size_h = FIELD_GET(GENMASK_ULL(63, 32), cfg->size);
+
+		pos = ffs(size_l);
+		if (pos) {
+			/* Address Translate Space Size is equal to 2^(atr_size+1).
+			 * "-2" means "-1-1", the first "-1" is because of the atr_size register,
+			 * the second is because of the ffs() will increase by one.
+			 */
+			atr_size = pos - 2;
+		} else {
+			pos = ffs(size_h);
+			/* "+30" means "+32-1-1", the meaning of "-1-1" is same as above,
+			 * "+32" is because atr_size is large, exceeding 32-bits.
+			 */
+			atr_size = pos + 30;
+		}
+	}
+
+	/* Calculate table offset */
+	offset = ATR_PORT_OFFSET * cfg->port + ATR_TABLE_OFFSET * cfg->table;
+	/* SRC_ADDR_H */
+	addr = REG_ATR_PCIE_WIN0_T0_SRC_ADDR_MSB + offset;
+	val = (u32)(cfg->src_addr >> 32);
+	mtk_pci_mac_write32(priv, addr, val);
+	/* SRC_ADDR_L */
+	addr = REG_ATR_PCIE_WIN0_T0_SRC_ADDR_LSB + offset;
+	val = (u32)(cfg->src_addr & 0xFFFFF000) | (atr_size << 1) | 0x1;
+	mtk_pci_mac_write32(priv, addr, val);
+
+	/* TRSL_ADDR_H */
+	addr = REG_ATR_PCIE_WIN0_T0_TRSL_ADDR_MSB + offset;
+	val = (u32)(cfg->trsl_addr >> 32);
+	mtk_pci_mac_write32(priv, addr, val);
+	/* TRSL_ADDR_L */
+	addr = REG_ATR_PCIE_WIN0_T0_TRSL_ADDR_LSB + offset;
+	val = (u32)(cfg->trsl_addr & 0xFFFFF000);
+	mtk_pci_mac_write32(priv, addr, val);
+
+	/* TRSL_PARAM */
+	addr = REG_ATR_PCIE_WIN0_T0_TRSL_PARAM + offset;
+	val = (cfg->trsl_param << 16) | cfg->trsl_id;
+	mtk_pci_mac_write32(priv, addr, val);
+
+	return 0;
+}
+
+static void mtk_pci_atr_disable(struct mtk_pci_priv *priv)
+{
+	int port, tbl, offset;
+
+	/* Disable all ATR table for all ports */
+	for (port = ATR_SRC_PCI_WIN0; port <= ATR_SRC_AXIS_3; port++)
+		for (tbl = 0; tbl < ATR_TABLE_NUM_PER_ATR; tbl++) {
+			/* Calculate table offset */
+			offset = ATR_PORT_OFFSET * port + ATR_TABLE_OFFSET * tbl;
+			/* Disable table by SRC_ADDR_L */
+			mtk_pci_mac_write32(priv, REG_ATR_PCIE_WIN0_T0_SRC_ADDR_LSB + offset, 0);
+		}
+}
+
+static int mtk_pci_atr_init(struct mtk_md_dev *mdev)
+{
+	struct pci_dev *pdev = to_pci_dev(mdev->dev);
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	struct mtk_atr_cfg cfg;
+	int port, ret;
+
+	mtk_pci_atr_disable(priv);
+
+	/* Config ATR for RC to access device's register */
+	cfg.src_addr = pci_resource_start(pdev, MTK_BAR_2_3_IDX);
+	cfg.size = ATR_PCIE_REG_SIZE;
+	cfg.trsl_addr = ATR_PCIE_REG_TRSL_ADDR;
+	cfg.type = ATR_PCI2AXI;
+	cfg.port = ATR_PCIE_REG_PORT;
+	cfg.table = ATR_PCIE_REG_TABLE_NUM;
+	cfg.trsl_id = ATR_PCIE_REG_TRSL_PORT;
+	cfg.trsl_param = 0x0;
+	cfg.transparent = 0x0;
+	ret = mtk_pci_setup_atr(mdev, &cfg);
+	if (ret)
+		return ret;
+
+	/* Config ATR for MHCCIF */
+	cfg.src_addr = pci_resource_start(pdev, MTK_BAR_2_3_IDX);
+	cfg.src_addr += priv->cfg->mhccif_rc_base_addr - ATR_PCIE_REG_TRSL_ADDR;
+	cfg.size = priv->cfg->mhccif_trsl_size;
+	cfg.trsl_addr = priv->cfg->mhccif_rc_reg_trsl_addr;
+	cfg.type = ATR_PCI2AXI;
+	cfg.port = ATR_PCIE_REG_PORT;
+	cfg.table = ART_PCIE_REG_MHCCIF_TABLE_NUM;
+	cfg.trsl_id = ATR_PCIE_REG_TRSL_PORT;
+	cfg.trsl_param = 0x0;
+	cfg.transparent = 0x0;
+	ret = mtk_pci_setup_atr(mdev, &cfg);
+	if (ret)
+		return ret;
+
+	/* Config ATR for EP to access RC's memory */
+	for (port = ATR_PCIE_DEV_DMA_PORT_START; port <= ATR_PCIE_DEV_DMA_PORT_END; port++) {
+		cfg.src_addr = ATR_PCIE_DEV_DMA_SRC_ADDR;
+		cfg.size = ATR_PCIE_DEV_DMA_SIZE;
+		cfg.trsl_addr = ATR_PCIE_DEV_DMA_TRSL_ADDR;
+		cfg.type = ATR_AXI2PCI;
+		cfg.port = port;
+		cfg.table = ATR_PCIE_DEV_DMA_TABLE_NUM;
+		cfg.trsl_id = ATR_DST_PCI_TRX;
+		cfg.trsl_param = 0x0;
+		/* Enable transparent translation */
+		cfg.transparent = ATR_PCIE_DEV_DMA_TRANSPARENT;
+		ret = mtk_pci_setup_atr(mdev, &cfg);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static u32 mtk_pci_read32(struct mtk_md_dev *mdev, u64 addr)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	return ioread32(priv->ext_reg_base + addr);
+}
+
+static void mtk_pci_write32(struct mtk_md_dev *mdev, u64 addr, u32 val)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	iowrite32(val, priv->ext_reg_base + addr);
+}
+
+static u32 mtk_pci_get_dev_state(struct mtk_md_dev *mdev)
+{
+	return mtk_pci_mac_read32(mdev->hw_priv, REG_PCIE_DEBUG_DUMMY_7);
+}
+
+static void mtk_pci_ack_dev_state(struct mtk_md_dev *mdev, u32 state)
+{
+	mtk_pci_mac_write32(mdev->hw_priv, REG_PCIE_DEBUG_DUMMY_7, state);
+}
+
+static int mtk_pci_get_irq_id(struct mtk_md_dev *mdev, enum mtk_irq_src irq_src)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	const int *irq_tbl = priv->cfg->irq_tbl;
+	int irq_id = -EINVAL;
+
+	if (irq_src > MTK_IRQ_SRC_MIN && irq_src < MTK_IRQ_SRC_MAX) {
+		irq_id = irq_tbl[irq_src];
+		if (unlikely(irq_id < 0 || irq_id >= MTK_IRQ_CNT_MAX))
+			irq_id = -EINVAL;
+	}
+
+	return irq_id;
+}
+
+static int mtk_pci_get_virq_id(struct mtk_md_dev *mdev, int irq_id)
+{
+	struct pci_dev *pdev = to_pci_dev(mdev->dev);
+	int nr = 0;
+
+	if (pdev->msix_enabled)
+		nr = irq_id % mdev->msi_nvecs;
+
+	return pci_irq_vector(pdev, nr);
+}
+
+static int mtk_pci_register_irq(struct mtk_md_dev *mdev, int irq_id,
+				int (*irq_cb)(int irq_id, void *data), void *data)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	if (unlikely((irq_id < 0 || irq_id >= MTK_IRQ_CNT_MAX) || !irq_cb))
+		return -EINVAL;
+
+	if (priv->irq_cb_list[irq_id]) {
+		dev_err(mdev->dev,
+			"Unable to register irq, irq_id=%d, it's already been register by %ps.\n",
+			irq_id, priv->irq_cb_list[irq_id]);
+		return -EFAULT;
+	}
+	priv->irq_cb_list[irq_id] = irq_cb;
+	priv->irq_cb_data[irq_id] = data;
+
+	return 0;
+}
+
+static int mtk_pci_unregister_irq(struct mtk_md_dev *mdev, int irq_id)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	if (unlikely(irq_id < 0 || irq_id >= MTK_IRQ_CNT_MAX))
+		return -EINVAL;
+
+	if (!priv->irq_cb_list[irq_id]) {
+		dev_err(mdev->dev, "irq_id=%d has not been registered\n", irq_id);
+		return -EFAULT;
+	}
+	priv->irq_cb_list[irq_id] = NULL;
+	priv->irq_cb_data[irq_id] = NULL;
+
+	return 0;
+}
+
+static int mtk_pci_mask_irq(struct mtk_md_dev *mdev, int irq_id)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	if (unlikely((irq_id < 0 || irq_id >= MTK_IRQ_CNT_MAX) || priv->irq_type != PCI_IRQ_MSIX)) {
+		dev_err(mdev->dev, "Failed to mask irq: input irq_id=%d\n", irq_id);
+		return -EINVAL;
+	}
+
+	mtk_pci_mac_write32(priv, REG_IMASK_HOST_MSIX_CLR_GRP0_0, BIT(irq_id));
+
+	return 0;
+}
+
+static int mtk_pci_unmask_irq(struct mtk_md_dev *mdev, int irq_id)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	if (unlikely((irq_id < 0 || irq_id >= MTK_IRQ_CNT_MAX) || priv->irq_type != PCI_IRQ_MSIX)) {
+		dev_err(mdev->dev, "Failed to unmask irq: input irq_id=%d\n", irq_id);
+		return -EINVAL;
+	}
+
+	mtk_pci_mac_write32(priv, REG_IMASK_HOST_MSIX_SET_GRP0_0, BIT(irq_id));
+
+	return 0;
+}
+
+static int mtk_pci_clear_irq(struct mtk_md_dev *mdev, int irq_id)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	if (unlikely((irq_id < 0 || irq_id >= MTK_IRQ_CNT_MAX) || priv->irq_type != PCI_IRQ_MSIX)) {
+		dev_err(mdev->dev, "Failed to clear irq: input irq_id=%d\n", irq_id);
+		return -EINVAL;
+	}
+
+	mtk_pci_mac_write32(priv, REG_MSIX_ISTATUS_HOST_GRP0_0, BIT(irq_id));
+
+	return 0;
+}
+
+static int mtk_mhccif_register_evt(struct mtk_md_dev *mdev, u32 chs,
+				   int (*evt_cb)(u32 status, void *data), void *data)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	struct mtk_mhccif_cb *cb;
+	unsigned long flag;
+	int ret = 0;
+
+	if (!chs || !evt_cb)
+		return -EINVAL;
+
+	spin_lock_irqsave(&priv->mhccif_lock, flag);
+	list_for_each_entry(cb, &priv->mhccif_cb_list, entry) {
+		if (cb->chs & chs) {
+			ret = -EFAULT;
+			dev_err(mdev->dev,
+				"Unable to register evt, chs=0x%08X&0x%08X registered_cb=%ps\n",
+				chs, cb->chs, cb->evt_cb);
+			goto err;
+		}
+	}
+	cb = devm_kzalloc(mdev->dev, sizeof(*cb), GFP_ATOMIC);
+	if (!cb) {
+		ret = -ENOMEM;
+		goto err;
+	}
+	cb->evt_cb = evt_cb;
+	cb->data = data;
+	cb->chs = chs;
+	list_add_tail(&cb->entry, &priv->mhccif_cb_list);
+
+err:
+	spin_unlock_irqrestore(&priv->mhccif_lock, flag);
+
+	return ret;
+}
+
+static int mtk_mhccif_unregister_evt(struct mtk_md_dev *mdev, u32 chs)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	struct mtk_mhccif_cb *cb, *next;
+	unsigned long flag;
+	int ret = 0;
+
+	if (!chs)
+		return -EINVAL;
+
+	spin_lock_irqsave(&priv->mhccif_lock, flag);
+	list_for_each_entry_safe(cb, next, &priv->mhccif_cb_list, entry) {
+		if (cb->chs == chs) {
+			list_del(&cb->entry);
+			devm_kfree(mdev->dev, cb);
+			goto out;
+		}
+	}
+	ret = -EFAULT;
+	dev_warn(mdev->dev, "Unable to unregister evt, no chs=0x%08X has been registered.\n", chs);
+out:
+	spin_unlock_irqrestore(&priv->mhccif_lock, flag);
+
+	return ret;
+}
+
+static void mtk_mhccif_mask_evt(struct mtk_md_dev *mdev, u32 chs)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	mtk_pci_write32(mdev, priv->cfg->mhccif_rc_base_addr +
+			MHCCIF_EP2RC_SW_INT_EAP_MASK_SET, chs);
+}
+
+static void mtk_mhccif_unmask_evt(struct mtk_md_dev *mdev, u32 chs)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	mtk_pci_write32(mdev, priv->cfg->mhccif_rc_base_addr +
+			MHCCIF_EP2RC_SW_INT_EAP_MASK_CLR, chs);
+}
+
+static void mtk_mhccif_clear_evt(struct mtk_md_dev *mdev, u32 chs)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	mtk_pci_write32(mdev, priv->cfg->mhccif_rc_base_addr +
+			MHCCIF_EP2RC_SW_INT_ACK, chs);
+}
+
+static int mtk_mhccif_send_evt(struct mtk_md_dev *mdev, u32 ch)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	u32 rc_base;
+
+	rc_base = priv->cfg->mhccif_rc_base_addr;
+	/* Only allow one ch to be triggered at a time */
+	if ((ch & (ch - 1)) || !ch) {
+		dev_err(mdev->dev, "Unsupported ext evt ch=0x%08X\n", ch);
+		return -EINVAL;
+	}
+
+	mtk_pci_write32(mdev, rc_base + MHCCIF_RC2EP_SW_BSY, ch);
+	mtk_pci_write32(mdev, rc_base + MHCCIF_RC2EP_SW_TCHNUM, ffs(ch) - 1);
+
+	return 0;
+}
+
+static u32 mtk_mhccif_get_evt_status(struct mtk_md_dev *mdev)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	return mtk_pci_read32(mdev, priv->cfg->mhccif_rc_base_addr + MHCCIF_EP2RC_SW_INT_STS);
+}
+
+static int mtk_pci_acpi_reset(struct mtk_md_dev *mdev, char *fn_name)
+{
+#ifdef CONFIG_ACPI
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	acpi_status acpi_ret;
+	acpi_handle handle;
+	int ret = 0;
+
+	handle = ACPI_HANDLE(mdev->dev);
+	if (!handle) {
+		dev_err(mdev->dev, "Unsupported, acpi handle isn't found\n");
+		ret = -ENODEV;
+		goto err;
+	}
+	if (!acpi_has_method(handle, fn_name)) {
+		dev_err(mdev->dev, "Unsupported, _RST method isn't found\n");
+		ret = -ENODEV;
+		goto err;
+	}
+	acpi_ret = acpi_evaluate_object(handle, fn_name, NULL, &buffer);
+	if (ACPI_FAILURE(acpi_ret)) {
+		dev_err(mdev->dev, "Failed to execute %s method: %s\n",
+			fn_name,
+			acpi_format_exception(acpi_ret));
+		ret = -EFAULT;
+		goto err;
+	}
+	dev_info(mdev->dev, "FLDR execute successfully\n");
+	acpi_os_free(buffer.pointer);
+err:
+	return ret;
+#else
+	dev_err(mdev->dev, "Unsupported, CONFIG ACPI hasn't been set to 'y'\n");
+	return -ENODEV;
+#endif
+}
+
+static int mtk_pci_fldr(struct mtk_md_dev *mdev)
+{
+	return mtk_pci_acpi_reset(mdev, "_RST");
+}
+
+static bool mtk_pci_link_check(struct mtk_md_dev *mdev)
+{
+	return !pci_device_is_present(to_pci_dev(mdev->dev));
+}
+
+static int mtk_pci_get_hp_status(struct mtk_md_dev *mdev)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	return priv->rc_hp_on;
+}
+
+static const struct mtk_hw_ops mtk_pci_ops = {
+	.read32                = mtk_pci_read32,
+	.write32               = mtk_pci_write32,
+	.get_dev_state         = mtk_pci_get_dev_state,
+	.ack_dev_state         = mtk_pci_ack_dev_state,
+	.get_irq_id            = mtk_pci_get_irq_id,
+	.get_virq_id           = mtk_pci_get_virq_id,
+	.register_irq          = mtk_pci_register_irq,
+	.unregister_irq        = mtk_pci_unregister_irq,
+	.mask_irq              = mtk_pci_mask_irq,
+	.unmask_irq            = mtk_pci_unmask_irq,
+	.clear_irq             = mtk_pci_clear_irq,
+	.register_ext_evt      = mtk_mhccif_register_evt,
+	.unregister_ext_evt    = mtk_mhccif_unregister_evt,
+	.mask_ext_evt          = mtk_mhccif_mask_evt,
+	.unmask_ext_evt        = mtk_mhccif_unmask_evt,
+	.clear_ext_evt         = mtk_mhccif_clear_evt,
+	.send_ext_evt          = mtk_mhccif_send_evt,
+	.get_ext_evt_status    = mtk_mhccif_get_evt_status,
+	.get_hp_status         = mtk_pci_get_hp_status,
+};
+
+static void mtk_mhccif_isr_work(struct work_struct *work)
+{
+	struct mtk_pci_priv *priv = container_of(work, struct mtk_pci_priv, mhccif_work);
+	struct mtk_md_dev *mdev = priv->irq_desc->mdev;
+	struct mtk_mhccif_cb *cb;
+	unsigned long flag;
+	u32 stat, mask;
+
+	stat = mtk_mhccif_get_evt_status(mdev);
+	mask = mtk_pci_read32(mdev, priv->cfg->mhccif_rc_base_addr
+		+ MHCCIF_EP2RC_SW_INT_EAP_MASK);
+	dev_dbg(mdev->dev, "External events: mhccif_stat=0x%08X mask=0x%08X\n", stat, mask);
+
+	if (unlikely(stat == U32_MAX && mtk_pci_link_check(mdev))) {
+		/* When link failed, we don't need to unmask/clear. */
+		dev_err(mdev->dev, "Failed to check link in MHCCIF handler.\n");
+		return;
+	}
+
+	stat &= ~mask;
+	spin_lock_irqsave(&priv->mhccif_lock, flag);
+	list_for_each_entry(cb, &priv->mhccif_cb_list, entry) {
+		if (cb->chs & stat)
+			cb->evt_cb(cb->chs & stat, cb->data);
+	}
+	spin_unlock_irqrestore(&priv->mhccif_lock, flag);
+
+	mtk_pci_clear_irq(mdev, priv->mhccif_irq_id);
+	mtk_pci_unmask_irq(mdev, priv->mhccif_irq_id);
+}
+
+static const struct mtk_pci_dev_cfg mtk_dev_cfg_0800 = {
+	.mhccif_rc_base_addr = 0x10012000,
+	.mhccif_trsl_size = 0x2000,
+	.mhccif_rc_reg_trsl_addr = 0x12020000,
+	.istatus_host_ctrl_addr = REG_ISTATUS_HOST_CTRL_NEW,
+	.irq_tbl = {
+		[MTK_IRQ_SRC_DPMAIF]  = 24,
+		[MTK_IRQ_SRC_CLDMA0]  = 25,
+		[MTK_IRQ_SRC_CLDMA1]  = 26,
+		[MTK_IRQ_SRC_CLDMA2]  = 27,
+		[MTK_IRQ_SRC_MHCCIF]  = 28,
+		[MTK_IRQ_SRC_DPMAIF2] = 29,
+		[MTK_IRQ_SRC_SAP_RGU] = 30,
+		[MTK_IRQ_SRC_CLDMA3]  = 31,
+		[MTK_IRQ_SRC_PM_LOCK] = 0,
+		[MTK_IRQ_SRC_DPMAIF3] = 7,
+	},
+};
+
+static const struct pci_device_id mtk_pci_ids[] = {
+	MTK_PCI_DEV_CFG(0x0800, mtk_dev_cfg_0800),
+	{ /* end: all zeroes */ }
+};
+MODULE_DEVICE_TABLE(pci, mtk_pci_ids);
+
+static int mtk_pci_bar_init(struct mtk_md_dev *mdev)
+{
+	struct pci_dev *pdev = to_pci_dev(mdev->dev);
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	int ret;
+
+	ret = pcim_iomap_regions(pdev, MTK_REQUESTED_BARS, mdev->dev_str);
+	if (ret) {
+		dev_err(mdev->dev, "Failed to init MMIO. ret=%d\n", ret);
+		return ret;
+	}
+
+	/* get ioremapped memory */
+	priv->mac_reg_base = pcim_iomap_table(pdev)[MTK_BAR_0_1_IDX];
+	priv->bar23_addr = pcim_iomap_table(pdev)[MTK_BAR_2_3_IDX];
+	dev_info(mdev->dev, "BAR0/1 Addr=0x%p, BAR2/3 Addr=0x%p\n",
+		 priv->mac_reg_base, priv->bar23_addr);
+	/* We use MD view base address "0" to observe registers */
+	priv->ext_reg_base = priv->bar23_addr - ATR_PCIE_REG_TRSL_ADDR;
+
+	return 0;
+}
+
+static void mtk_pci_bar_exit(struct mtk_md_dev *mdev)
+{
+	pcim_iounmap_regions(to_pci_dev(mdev->dev), MTK_REQUESTED_BARS);
+}
+
+static int mtk_mhccif_irq_cb(int irq_id, void *data)
+{
+	struct mtk_md_dev *mdev = data;
+	struct mtk_pci_priv *priv;
+
+	priv = mdev->hw_priv;
+	queue_work(system_highpri_wq, &priv->mhccif_work);
+
+	return 0;
+}
+
+static int mtk_mhccif_init(struct mtk_md_dev *mdev)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	int ret;
+
+	INIT_LIST_HEAD(&priv->mhccif_cb_list);
+	spin_lock_init(&priv->mhccif_lock);
+	INIT_WORK(&priv->mhccif_work, mtk_mhccif_isr_work);
+
+	ret = mtk_pci_get_irq_id(mdev, MTK_IRQ_SRC_MHCCIF);
+	if (ret < 0) {
+		dev_err(mdev->dev, "Failed to get mhccif_irq_id. ret=%d\n", ret);
+		goto err;
+	}
+	priv->mhccif_irq_id = ret;
+
+	ret = mtk_pci_register_irq(mdev, priv->mhccif_irq_id, mtk_mhccif_irq_cb, mdev);
+	if (ret) {
+		dev_err(mdev->dev, "Failed to register mhccif_irq callback\n");
+		goto err;
+	}
+
+	/* To check if the device rebooted.
+	 * The reboot of some PC doesn't cause the device power cycle.
+	 */
+	mtk_pci_read32(mdev, priv->cfg->mhccif_rc_base_addr + MHCCIF_EP2RC_SW_INT_EAP_MASK);
+
+err:
+	return ret;
+}
+
+static void mtk_mhccif_exit(struct mtk_md_dev *mdev)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+
+	mtk_pci_unregister_irq(mdev, priv->mhccif_irq_id);
+	cancel_work_sync(&priv->mhccif_work);
+}
+
+static irqreturn_t mtk_pci_irq_handler(struct mtk_md_dev *mdev, u32 irq_state)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	int irq_id;
+
+	/* Check whether each set bit has a callback, if has, call it */
+	do {
+		irq_id = fls(irq_state) - 1;
+		irq_state &= ~BIT(irq_id);
+		if (likely(priv->irq_cb_list[irq_id]))
+			priv->irq_cb_list[irq_id](irq_id, priv->irq_cb_data[irq_id]);
+		else
+			dev_err(mdev->dev, "Unhandled irq_id=%d, no callback for it.\n", irq_id);
+	} while (irq_state);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t mtk_pci_irq_msix(int irq, void *data)
+{
+	struct mtk_pci_irq_desc *irq_desc = data;
+	struct mtk_md_dev *mdev = irq_desc->mdev;
+	struct mtk_pci_priv *priv;
+	u32 irq_state, irq_enable;
+
+	priv = mdev->hw_priv;
+	irq_state = mtk_pci_mac_read32(priv, REG_MSIX_ISTATUS_HOST_GRP0_0);
+	irq_enable = mtk_pci_mac_read32(priv, REG_IMASK_HOST_MSIX_GRP0_0);
+	dev_dbg(mdev->dev, "irq_state=0x%08X, irq_enable=0x%08X, msix_bits=0x%08X\n",
+		irq_state, irq_enable, irq_desc->msix_bits);
+	irq_state &= irq_enable;
+
+	if (unlikely(!irq_state) ||
+	    unlikely(!((irq_state % GENMASK(priv->irq_cnt - 1, 0)) & irq_desc->msix_bits)))
+		return IRQ_NONE;
+
+	/* Mask the bit and user needs to unmask by itself */
+	mtk_pci_mac_write32(priv, REG_IMASK_HOST_MSIX_CLR_GRP0_0, irq_state & ~BIT(30));
+
+	return mtk_pci_irq_handler(mdev, irq_state);
+}
+
+static int mtk_pci_request_irq_msix(struct mtk_md_dev *mdev, int irq_cnt_allocated)
+{
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	struct mtk_pci_irq_desc *irq_desc;
+	struct pci_dev *pdev;
+	int irq_cnt;
+	int ret, i;
+
+	/* calculate the nearest 2's power number */
+	irq_cnt = BIT(fls(irq_cnt_allocated) - 1);
+	pdev = to_pci_dev(mdev->dev);
+	irq_desc = priv->irq_desc;
+	for (i = 0; i < irq_cnt; i++) {
+		irq_desc[i].mdev = mdev;
+		irq_desc[i].msix_bits = BIT(i);
+		snprintf(irq_desc[i].name, MTK_IRQ_NAME_LEN, "msix%d-%s",
+			 i, pci_name(to_pci_dev(mdev->dev)));
+		ret = pci_request_irq(pdev, i, mtk_pci_irq_msix, NULL,
+				      &irq_desc[i], irq_desc[i].name);
+		if (ret) {
+			dev_err(mdev->dev, "Failed to request %s: ret=%d\n", irq_desc[i].name, ret);
+			for (i--; i >= 0; i--)
+				pci_free_irq(pdev, i, &irq_desc[i]);
+			return ret;
+		}
+	}
+	priv->irq_cnt = irq_cnt;
+	priv->irq_type = PCI_IRQ_MSIX;
+
+	if (irq_cnt != MTK_IRQ_CNT_MAX)
+		mtk_pci_set_msix_merged(priv, irq_cnt);
+
+	return 0;
+}
+
+static int mtk_pci_request_irq(struct mtk_md_dev *mdev)
+{
+	struct pci_dev *pdev = to_pci_dev(mdev->dev);
+	int irq_cnt;
+
+	irq_cnt = pci_alloc_irq_vectors(pdev, MTK_IRQ_CNT_MIN, MTK_IRQ_CNT_MAX, PCI_IRQ_MSIX);
+	mdev->msi_nvecs = irq_cnt;
+
+	if (irq_cnt < MTK_IRQ_CNT_MIN) {
+		dev_err(mdev->dev,
+			"Unable to alloc pci irq vectors. ret=%d maxirqcnt=%d irqtype=0x%x\n",
+			irq_cnt, MTK_IRQ_CNT_MAX, PCI_IRQ_MSIX);
+		return -EFAULT;
+	}
+
+	return mtk_pci_request_irq_msix(mdev, irq_cnt);
+}
+
+static void mtk_pci_free_irq(struct mtk_md_dev *mdev)
+{
+	struct pci_dev *pdev = to_pci_dev(mdev->dev);
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	int i;
+
+	for (i = 0; i < priv->irq_cnt; i++)
+		pci_free_irq(pdev, i, &priv->irq_desc[i]);
+
+	pci_free_irq_vectors(pdev);
+}
+
+static int mtk_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct device *dev = &pdev->dev;
+	struct mtk_pci_priv *priv;
+	struct mtk_md_dev *mdev;
+	int ret;
+
+	mdev = devm_kzalloc(dev, sizeof(*mdev), GFP_KERNEL);
+	if (!mdev) {
+		ret = -ENOMEM;
+		goto exit;
+	}
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto free_cntx_data;
+	}
+
+	pci_set_drvdata(pdev, mdev);
+	priv->cfg = (struct mtk_pci_dev_cfg *)id->driver_data;
+	priv->mdev = mdev;
+	mdev->hw_ver  = pdev->device;
+	mdev->hw_ops  = &mtk_pci_ops;
+	mdev->hw_priv = priv;
+	mdev->dev     = dev;
+	snprintf(mdev->dev_str, MTK_DEV_STR_LEN, "%02x%02x%d",
+		 pdev->bus->number, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+
+	dev_info(mdev->dev, "Start probe 0x%x, state_saved[%d]\n",
+		 mdev->hw_ver, pdev->state_saved);
+
+	if (pdev->state_saved)
+		pci_restore_state(pdev);
+
+	/* enable host to device access. */
+	ret = pcim_enable_device(pdev);
+	if (ret) {
+		dev_err(mdev->dev, "Failed to enable pci device.\n");
+		goto free_priv_data;
+	}
+
+	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+	if (ret) {
+		dev_err(mdev->dev, "Failed to set DMA Mask and Coherent. (ret=%d)\n", ret);
+		goto disable_device;
+	}
+
+	ret = mtk_pci_bar_init(mdev);
+	if (ret)
+		goto disable_device;
+
+	ret = mtk_pci_atr_init(mdev);
+	if (ret)
+		goto free_bar;
+
+	ret = mtk_mhccif_init(mdev);
+	if (ret)
+		goto free_bar;
+
+	ret = mtk_pci_request_irq(mdev);
+	if (ret)
+		goto free_mhccif;
+
+	/* enable device to host interrupt. */
+	pci_set_master(pdev);
+
+	mtk_pci_unmask_irq(mdev, priv->mhccif_irq_id);
+
+	dev_info(mdev->dev, "Probe done hw_ver=0x%x\n", mdev->hw_ver);
+	return 0;
+
+free_mhccif:
+	mtk_mhccif_exit(mdev);
+free_bar:
+	mtk_pci_bar_exit(mdev);
+disable_device:
+	pci_disable_device(pdev);
+free_priv_data:
+	devm_kfree(dev, priv);
+free_cntx_data:
+	devm_kfree(dev, mdev);
+exit:
+
+	return ret;
+}
+
+static void mtk_pci_remove(struct pci_dev *pdev)
+{
+	struct mtk_md_dev *mdev = pci_get_drvdata(pdev);
+	struct mtk_pci_priv *priv = mdev->hw_priv;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	mtk_pci_mask_irq(mdev, priv->mhccif_irq_id);
+
+	ret = mtk_pci_fldr(mdev);
+	if (ret)
+		mtk_mhccif_send_evt(mdev, EXT_EVT_H2D_DEVICE_RESET);
+
+	pci_clear_master(pdev);
+	mtk_mhccif_exit(mdev);
+	mtk_pci_free_irq(mdev);
+	mtk_pci_bar_exit(mdev);
+	pci_disable_device(pdev);
+	pci_load_and_free_saved_state(pdev, &priv->saved_state);
+	devm_kfree(dev, priv);
+	devm_kfree(dev, mdev);
+	dev_info(dev, "Remove done, state_saved[%d]\n", pdev->state_saved);
+}
+
+static pci_ers_result_t mtk_pci_error_detected(struct pci_dev *pdev,
+					       pci_channel_state_t state)
+{
+	return PCI_ERS_RESULT_NEED_RESET;
+}
+
+static pci_ers_result_t mtk_pci_slot_reset(struct pci_dev *pdev)
+{
+	return PCI_ERS_RESULT_RECOVERED;
+}
+
+static void mtk_pci_io_resume(struct pci_dev *pdev)
+{
+}
+
+static const struct pci_error_handlers mtk_pci_err_handler = {
+	.error_detected = mtk_pci_error_detected,
+	.slot_reset = mtk_pci_slot_reset,
+	.resume = mtk_pci_io_resume,
+};
+
+static struct pci_driver mtk_pci_drv = {
+	.name = "mtk_pci_drv",
+	.id_table = mtk_pci_ids,
+
+	.probe =    mtk_pci_probe,
+	.remove =   mtk_pci_remove,
+
+	.err_handler = &mtk_pci_err_handler
+};
+
+static int __init mtk_drv_init(void)
+{
+	return pci_register_driver(&mtk_pci_drv);
+}
+module_init(mtk_drv_init);
+
+static void __exit mtk_drv_exit(void)
+{
+	pci_unregister_driver(&mtk_pci_drv);
+}
+module_exit(mtk_drv_exit);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/wwan/mediatek/pcie/mtk_pci.h b/drivers/net/wwan/mediatek/pcie/mtk_pci.h
new file mode 100644
index 000000000000..b487ca9b302e
--- /dev/null
+++ b/drivers/net/wwan/mediatek/pcie/mtk_pci.h
@@ -0,0 +1,144 @@
+/* SPDX-License-Identifier: BSD-3-Clause-Clear
+ *
+ * Copyright (c) 2022, MediaTek Inc.
+ */
+
+#ifndef __MTK_PCI_H__
+#define __MTK_PCI_H__
+
+#include <linux/pci.h>
+#include <linux/spinlock.h>
+
+#include "../mtk_dev.h"
+
+enum mtk_atr_type {
+	ATR_PCI2AXI = 0,
+	ATR_AXI2PCI
+};
+
+enum mtk_atr_src_port {
+	ATR_SRC_PCI_WIN0 = 0,
+	ATR_SRC_AXIS_0 = 2,
+	ATR_SRC_AXIS_2 = 3,
+	ATR_SRC_AXIS_3 = 4,
+};
+
+enum mtk_atr_dst_port {
+	ATR_DST_PCI_TRX = 0,
+	ATR_DST_AXIM_0 = 4,
+};
+
+#define ATR_PORT_OFFSET                 0x100
+#define ATR_TABLE_OFFSET                0x20
+#define ATR_TABLE_NUM_PER_ATR           8
+#define ATR_WIN0_SRC_ADDR_LSB_DEFT      0x0000007f
+#define ATR_PCIE_REG_TRSL_ADDR          0x10000000
+#define ATR_PCIE_REG_SIZE               0x00400000
+#define ATR_PCIE_REG_PORT               ATR_SRC_PCI_WIN0
+#define ATR_PCIE_REG_TABLE_NUM          1
+#define ART_PCIE_REG_MHCCIF_TABLE_NUM	0
+#define ATR_PCIE_REG_TRSL_PORT          ATR_DST_AXIM_0
+#define ATR_PCIE_DEV_DMA_PORT_START     ATR_SRC_AXIS_0
+#define ATR_PCIE_DEV_DMA_PORT_END       ATR_SRC_AXIS_2
+#define ATR_PCIE_DEV_DMA_SRC_ADDR       0x00000000
+#define ATR_PCIE_DEV_DMA_TRANSPARENT    1
+#define ATR_PCIE_DEV_DMA_SIZE           0
+#define ATR_PCIE_DEV_DMA_TABLE_NUM      0
+#define ATR_PCIE_DEV_DMA_TRSL_ADDR      0x00000000
+
+#define MTK_BAR_0_1_IDX        0
+#define MTK_BAR_2_3_IDX        2
+/* Only use BAR0/1 and 2/3, so we should input 0b0101 for the two bar,
+ * Input 0xf would cause error.
+ */
+#define MTK_REQUESTED_BARS     ((1 << MTK_BAR_0_1_IDX) | (1 << MTK_BAR_2_3_IDX))
+
+#define MTK_IRQ_CNT_MIN        1
+#define MTK_IRQ_CNT_MAX        32
+#define MTK_IRQ_NAME_LEN	20
+
+#define MTK_INVAL_IRQ_SRC      -1
+
+#define MTK_FORCE_MAC_ACTIVE_BIT	BIT(6)
+#define MTK_DS_LOCK_REG_BIT		BIT(7)
+
+/* mhccif registers */
+#define MHCCIF_RC2EP_SW_BSY                0x4
+#define MHCCIF_RC2EP_SW_INT_START          0x8
+#define MHCCIF_RC2EP_SW_TCHNUM             0xC
+#define MHCCIF_EP2RC_SW_INT_STS            0x10
+#define MHCCIF_EP2RC_SW_INT_ACK            0x14
+#define MHCCIF_EP2RC_SW_INT_EAP_MASK       0x20
+#define MHCCIF_EP2RC_SW_INT_EAP_MASK_SET   0x30
+#define MHCCIF_EP2RC_SW_INT_EAP_MASK_CLR   0x40
+#define MHCCIF_RC2EP_PCIE_PM_COUNTER       0x12C
+
+#define MTK_PCI_CLASS         0x0D4000
+#define MTK_PCI_VENDOR_ID     0x14C3
+
+#define MTK_DISABLE_DS_BIT(grp)	BIT(grp)
+#define MTK_ENABLE_DS_BIT(grp)	BIT((grp) << 8)
+
+#define MTK_PCI_DEV_CFG(id, cfg) \
+{ \
+	PCI_DEVICE(MTK_PCI_VENDOR_ID, id), \
+	MTK_PCI_CLASS, PCI_ANY_ID, \
+	.driver_data = (kernel_ulong_t)&(cfg), \
+}
+
+struct mtk_pci_dev_cfg {
+	u32 flag;
+	u32 mhccif_rc_base_addr;
+	u32 mhccif_rc_reg_trsl_addr;
+	u32 mhccif_trsl_size;
+	u32 istatus_host_ctrl_addr;
+	int irq_tbl[MTK_IRQ_SRC_MAX];
+};
+
+struct mtk_pci_irq_desc {
+	struct mtk_md_dev *mdev;
+	u32 msix_bits;
+	char name[MTK_IRQ_NAME_LEN];
+};
+
+struct mtk_pci_priv {
+	const struct mtk_pci_dev_cfg *cfg;
+	void *mdev;
+	void __iomem *bar23_addr;
+	void __iomem *mac_reg_base;
+	void __iomem *ext_reg_base;
+	int rc_hp_on; /* Bridge hotplug status */
+	int irq_cnt;
+	int irq_type;
+	void *irq_cb_data[MTK_IRQ_CNT_MAX];
+
+	int (*irq_cb_list[MTK_IRQ_CNT_MAX])(int irq_id, void *data);
+	struct mtk_pci_irq_desc irq_desc[MTK_IRQ_CNT_MAX];
+	struct list_head mhccif_cb_list;
+	/* mhccif_lock: lock to protect mhccif_cb_list */
+	spinlock_t mhccif_lock;
+	struct work_struct mhccif_work;
+	int mhccif_irq_id;
+	struct pci_saved_state *saved_state;
+};
+
+struct mtk_mhccif_cb {
+	struct list_head entry;
+	int (*evt_cb)(u32 status, void *data);
+	void *data;
+	u32 chs;
+};
+
+struct mtk_atr_cfg {
+	u64 src_addr;
+	u64 trsl_addr;
+	u64 size;
+	u32 type;      /* Port type */
+	u32 port;      /* Port number */
+	u32 table;     /* Table number (8 tables for each port) */
+	u32 trsl_id;
+	u32 trsl_param;
+	u32 transparent;
+};
+
+#endif /* __MTK_PCI_H__ */
diff --git a/drivers/net/wwan/mediatek/pcie/mtk_reg.h b/drivers/net/wwan/mediatek/pcie/mtk_reg.h
new file mode 100644
index 000000000000..23fa7fd9518e
--- /dev/null
+++ b/drivers/net/wwan/mediatek/pcie/mtk_reg.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: BSD-3-Clause-Clear
+ *
+ * Copyright (c) 2022, MediaTek Inc.
+ */
+
+#ifndef __MTK_REG_H__
+#define __MTK_REG_H__
+
+enum mtk_ext_evt_h2d {
+	EXT_EVT_H2D_EXCEPT_ACK             = 1 << 1,
+	EXT_EVT_H2D_EXCEPT_CLEARQ_ACK      = 1 << 2,
+	EXT_EVT_H2D_PCIE_DS_LOCK           = 1 << 3,
+	EXT_EVT_H2D_RESERVED_FOR_DPMAIF    = 1 << 8,
+	EXT_EVT_H2D_PCIE_PM_SUSPEND_REQ    = 1 << 9,
+	EXT_EVT_H2D_PCIE_PM_RESUME_REQ     = 1 << 10,
+	EXT_EVT_H2D_PCIE_PM_SUSPEND_REQ_AP = 1 << 11,
+	EXT_EVT_H2D_PCIE_PM_RESUME_REQ_AP  = 1 << 12,
+	EXT_EVT_H2D_DEVICE_RESET           = 1 << 13,
+};
+
+#define REG_PCIE_SW_TRIG_INT			0x00BC
+#define REG_PCIE_LTSSM_STATUS			0x0150
+#define REG_IMASK_LOCAL				0x0180
+#define REG_ISTATUS_LOCAL			0x0184
+#define REG_INT_ENABLE_HOST			0x0188
+#define REG_ISTATUS_HOST			0x018C
+#define REG_PCIE_LOW_POWER_CTRL			0x0194
+#define REG_ISTATUS_HOST_CTRL			0x01AC
+#define REG_ISTATUS_PENDING_ADT			0x01D4
+#define REG_INT_ENABLE_HOST_SET			0x01F0
+#define REG_INT_ENABLE_HOST_CLR			0x01F4
+#define REG_PCIE_DMA_DUMMY_0			0x01F8
+#define REG_ISTATUS_HOST_CTRL_NEW		0x031C
+#define REG_PCIE_MISC_CTRL			0x0348
+#define REG_PCIE_DUMMY_0			0x03C0
+#define REG_SW_TRIG_INTR_SET			0x03C8
+#define REG_SW_TRIG_INTR_CLR			0x03CC
+#define REG_PCIE_CFG_MSIX			0x03EC
+#define REG_ATR_PCIE_WIN0_T0_SRC_ADDR_LSB	0x0600
+#define REG_ATR_PCIE_WIN0_T0_SRC_ADDR_MSB	0x0604
+#define REG_ATR_PCIE_WIN0_T0_TRSL_ADDR_LSB	0x0608
+#define REG_ATR_PCIE_WIN0_T0_TRSL_ADDR_MSB	0x060C
+#define REG_ATR_PCIE_WIN0_T0_TRSL_PARAM		0x0610
+#define REG_PCIE_DEBUG_DUMMY_0			0x0D00
+#define REG_PCIE_DEBUG_DUMMY_1			0x0D04
+#define REG_PCIE_DEBUG_DUMMY_2			0x0D08
+#define REG_PCIE_DEBUG_DUMMY_3			0x0D0C
+#define REG_PCIE_DEBUG_DUMMY_4			0x0D10
+#define REG_PCIE_DEBUG_DUMMY_5			0x0D14
+#define REG_PCIE_DEBUG_DUMMY_6			0x0D18
+#define REG_PCIE_DEBUG_DUMMY_7			0x0D1C
+#define REG_PCIE_RESOURCE_STATUS		0x0D28
+#define REG_RC2EP_SW_TRIG_LOCAL_INTR_STAT	0x0D94
+#define REG_RC2EP_SW_TRIG_LOCAL_INTR_SET	0x0D98
+#define REG_RC2EP_SW_TRIG_LOCAL_INTR_CLR	0x0D9C
+#define REG_DIS_ASPM_LOWPWR_SET_0		0x0E50
+#define REG_DIS_ASPM_LOWPWR_CLR_0		0x0E54
+#define REG_DIS_ASPM_LOWPWR_SET_1		0x0E58
+#define REG_DIS_ASPM_LOWPWR_CLR_1		0x0E5C
+#define REG_DIS_ASPM_LOWPWR_STS_0		0x0E60
+#define REG_DIS_ASPM_LOWPWR_STS_1		0x0E64
+#define REG_PCIE_PEXTP_MAC_SLEEP_CTRL		0x0E70
+#define REG_MSIX_SW_TRIG_SET_GRP0_0		0x0E80
+#define REG_MSIX_ISTATUS_HOST_GRP0_0		0x0F00
+#define REG_IMASK_HOST_MSIX_SET_GRP0_0		0x3000
+#define REG_IMASK_HOST_MSIX_CLR_GRP0_0		0x3080
+#define REG_IMASK_HOST_MSIX_GRP0_0		0x3100
+#define REG_DEV_INFRA_BASE			0x10001000
+#endif /* __MTK_REG_H__ */
-- 
2.32.0
Re: [PATCH net-next v4 01/10] net: wwan: tmi: Add PCIe core
Posted by Loic Poulain 1 year, 8 months ago
Hi Yanchao,

On Fri, 17 Mar 2023 at 09:10, Yanchao Yang <yanchao.yang@mediatek.com> wrote:
>
> Registers the TMI device driver with the kernel. Set up all the fundamental
> configurations for the device: PCIe layer, Modem Host Cross Core Interface
> (MHCCIF), Reset Generation Unit (RGU), modem common control operations and
> build infrastructure.
>
> * PCIe layer code implements driver probe and removal, MSI-X interrupt
> initialization and de-initialization, and the way of resetting the device.
> * MHCCIF provides interrupt channels to communicate events such as handshake,
> PM and port enumeration.
> * RGU provides interrupt channels to generate notifications from the device
> so that the TMI driver could get the device reset.
> * Modem common control operations provide the basic read/write functions of
> the device's hardware registers, mask/unmask/get/clear functions of the
> device's interrupt registers and inquiry functions of the device's status.
>
> Signed-off-by: Yanchao Yang <yanchao.yang@mediatek.com>
> Signed-off-by: Ting Wang <ting.wang@mediatek.com>
> ---
>  drivers/net/wwan/Kconfig                 |  14 +
>  drivers/net/wwan/Makefile                |   1 +
>  drivers/net/wwan/mediatek/Makefile       |   8 +
>  drivers/net/wwan/mediatek/mtk_dev.h      | 203 ++++++
>  drivers/net/wwan/mediatek/pcie/mtk_pci.c | 887 +++++++++++++++++++++++
>  drivers/net/wwan/mediatek/pcie/mtk_pci.h | 144 ++++
>  drivers/net/wwan/mediatek/pcie/mtk_reg.h |  69 ++
>  7 files changed, 1326 insertions(+)
>  create mode 100644 drivers/net/wwan/mediatek/Makefile
>  create mode 100644 drivers/net/wwan/mediatek/mtk_dev.h
>  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.c
>  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.h
>  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_reg.h
>

[...]

> +static int mtk_pci_get_virq_id(struct mtk_md_dev *mdev, int irq_id)
> +{
> +       struct pci_dev *pdev = to_pci_dev(mdev->dev);
> +       int nr = 0;
> +
> +       if (pdev->msix_enabled)
> +               nr = irq_id % mdev->msi_nvecs;
> +
> +       return pci_irq_vector(pdev, nr);
> +}
> +
> +static int mtk_pci_register_irq(struct mtk_md_dev *mdev, int irq_id,
> +                               int (*irq_cb)(int irq_id, void *data), void *data)
> +{
> +       struct mtk_pci_priv *priv = mdev->hw_priv;
> +
> +       if (unlikely((irq_id < 0 || irq_id >= MTK_IRQ_CNT_MAX) || !irq_cb))
> +               return -EINVAL;
> +
> +       if (priv->irq_cb_list[irq_id]) {
> +               dev_err(mdev->dev,
> +                       "Unable to register irq, irq_id=%d, it's already been register by %ps.\n",
> +                       irq_id, priv->irq_cb_list[irq_id]);
> +               return -EFAULT;
> +       }
> +       priv->irq_cb_list[irq_id] = irq_cb;
> +       priv->irq_cb_data[irq_id] = data;

So it looks like you re-implement your own irq chip internally. What
about creating a new irq-chip/domain for this (cf irq_domain_add_simple)?
That would allow the client code to use the regular irq interface and helpers
and it should simply code and improve its debuggability (/proc/irq...).

[...]

> +static int mtk_mhccif_register_evt(struct mtk_md_dev *mdev, u32 chs,
> +                                  int (*evt_cb)(u32 status, void *data), void *data)
> +{
> +       struct mtk_pci_priv *priv = mdev->hw_priv;
> +       struct mtk_mhccif_cb *cb;
> +       unsigned long flag;
> +       int ret = 0;
> +
> +       if (!chs || !evt_cb)
> +               return -EINVAL;
> +
> +       spin_lock_irqsave(&priv->mhccif_lock, flag);

Why spinlock here and not mutex. AFAIU, you always take this lock in a
non-atomic/process context.

> +       list_for_each_entry(cb, &priv->mhccif_cb_list, entry) {
> +               if (cb->chs & chs) {
> +                       ret = -EFAULT;
> +                       dev_err(mdev->dev,
> +                               "Unable to register evt, chs=0x%08X&0x%08X registered_cb=%ps\n",
> +                               chs, cb->chs, cb->evt_cb);
> +                       goto err;
> +               }
> +       }
> +       cb = devm_kzalloc(mdev->dev, sizeof(*cb), GFP_ATOMIC);
> +       if (!cb) {
> +               ret = -ENOMEM;
> +               goto err;
> +       }
> +       cb->evt_cb = evt_cb;
> +       cb->data = data;
> +       cb->chs = chs;
> +       list_add_tail(&cb->entry, &priv->mhccif_cb_list);
> +
> +err:
> +       spin_unlock_irqrestore(&priv->mhccif_lock, flag);
> +
> +       return ret;
> +}

[...]

> +
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/wwan/mediatek/pcie/mtk_pci.h b/drivers/net/wwan/mediatek/pcie/mtk_pci.h
> new file mode 100644
> index 000000000000..b487ca9b302e
> --- /dev/null
> +++ b/drivers/net/wwan/mediatek/pcie/mtk_pci.h

Why a separated header file, isn't the content (e.g. mtk_pci_priv)
used only from mtk_pci.c?

> @@ -0,0 +1,144 @@
> +/* SPDX-License-Identifier: BSD-3-Clause-Clear
> + *
> + * Copyright (c) 2022, MediaTek Inc.
> + */
> +
> +#ifndef __MTK_PCI_H__
> +#define __MTK_PCI_H__
> +
> +#include <linux/pci.h>
> +#include <linux/spinlock.h>
> +
> +#include "../mtk_dev.h"
> +
> +enum mtk_atr_type {
> +       ATR_PCI2AXI = 0,
> +       ATR_AXI2PCI
> +};

[...]

Regards,
Loic
Re: [PATCH net-next v4 01/10] net: wwan: tmi: Add PCIe core
Posted by Yanchao Yang (杨彦超) 1 year, 7 months ago
Hi Loic,

On Fri, 2023-03-17 at 13:34 +0100, Loic Poulain wrote:
> Hi Yanchao,
> 
> On Fri, 17 Mar 2023 at 09:10, Yanchao Yang <yanchao.yang@mediatek.com
> > wrote:
> > 
> > Registers the TMI device driver with the kernel. Set up all the
> > fundamental
> > configurations for the device: PCIe layer, Modem Host Cross Core
> > Interface
> > (MHCCIF), Reset Generation Unit (RGU), modem common control
> > operations and
> > build infrastructure.
> > 
> > * PCIe layer code implements driver probe and removal, MSI-X
> > interrupt
> > initialization and de-initialization, and the way of resetting the
> > device.
> > * MHCCIF provides interrupt channels to communicate events such as
> > handshake,
> > PM and port enumeration.
> > * RGU provides interrupt channels to generate notifications from
> > the device
> > so that the TMI driver could get the device reset.
> > * Modem common control operations provide the basic read/write
> > functions of
> > the device's hardware registers, mask/unmask/get/clear functions of
> > the
> > device's interrupt registers and inquiry functions of the device's
> > status.
> > 
> > Signed-off-by: Yanchao Yang <yanchao.yang@mediatek.com>
> > Signed-off-by: Ting Wang <ting.wang@mediatek.com>
> > ---
> >  drivers/net/wwan/Kconfig                 |  14 +
> >  drivers/net/wwan/Makefile                |   1 +
> >  drivers/net/wwan/mediatek/Makefile       |   8 +
> >  drivers/net/wwan/mediatek/mtk_dev.h      | 203 ++++++
> >  drivers/net/wwan/mediatek/pcie/mtk_pci.c | 887
> > +++++++++++++++++++++++
> >  drivers/net/wwan/mediatek/pcie/mtk_pci.h | 144 ++++
> >  drivers/net/wwan/mediatek/pcie/mtk_reg.h |  69 ++
> >  7 files changed, 1326 insertions(+)
> >  create mode 100644 drivers/net/wwan/mediatek/Makefile
> >  create mode 100644 drivers/net/wwan/mediatek/mtk_dev.h
> >  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.c
> >  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.h
> >  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_reg.h
> > 
> 
> [...]
> 
> > +static int mtk_pci_get_virq_id(struct mtk_md_dev *mdev, int
> > irq_id)
> > +{
> > +       struct pci_dev *pdev = to_pci_dev(mdev->dev);
> > +       int nr = 0;
> > +
> > +       if (pdev->msix_enabled)
> > +               nr = irq_id % mdev->msi_nvecs;
> > +
> > +       return pci_irq_vector(pdev, nr);
> > +}
> > +
> > +static int mtk_pci_register_irq(struct mtk_md_dev *mdev, int
> > irq_id,
> > +                               int (*irq_cb)(int irq_id, void
> > *data), void *data)
> > +{
> > +       struct mtk_pci_priv *priv = mdev->hw_priv;
> > +
> > +       if (unlikely((irq_id < 0 || irq_id >= MTK_IRQ_CNT_MAX) ||
> > !irq_cb))
> > +               return -EINVAL;
> > +
> > +       if (priv->irq_cb_list[irq_id]) {
> > +               dev_err(mdev->dev,
> > +                       "Unable to register irq, irq_id=%d, it's
> > already been register by %ps.\n",
> > +                       irq_id, priv->irq_cb_list[irq_id]);
> > +               return -EFAULT;
> > +       }
> > +       priv->irq_cb_list[irq_id] = irq_cb;
> > +       priv->irq_cb_data[irq_id] = data;
> 
> So it looks like you re-implement your own irq chip internally. What
> about creating a new irq-chip/domain for this (cf
> irq_domain_add_simple)?
> That would allow the client code to use the regular irq interface and
> helpers
> and it should simply code and improve its debuggability
> (/proc/irq...).
We will check it and update you later.
> 
> [...]
> 
> > +static int mtk_mhccif_register_evt(struct mtk_md_dev *mdev, u32
> > chs,
> > +                                  int (*evt_cb)(u32 status, void
> > *data), void *data)
> > +{
> > +       struct mtk_pci_priv *priv = mdev->hw_priv;
> > +       struct mtk_mhccif_cb *cb;
> > +       unsigned long flag;
> > +       int ret = 0;
> > +
> > +       if (!chs || !evt_cb)
> > +               return -EINVAL;
> > +
> > +       spin_lock_irqsave(&priv->mhccif_lock, flag);
> 
> Why spinlock here and not mutex. AFAIU, you always take this lock in
> a
> non-atomic/process context.
Currently, the function is only called in the FSM initialization and
PM(power management) initialization process. Both are atomic.
On the other hand, this registration function will operate the global
variables “mhccif_cb_list”, but it takes very little time. So, we think
spinlock is preferred.

> 
> > +       list_for_each_entry(cb, &priv->mhccif_cb_list, entry) {
> > +               if (cb->chs & chs) {
> > +                       ret = -EFAULT;
> > +                       dev_err(mdev->dev,
> > +                               "Unable to register evt,
> > chs=0x%08X&0x%08X registered_cb=%ps\n",
> > +                               chs, cb->chs, cb->evt_cb);
> > +                       goto err;
> > +               }
> > +       }
> > +       cb = devm_kzalloc(mdev->dev, sizeof(*cb), GFP_ATOMIC);
> > +       if (!cb) {
> > +               ret = -ENOMEM;
> > +               goto err;
> > +       }
> > +       cb->evt_cb = evt_cb;
> > +       cb->data = data;
> > +       cb->chs = chs;
> > +       list_add_tail(&cb->entry, &priv->mhccif_cb_list);
> > +
> > +err:
> > +       spin_unlock_irqrestore(&priv->mhccif_lock, flag);
> > +
> > +       return ret;
> > +}
> 
> [...]
> 
> > +
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > b/drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > new file mode 100644
> > index 000000000000..b487ca9b302e
> > --- /dev/null
> > +++ b/drivers/net/wwan/mediatek/pcie/mtk_pci.h
> 
> Why a separated header file, isn't the content (e.g. mtk_pci_priv)
> used only from mtk_pci.c?
Do you mean that we should move all contents of “mtk_pci.h” into
“mtk_pci.c” directly? The “mtk_pci.h” seems to be redundant, right?

> 
> > @@ -0,0 +1,144 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause-Clear
> > + *
> > + * Copyright (c) 2022, MediaTek Inc.
> > + */
> > +
> > +#ifndef __MTK_PCI_H__
> > +#define __MTK_PCI_H__
> > +
> > +#include <linux/pci.h>
> > +#include <linux/spinlock.h>
> > +
> > +#include "../mtk_dev.h"
> > +
> > +enum mtk_atr_type {
> > +       ATR_PCI2AXI = 0,
> > +       ATR_AXI2PCI
> > +};
> 
> [...]
> 
> Regards,
> Loic

Many thanks.
Yanchao.Yang
Re: [PATCH net-next v4 01/10] net: wwan: tmi: Add PCIe core
Posted by Yanchao Yang (杨彦超) 1 year, 7 months ago
Hi Loic,

sorry for late response, please check following reply.

On Mon, 2023-03-20 at 21:26 +0800, Yanchao Yang wrote:
> Hi Loic,
> 
> On Fri, 2023-03-17 at 13:34 +0100, Loic Poulain wrote:
> > Hi Yanchao,
> > 
> > On Fri, 17 Mar 2023 at 09:10, Yanchao Yang <
> > yanchao.yang@mediatek.com
> > > wrote:
> > > 
> > > Registers the TMI device driver with the kernel. Set up all the
> > > fundamental
> > > configurations for the device: PCIe layer, Modem Host Cross Core
> > > Interface
> > > (MHCCIF), Reset Generation Unit (RGU), modem common control
> > > operations and
> > > build infrastructure.
> > > 
> > > * PCIe layer code implements driver probe and removal, MSI-X
> > > interrupt
> > > initialization and de-initialization, and the way of resetting
> > > the
> > > device.
> > > * MHCCIF provides interrupt channels to communicate events such
> > > as
> > > handshake,
> > > PM and port enumeration.
> > > * RGU provides interrupt channels to generate notifications from
> > > the device
> > > so that the TMI driver could get the device reset.
> > > * Modem common control operations provide the basic read/write
> > > functions of
> > > the device's hardware registers, mask/unmask/get/clear functions
> > > of
> > > the
> > > device's interrupt registers and inquiry functions of the
> > > device's
> > > status.
> > > 
> > > Signed-off-by: Yanchao Yang <yanchao.yang@mediatek.com>
> > > Signed-off-by: Ting Wang <ting.wang@mediatek.com>
> > > ---
> > >  drivers/net/wwan/Kconfig                 |  14 +
> > >  drivers/net/wwan/Makefile                |   1 +
> > >  drivers/net/wwan/mediatek/Makefile       |   8 +
> > >  drivers/net/wwan/mediatek/mtk_dev.h      | 203 ++++++
> > >  drivers/net/wwan/mediatek/pcie/mtk_pci.c | 887
> > > +++++++++++++++++++++++
> > >  drivers/net/wwan/mediatek/pcie/mtk_pci.h | 144 ++++
> > >  drivers/net/wwan/mediatek/pcie/mtk_reg.h |  69 ++
> > >  7 files changed, 1326 insertions(+)
> > >  create mode 100644 drivers/net/wwan/mediatek/Makefile
> > >  create mode 100644 drivers/net/wwan/mediatek/mtk_dev.h
> > >  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.c
> > >  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > >  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_reg.h
> > > 
> > 
> > [...]
> > 
> > > +static int mtk_pci_get_virq_id(struct mtk_md_dev *mdev, int
> > > irq_id)
> > > +{
> > > +       struct pci_dev *pdev = to_pci_dev(mdev->dev);
> > > +       int nr = 0;
> > > +
> > > +       if (pdev->msix_enabled)
> > > +               nr = irq_id % mdev->msi_nvecs;
> > > +
> > > +       return pci_irq_vector(pdev, nr);
> > > +}
> > > +
> > > +static int mtk_pci_register_irq(struct mtk_md_dev *mdev, int
> > > irq_id,
> > > +                               int (*irq_cb)(int irq_id, void
> > > *data), void *data)
> > > +{
> > > +       struct mtk_pci_priv *priv = mdev->hw_priv;
> > > +
> > > +       if (unlikely((irq_id < 0 || irq_id >= MTK_IRQ_CNT_MAX) ||
> > > !irq_cb))
> > > +               return -EINVAL;
> > > +
> > > +       if (priv->irq_cb_list[irq_id]) {
> > > +               dev_err(mdev->dev,
> > > +                       "Unable to register irq, irq_id=%d, it's
> > > already been register by %ps.\n",
> > > +                       irq_id, priv->irq_cb_list[irq_id]);
> > > +               return -EFAULT;
> > > +       }
> > > +       priv->irq_cb_list[irq_id] = irq_cb;
> > > +       priv->irq_cb_data[irq_id] = data;
> > 
> > So it looks like you re-implement your own irq chip internally.
> > What
> > about creating a new irq-chip/domain for this (cf
> > irq_domain_add_simple)?
> > That would allow the client code to use the regular irq interface
> > and
> > helpers
> > and it should simply code and improve its debuggability
> > (/proc/irq...).
> 
> We will check it and update you later.
No, we don’t re-implement irq chip. After studying the irq_domain
interface you suggest, the TMI driver leverages on MSI irq domain. We
use pci_alloc_irq_vectors to allocate MSI-X irq desc and use
pci_request_irq to bind interrupt sources with irq handlers.
> > 
> > [...]
> > 
> > > +static int mtk_mhccif_register_evt(struct mtk_md_dev *mdev, u32
> > > chs,
> > > +                                  int (*evt_cb)(u32 status, void
> > > *data), void *data)
> > > +{
> > > +       struct mtk_pci_priv *priv = mdev->hw_priv;
> > > +       struct mtk_mhccif_cb *cb;
> > > +       unsigned long flag;
> > > +       int ret = 0;
> > > +
> > > +       if (!chs || !evt_cb)
> > > +               return -EINVAL;
> > > +
> > > +       spin_lock_irqsave(&priv->mhccif_lock, flag);
> > 
> > Why spinlock here and not mutex. AFAIU, you always take this lock
> > in
> > a
> > non-atomic/process context.
> 
> Currently, the function is only called in the FSM initialization and
> PM(power management) initialization process. Both are atomic.
> On the other hand, this registration function will operate the global
> variables “mhccif_cb_list”, but it takes very little time. So, we
> think
> spinlock is preferred.
Any ideas or comments for this? Please help share it at your
convenience.
> 
> > 
> > > +       list_for_each_entry(cb, &priv->mhccif_cb_list, entry) {
> > > +               if (cb->chs & chs) {
> > > +                       ret = -EFAULT;
> > > +                       dev_err(mdev->dev,
> > > +                               "Unable to register evt,
> > > chs=0x%08X&0x%08X registered_cb=%ps\n",
> > > +                               chs, cb->chs, cb->evt_cb);
> > > +                       goto err;
> > > +               }
> > > +       }
> > > +       cb = devm_kzalloc(mdev->dev, sizeof(*cb), GFP_ATOMIC);
> > > +       if (!cb) {
> > > +               ret = -ENOMEM;
> > > +               goto err;
> > > +       }
> > > +       cb->evt_cb = evt_cb;
> > > +       cb->data = data;
> > > +       cb->chs = chs;
> > > +       list_add_tail(&cb->entry, &priv->mhccif_cb_list);
> > > +
> > > +err:
> > > +       spin_unlock_irqrestore(&priv->mhccif_lock, flag);
> > > +
> > > +       return ret;
> > > +}
> > 
> > [...]
> > 
> > > +
> > > +MODULE_LICENSE("GPL");
> > > diff --git a/drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > > b/drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > > new file mode 100644
> > > index 000000000000..b487ca9b302e
> > > --- /dev/null
> > > +++ b/drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > 
> > Why a separated header file, isn't the content (e.g. mtk_pci_priv)
> > used only from mtk_pci.c?
> 
> Do you mean that we should move all contents of “mtk_pci.h” into
> “mtk_pci.c” directly? The “mtk_pci.h” seems to be redundant, right?
Any ideas or comments for this? Please help share it at your
convenience.
> 
> > 
> > > @@ -0,0 +1,144 @@
> > > +/* SPDX-License-Identifier: BSD-3-Clause-Clear
> > > + *
> > > + * Copyright (c) 2022, MediaTek Inc.
> > > + */
> > > +
> > > +#ifndef __MTK_PCI_H__
> > > +#define __MTK_PCI_H__
> > > +
> > > +#include <linux/pci.h>
> > > +#include <linux/spinlock.h>
> > > +
> > > +#include "../mtk_dev.h"
> > > +
> > > +enum mtk_atr_type {
> > > +       ATR_PCI2AXI = 0,
> > > +       ATR_AXI2PCI
> > > +};
> > 
> > [...]
> > 
> > Regards,
> > Loic
> 
> Many thanks.
> Yanchao.Yang

BTW, can you help check other patches and share your suggestions?

Many thanks.
Yanchao.Yang
Re: [PATCH net-next v4 01/10] net: wwan: tmi: Add PCIe core
Posted by Loic Poulain 1 year, 6 months ago
Hello,


On Mon, 10 Apr 2023 at 14:16, Yanchao Yang (杨彦超)
<Yanchao.Yang@mediatek.com> wrote:
>
> Hi Loic,
>
> sorry for late response, please check following reply.
>
> On Mon, 2023-03-20 at 21:26 +0800, Yanchao Yang wrote:
> > Hi Loic,
> >
> > On Fri, 2023-03-17 at 13:34 +0100, Loic Poulain wrote:
> > > Hi Yanchao,
> > >
> > > On Fri, 17 Mar 2023 at 09:10, Yanchao Yang <
> > > yanchao.yang@mediatek.com
> > > > wrote:
> > > >
> > > > Registers the TMI device driver with the kernel. Set up all the
> > > > fundamental
> > > > configurations for the device: PCIe layer, Modem Host Cross Core
> > > > Interface
> > > > (MHCCIF), Reset Generation Unit (RGU), modem common control
> > > > operations and
> > > > build infrastructure.
> > > >
> > > > * PCIe layer code implements driver probe and removal, MSI-X
> > > > interrupt
> > > > initialization and de-initialization, and the way of resetting
> > > > the
> > > > device.
> > > > * MHCCIF provides interrupt channels to communicate events such
> > > > as
> > > > handshake,
> > > > PM and port enumeration.
> > > > * RGU provides interrupt channels to generate notifications from
> > > > the device
> > > > so that the TMI driver could get the device reset.
> > > > * Modem common control operations provide the basic read/write
> > > > functions of
> > > > the device's hardware registers, mask/unmask/get/clear functions
> > > > of
> > > > the
> > > > device's interrupt registers and inquiry functions of the
> > > > device's
> > > > status.
> > > >
> > > > Signed-off-by: Yanchao Yang <yanchao.yang@mediatek.com>
> > > > Signed-off-by: Ting Wang <ting.wang@mediatek.com>
> > > > ---
> > > >  drivers/net/wwan/Kconfig                 |  14 +
> > > >  drivers/net/wwan/Makefile                |   1 +
> > > >  drivers/net/wwan/mediatek/Makefile       |   8 +
> > > >  drivers/net/wwan/mediatek/mtk_dev.h      | 203 ++++++
> > > >  drivers/net/wwan/mediatek/pcie/mtk_pci.c | 887
> > > > +++++++++++++++++++++++
> > > >  drivers/net/wwan/mediatek/pcie/mtk_pci.h | 144 ++++
> > > >  drivers/net/wwan/mediatek/pcie/mtk_reg.h |  69 ++
> > > >  7 files changed, 1326 insertions(+)
> > > >  create mode 100644 drivers/net/wwan/mediatek/Makefile
> > > >  create mode 100644 drivers/net/wwan/mediatek/mtk_dev.h
> > > >  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.c
> > > >  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > > >  create mode 100644 drivers/net/wwan/mediatek/pcie/mtk_reg.h
> > > >
> > >
> > > [...]
> > >
> > > > +static int mtk_pci_get_virq_id(struct mtk_md_dev *mdev, int
> > > > irq_id)
> > > > +{
> > > > +       struct pci_dev *pdev = to_pci_dev(mdev->dev);
> > > > +       int nr = 0;
> > > > +
> > > > +       if (pdev->msix_enabled)
> > > > +               nr = irq_id % mdev->msi_nvecs;
> > > > +
> > > > +       return pci_irq_vector(pdev, nr);
> > > > +}
> > > > +
> > > > +static int mtk_pci_register_irq(struct mtk_md_dev *mdev, int
> > > > irq_id,
> > > > +                               int (*irq_cb)(int irq_id, void
> > > > *data), void *data)
> > > > +{
> > > > +       struct mtk_pci_priv *priv = mdev->hw_priv;
> > > > +
> > > > +       if (unlikely((irq_id < 0 || irq_id >= MTK_IRQ_CNT_MAX) ||
> > > > !irq_cb))
> > > > +               return -EINVAL;
> > > > +
> > > > +       if (priv->irq_cb_list[irq_id]) {
> > > > +               dev_err(mdev->dev,
> > > > +                       "Unable to register irq, irq_id=%d, it's
> > > > already been register by %ps.\n",
> > > > +                       irq_id, priv->irq_cb_list[irq_id]);
> > > > +               return -EFAULT;
> > > > +       }
> > > > +       priv->irq_cb_list[irq_id] = irq_cb;
> > > > +       priv->irq_cb_data[irq_id] = data;
> > >
> > > So it looks like you re-implement your own irq chip internally.
> > > What
> > > about creating a new irq-chip/domain for this (cf
> > > irq_domain_add_simple)?
> > > That would allow the client code to use the regular irq interface
> > > and
> > > helpers
> > > and it should simply code and improve its debuggability
> > > (/proc/irq...).
> >
> > We will check it and update you later.
> No, we don’t re-implement irq chip. After studying the irq_domain
> interface you suggest, the TMI driver leverages on MSI irq domain. We
> use pci_alloc_irq_vectors to allocate MSI-X irq desc and use
> pci_request_irq to bind interrupt sources with irq handlers.

What I mean is that you're implementing some interrupt muxing into
your driver, which could possibly be abstracted with a 'virtual' irq
domain and generic ops (e.g with proper irq domain your
mtk_pci_register_irq method could be replaced with the usual
request_irq), Think about it as future improvement, I assume we can go
with the current solution for now.

> > >
> > > [...]
> > >
> > > > +static int mtk_mhccif_register_evt(struct mtk_md_dev *mdev, u32
> > > > chs,
> > > > +                                  int (*evt_cb)(u32 status, void
> > > > *data), void *data)
> > > > +{
> > > > +       struct mtk_pci_priv *priv = mdev->hw_priv;
> > > > +       struct mtk_mhccif_cb *cb;
> > > > +       unsigned long flag;
> > > > +       int ret = 0;
> > > > +
> > > > +       if (!chs || !evt_cb)
> > > > +               return -EINVAL;
> > > > +
> > > > +       spin_lock_irqsave(&priv->mhccif_lock, flag);
> > >
> > > Why spinlock here and not mutex. AFAIU, you always take this lock
> > > in
> > > a
> > > non-atomic/process context.
> >
> > Currently, the function is only called in the FSM initialization and
> > PM(power management) initialization process. Both are atomic.
> > On the other hand, this registration function will operate the global
> > variables “mhccif_cb_list”, but it takes very little time. So, we
> > think
> > spinlock is preferred.
> Any ideas or comments for this? Please help share it at your
> convenience.

They do not seem atomic... moreover you're using the _irqsave variant
while you're never accessing the list from (hard|soft)irq context.

> >
> > >
> > > > +       list_for_each_entry(cb, &priv->mhccif_cb_list, entry) {
> > > > +               if (cb->chs & chs) {
> > > > +                       ret = -EFAULT;
> > > > +                       dev_err(mdev->dev,
> > > > +                               "Unable to register evt,
> > > > chs=0x%08X&0x%08X registered_cb=%ps\n",
> > > > +                               chs, cb->chs, cb->evt_cb);
> > > > +                       goto err;
> > > > +               }
> > > > +       }
> > > > +       cb = devm_kzalloc(mdev->dev, sizeof(*cb), GFP_ATOMIC);

Maybe alloc this outside the lock, so that you can use the 'regular'
GFP_KERNEL alloc.

> > > > +       if (!cb) {
> > > > +               ret = -ENOMEM;
> > > > +               goto err;
> > > > +       }
> > > > +       cb->evt_cb = evt_cb;
> > > > +       cb->data = data;
> > > > +       cb->chs = chs;
> > > > +       list_add_tail(&cb->entry, &priv->mhccif_cb_list);
> > > > +
> > > > +err:
> > > > +       spin_unlock_irqrestore(&priv->mhccif_lock, flag);
> > > > +
> > > > +       return ret;
> > > > +}
> > >
> > > [...]
> > >
> > > > +
> > > > +MODULE_LICENSE("GPL");
> > > > diff --git a/drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > > > b/drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > > > new file mode 100644
> > > > index 000000000000..b487ca9b302e
> > > > --- /dev/null
> > > > +++ b/drivers/net/wwan/mediatek/pcie/mtk_pci.h
> > >
> > > Why a separated header file, isn't the content (e.g. mtk_pci_priv)
> > > used only from mtk_pci.c?
> >
> > Do you mean that we should move all contents of “mtk_pci.h” into
> > “mtk_pci.c” directly? The “mtk_pci.h” seems to be redundant, right?
> Any ideas or comments for this? Please help share it at your
> convenience.

Yes, keep that in the  .c file.

Regards,
Loic