RE: [PATCH 01/11] net: wwan: t9xx: Add PCIe core

From: Jagielski, Jedrzej

Date: Mon Jun 01 2026 - 07:24:43 EST

From: Jack Wu via B4 Relay <devnull+jackbb_wu.compal.com@xxxxxxxxxx>
Sent: Friday, May 29, 2026 12:32 PM

>From: Jack Wu <jackbb_wu@xxxxxxxxxx>
>
>Registers the T900 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 T900 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: Jack Wu <jackbb_wu@xxxxxxxxxx>
>---
> drivers/net/wwan/Kconfig | 12 +
> drivers/net/wwan/Makefile | 1 +
> drivers/net/wwan/t9xx/Makefile | 10 +
> drivers/net/wwan/t9xx/mtk_dev.h | 108 +++
> drivers/net/wwan/t9xx/pcie/mtk_pci.c | 926 ++++++++++++++++++++++++++
> drivers/net/wwan/t9xx/pcie/mtk_pci.h | 219 ++++++
> drivers/net/wwan/t9xx/pcie/mtk_pci_drv_m9xx.c | 69 ++
> drivers/net/wwan/t9xx/pcie/mtk_pci_reg.h | 71 ++
> 8 files changed, 1416 insertions(+)
>
>diff --git a/drivers/net/wwan/Kconfig b/drivers/net/wwan/Kconfig
>index 88df55d78d90..4cee537c739f 100644
>--- a/drivers/net/wwan/Kconfig
>+++ b/drivers/net/wwan/Kconfig
>@@ -121,6 +121,18 @@ config MTK_T7XX
>
> If unsure, say N.
>
>+config MTK_T9XX
>+ tristate "MediaTek PCIe 5G WWAN modem T9xx device"
>+ depends on PCI
>+ select NET_DEVLINK
>+ help
>+ Enables MediaTek PCIe based 5G WWAN modem (T9xx series) device.
>+
>+ To compile this driver as a module, choose M here: the module will be
>+ called mtk_t9xx.
>+
>+ If unsure, say N.
>+
> endif # WWAN
>
> endmenu
>diff --git a/drivers/net/wwan/Makefile b/drivers/net/wwan/Makefile
>index 3960c0ae2445..7361eef4c472 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_T9XX) += t9xx/
>diff --git a/drivers/net/wwan/t9xx/Makefile b/drivers/net/wwan/t9xx/Makefile
>new file mode 100644
>index 000000000000..6f2dd3f91454
>--- /dev/null
>+++ b/drivers/net/wwan/t9xx/Makefile
>@@ -0,0 +1,10 @@
>+# SPDX-License-Identifier: GPL-2.0-only
>+
>+ccflags-y += -I$(src)/pcie
>+ccflags-y += -I$(src)
>+
>+obj-$(CONFIG_MTK_T9XX) += mtk_t9xx.o
>+
>+mtk_t9xx-y := \
>+ pcie/mtk_pci.o \
>+ pcie/mtk_pci_drv_m9xx.o
>diff --git a/drivers/net/wwan/t9xx/mtk_dev.h b/drivers/net/wwan/t9xx/mtk_dev.h
>new file mode 100644
>index 000000000000..8278a0e2875e
>--- /dev/null
>+++ b/drivers/net/wwan/t9xx/mtk_dev.h
>@@ -0,0 +1,108 @@
>+/* SPDX-License-Identifier: GPL-2.0-only
>+ *
>+ * Copyright (c) 2022, MediaTek Inc.
>+ */
>+
>+#ifndef __MTK_DEV_H__
>+#define __MTK_DEV_H__
>+
>+#include <linux/dma-mapping.h>
>+#include <linux/dmapool.h>
>+#include <linux/kernel.h>
>+#include <linux/module.h>
>+#include <linux/sched.h>
>+#include <linux/slab.h>
>+#include <linux/spinlock.h>
>+
>+#define MTK_DEV_STR_LEN 16
>+
>+enum mtk_user_id {
>+ MTK_USER_MIN,
>+ MTK_USER_CTRL,
>+ MTK_USER_DATA,
>+ MTK_USER_MAX
>+};
>+
>+enum mtk_dev_evt_h2d {
>+ DEV_EVT_H2D_DEVICE_RESET = BIT(2),
>+ DEV_EVT_H2D_MAX = BIT(5)
>+};
>+
>+enum mtk_dev_evt_d2h {
>+ DEV_EVT_D2H_BOOT_FLOW_SYNC = BIT(4),
>+ DEV_EVT_D2H_ASYNC_HS_NOTIFY_SAP = BIT(5),
>+ DEV_EVT_D2H_ASYNC_HS_NOTIFY_MD = BIT(6),
>+ DEV_EVT_D2H_MAX = BIT(11)
>+};
>+
>+struct mtk_md_dev;
>+
>+struct mtk_dev_ops {
>+ u32 (*get_dev_state)(struct mtk_md_dev *mdev);
>+ void (*ack_dev_state)(struct mtk_md_dev *mdev, u32 state);
>+ u32 (*get_dev_cfg)(struct mtk_md_dev *mdev);
>+ int (*register_dev_evt)(struct mtk_md_dev *mdev, u32 dev_evt,
>+ int (*evt_cb)(u32 status, void *data), void *data);
>+ void (*unregister_dev_evt)(struct mtk_md_dev *mdev, u32 dev_evt);
>+ void (*mask_dev_evt)(struct mtk_md_dev *mdev, u32 dev_evt);
>+ void (*unmask_dev_evt)(struct mtk_md_dev *mdev, u32 dev_evt);
>+ void (*clear_dev_evt)(struct mtk_md_dev *mdev, u32 dev_evt);
>+ int (*send_dev_evt)(struct mtk_md_dev *mdev, u32 dev_evt);
>+};
>+
>+/* mtk_md_dev defines the structure of MTK modem device */
>+struct mtk_md_dev {
>+ struct device *dev;
>+ const struct mtk_dev_ops *dev_ops;
>+ void *hw_priv;
>+ u32 hw_ver;
>+ char dev_str[MTK_DEV_STR_LEN];
>+};
>+
>+static inline u32 mtk_dev_get_dev_state(struct mtk_md_dev *mdev)
>+{
>+ return mdev->dev_ops->get_dev_state(mdev);
>+}
>+
>+static inline void mtk_dev_ack_dev_state(struct mtk_md_dev *mdev, u32 state)
>+{
>+ return mdev->dev_ops->ack_dev_state(mdev, state);
>+}
>+
>+static inline u32 mtk_dev_get_dev_cfg(struct mtk_md_dev *mdev)
>+{
>+ return mdev->dev_ops->get_dev_cfg(mdev);
>+}
>+
>+static inline int mtk_dev_register_dev_evt(struct mtk_md_dev *mdev, u32 dev_evt,
>+ int (*evt_cb)(u32 status, void *data), void *data)
>+{
>+ return mdev->dev_ops->register_dev_evt(mdev, dev_evt, evt_cb, data);
>+}
>+
>+static inline void mtk_dev_unregister_dev_evt(struct mtk_md_dev *mdev, u32 dev_evt)
>+{
>+ mdev->dev_ops->unregister_dev_evt(mdev, dev_evt);
>+}
>+
>+static inline void mtk_dev_mask_dev_evt(struct mtk_md_dev *mdev, u32 dev_evt)
>+{
>+ mdev->dev_ops->mask_dev_evt(mdev, dev_evt);
>+}
>+
>+static inline void mtk_dev_unmask_dev_evt(struct mtk_md_dev *mdev, u32 dev_evt)
>+{
>+ mdev->dev_ops->unmask_dev_evt(mdev, dev_evt);
>+}
>+
>+static inline void mtk_dev_clear_dev_evt(struct mtk_md_dev *mdev, u32 dev_evt)
>+{
>+ mdev->dev_ops->clear_dev_evt(mdev, dev_evt);
>+}
>+
>+static inline int mtk_dev_send_dev_evt(struct mtk_md_dev *mdev, u32 dev_evt)
>+{
>+ return mdev->dev_ops->send_dev_evt(mdev, dev_evt);
>+}
>+
>+#endif /* __MTK_DEV_H__ */
>diff --git a/drivers/net/wwan/t9xx/pcie/mtk_pci.c b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
>new file mode 100644
>index 000000000000..adec3ccdee08
>--- /dev/null
>+++ b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
>@@ -0,0 +1,926 @@
>+// SPDX-License-Identifier: GPL-2.0-only
>+/*
>+ * Copyright (c) 2022, MediaTek Inc.
>+ */
>+
>+#include <linux/acpi.h>
>+#include <linux/aer.h>
>+#include <linux/bitfield.h>
>+#include <linux/debugfs.h>
>+#include <linux/delay.h>
>+#include <linux/device.h>
>+#include <linux/dma-mapping.h>
>+#include <linux/kernel.h>
>+#include <linux/module.h>
>+
>+#include "mtk_dev.h"
>+#include "mtk_pci.h"
>+#include "mtk_pci_reg.h"
>+
>+#define BAR_NUM 6

please add driver prefix

>+#define MTK_PCI_TRANSPARENT_ATR_SIZE (0x3F)
>+#define MTK_PCI_MINIMUM_ATR_SIZE (0x1000)
>+#define LE32_TO_U32(x) ((__force u32)(__le32)(x))
>+#define SET_HW_BITS(dest, chs, mhccif, dev) \
>+ ({ \
>+ if ((chs) & (dev))

what if any of these is equal to 0?
just skip do not log anything?

>+ (dest) |= FIELD_PREP(mhccif, 1); \
>+ })
>+
>+extern const struct mtk_pci_dev_cfg mtk_dev_cfg_0900;
>+
>+struct mtk_mhccif_cb {
>+ struct list_head entry;
>+ int (*evt_cb)(u32 status, void *data);
>+ void *data;
>+ u32 chs;
>+};
>+
>+u32 mtk_pci_mac_read32(struct mtk_pci_priv *priv, u64 addr)
>+{
>+ return ioread32(priv->mac_reg_base + addr);
>+}
>+
>+void mtk_pci_mac_write32(struct mtk_pci_priv *priv, u64 addr, u32 val)
>+{
>+ iowrite32(val, priv->mac_reg_base + addr);
>+}
>+
>+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);
>+}
>+
>+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);
>+}
>+

would be lovely to have kdoc of the non-static functions from the series

>+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->size < MTK_PCI_MINIMUM_ATR_SIZE)
>+ cfg->size = MTK_PCI_MINIMUM_ATR_SIZE;
>+
>+ 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) {
>+ atr_size = pos - 2;
>+ } else {
>+ pos = ffs(size_h);
>+ atr_size = pos + 30;

i believe better would be to have some defines instead of magic

>+ }

please put some breaks to have the code logically separated

>+ }
>+
>+ /* 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);

comments seem to be redundant imo; clearer would be to have just newline
instead

>+
>+ /* 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);

again a lot of magic here

>+
>+ return 0;
>+}
>+
>+void mtk_pci_atr_disable(struct mtk_pci_priv *priv)
>+{
>+ int port, tbl, offset;
>+ u32 val;
>+
>+ /* 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;
>+ val = mtk_pci_mac_read32(priv, REG_ATR_PCIE_WIN0_T0_SRC_ADDR_LSB + offset);
>+ val = val & (~BIT(0));
>+ /* Disable table by SRC_ADDR_L */
>+ mtk_pci_mac_write32(priv, REG_ATR_PCIE_WIN0_T0_SRC_ADDR_LSB + offset, val);
>+ }
>+}
>+
>+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);
>+}
>+
>+u32 mtk_pci_get_dev_state(struct mtk_md_dev *mdev)
>+{
>+ return mtk_pci_mac_read32(mdev->hw_priv, REG_PCIE_DEBUG_DUMMY_7);
>+}
>+
>+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);
>+}
>+
>+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;
>+}
>+
>+int mtk_pci_get_virq_id(struct mtk_md_dev *mdev, int irq_id)
>+{
>+ struct pci_dev *pdev = to_pci_dev(mdev->dev);
>+ struct mtk_pci_priv *priv = mdev->hw_priv;
>+ int nr = 0;

what's the point of zeroiniting if the value is assigned at
the next line?

>+
>+ nr = irq_id % priv->irq_cnt;

are we sure irq_cnt won't be equal to 0 in any scenario?

>+
>+ return pci_irq_vector(pdev, nr);
>+}
>+
>+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;
>+}
>+
>+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;

is it anyhow beneficial to put unlikely here and in case of other
appearances within the series?

>+
>+ 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;
>+}
>+
>+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)) {

same here

>+ 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;
>+}
>+
>+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;
>+}
>+
>+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 u32 mtk_pci_ext_d2h_evt_hw_bits(u32 chs)
>+{
>+ u32 hw_bits = 0;
>+
>+ SET_HW_BITS(hw_bits, chs, MHCCIF_EP2RC_EVT_BOOT_FLOW_SYNC,
>+ DEV_EVT_D2H_BOOT_FLOW_SYNC);
>+ SET_HW_BITS(hw_bits, chs, MHCCIF_EP2RC_EVT_ASYNC_HS_NOTIFY_SAP,
>+ DEV_EVT_D2H_ASYNC_HS_NOTIFY_SAP);
>+ SET_HW_BITS(hw_bits, chs, MHCCIF_EP2RC_EVT_ASYNC_HS_NOTIFY_MD,
>+ DEV_EVT_D2H_ASYNC_HS_NOTIFY_MD);
>+
>+ return LE32_TO_U32(cpu_to_le32(hw_bits));
>+}
>+
>+static u32 mtk_pci_ext_d2h_evt_chs(u32 hw_bits)
>+{
>+ u32 chs = 0;
>+
>+ if (!hw_bits)
>+ return chs;
>+
>+ chs = FIELD_PREP(DEV_EVT_D2H_BOOT_FLOW_SYNC,
>+ FIELD_GET(MHCCIF_EP2RC_EVT_BOOT_FLOW_SYNC, hw_bits)) |
>+ FIELD_PREP(DEV_EVT_D2H_ASYNC_HS_NOTIFY_SAP,
>+ FIELD_GET(MHCCIF_EP2RC_EVT_ASYNC_HS_NOTIFY_SAP, hw_bits)) |
>+ FIELD_PREP(DEV_EVT_D2H_ASYNC_HS_NOTIFY_MD,
>+ FIELD_GET(MHCCIF_EP2RC_EVT_ASYNC_HS_NOTIFY_MD, hw_bits));
>+
>+ return chs;
>+}
>+
>+int mtk_pci_register_ext_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;
>+ int ret = 0;
>+
>+ if (!chs || !evt_cb)
>+ return -EINVAL;
>+
>+ spin_lock_bh(&priv->mhccif_lock);
>+ list_for_each_entry(cb, &priv->mhccif_cb_list, entry) {
>+ if (cb->chs & chs) {
>+ ret = -EFAULT;
>+ dev_err((mdev)->dev,
>+ "Unable to register evt, intersection: chs=0x%08x&0x%08x registered_cb=%ps\n",
>+ chs, cb->chs, cb->evt_cb);
>+ goto err_spin_unlock;
>+ }
>+ }
>+ cb = devm_kzalloc(mdev->dev, sizeof(*cb), GFP_ATOMIC);
>+ if (!cb) {
>+ ret = -ENOMEM;
>+ goto err_spin_unlock;
>+ }
>+ cb->evt_cb = evt_cb;
>+ cb->data = data;
>+ cb->chs = chs;
>+ list_add_tail(&cb->entry, &priv->mhccif_cb_list);
>+err_spin_unlock:
>+ spin_unlock_bh(&priv->mhccif_lock);
>+
>+ return ret;
>+}
>+
>+void mtk_pci_unregister_ext_evt(struct mtk_md_dev *mdev, u32 chs)
>+{
>+ struct mtk_pci_priv *priv = mdev->hw_priv;
>+ struct mtk_mhccif_cb *cb, *next;
>+
>+ if (!chs)
>+ return;
>+
>+ spin_lock_bh(&priv->mhccif_lock);
>+ 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;
>+ }
>+ }
>+ dev_warn((mdev)->dev,
>+ "Unable to unregister evt, no chs=0x%08x has been registered.\n", chs);
>+out:
>+ spin_unlock_bh(&priv->mhccif_lock);
>+}
>+
>+void mtk_pci_mask_ext_evt(struct mtk_md_dev *mdev, u32 chs)
>+{
>+ struct mtk_pci_priv *priv = mdev->hw_priv;
>+ u32 hw_bits;
>+
>+ hw_bits = mtk_pci_ext_d2h_evt_hw_bits(chs);

one of these is inited at declaration, 2nd one isnt
please stay consistant, @hw_bits can be inited as well

>+
>+ mtk_pci_write32(mdev, priv->cfg->mhccif_rc_base_addr +
>+ MHCCIF_EP2RC_SW_INT_EAP_MASK_SET, hw_bits);
>+}
>+
>+void mtk_pci_unmask_ext_evt(struct mtk_md_dev *mdev, u32 chs)
>+{
>+ struct mtk_pci_priv *priv = mdev->hw_priv;
>+ u32 hw_bits;
>+
>+ hw_bits = mtk_pci_ext_d2h_evt_hw_bits(chs);
>+
>+ mtk_pci_write32(mdev, priv->cfg->mhccif_rc_base_addr +
>+ MHCCIF_EP2RC_SW_INT_EAP_MASK_CLR, hw_bits);
>+}
>+
>+void mtk_pci_clear_ext_evt(struct mtk_md_dev *mdev, u32 chs)
>+{
>+ struct mtk_pci_priv *priv = mdev->hw_priv;
>+ u32 hw_bits;
>+
>+ hw_bits = mtk_pci_ext_d2h_evt_hw_bits(chs);
>+
>+ mtk_pci_write32(mdev, priv->cfg->mhccif_rc_base_addr +
>+ MHCCIF_EP2RC_SW_INT_ACK, hw_bits);
>+}
>+
>+static u32 mtk_pci_ext_h2d_evt_hw_bits(u32 chs)
>+{
>+ u32 hw_bits = 0;
>+
>+ SET_HW_BITS(hw_bits, chs, MHCCIF_RC2EP_EVT_DEVICE_RESET,
>+ DEV_EVT_H2D_DEVICE_RESET);
>+ SET_HW_BITS(hw_bits, chs, MHCCIF_RC2EP_EVT_DRM_DISABLE_AP,
>+ EXT_EVT_H2D_DRM_DISABLE_AP);
>+ return LE32_TO_U32(cpu_to_le32(hw_bits));
>+}
>+

missing kdoc here and there

>+int mtk_pci_send_ext_evt(struct mtk_md_dev *mdev, u32 ch)
>+{
>+ struct mtk_pci_priv *priv = mdev->hw_priv;
>+ u32 rc_base;
>+ u32 hw_bits;

please squash variables of the same type into single line

>+
>+ rc_base = priv->cfg->mhccif_rc_base_addr;
>+
>+ /* Only allow one ch to be triggered at a time */
>+ if (!is_power_of_2(ch)) {
>+ dev_err((mdev)->dev, "Unsupported ext evt ch=0x%08x\n", ch);
>+ return -EINVAL;
>+ }
>+
>+ hw_bits = mtk_pci_ext_h2d_evt_hw_bits(ch);
>+ mtk_pci_write32(mdev, rc_base + MHCCIF_RC2EP_SW_BSY, hw_bits);
>+ mtk_pci_write32(mdev, rc_base + MHCCIF_RC2EP_SW_TCHNUM, ffs(hw_bits) - 1);
>+ return 0;
>+}
>+
>+static u32 mtk_pci_get_ext_evt_hw_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);
>+}
>+
>+int mtk_pci_fldr(struct mtk_md_dev *mdev)
>+{
>+#ifdef CONFIG_ACPI
>+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>+ acpi_status acpi_ret;
>+ acpi_handle handle;
>+
>+ if (acpi_disabled) {
>+ dev_err((mdev)->dev, "Unsupported, acpi function isn't enable\n");
>+ return -ENODEV;
>+ }
>+ handle = ACPI_HANDLE(mdev->dev);
>+ if (!handle) {
>+ dev_err((mdev)->dev, "Unsupported, acpi handle isn't found\n");
>+ return -ENODEV;
>+ }
>+ if (!acpi_has_method(handle, "_RST")) {
>+ dev_err((mdev)->dev, "Unsupported, _RST method isn't found\n");
>+ return -ENODEV;
>+ }
>+ acpi_ret = acpi_evaluate_object(handle, "_RST", NULL, &buffer);
>+ if (ACPI_FAILURE(acpi_ret)) {
>+ dev_err((mdev)->dev, "Failed to execute _RST method: %s\n",
>+ acpi_format_exception(acpi_ret));
>+ return -EFAULT;
>+ }
>+ acpi_os_free(buffer.pointer);
>+
>+ return 0;
>+#else

#else /* !CONFIG_ACPI */

>+ dev_err((mdev)->dev, "Unsupported, CONFIG ACPI hasn't been set to 'y'\n");
>+
>+ return -ENODEV;
>+#endif

#endif /* CONFIG_ACPI */

>+}
>+
>+int mtk_pci_pldr(struct mtk_md_dev *mdev)
>+{
>+#ifdef CONFIG_ACPI
>+ struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>+ struct pci_dev *bridge;
>+ acpi_status acpi_ret;
>+ acpi_handle handle;
>+
>+ if (acpi_disabled) {
>+ dev_err((mdev)->dev, "Unsupported, acpi function isn't enable\n");
>+ return -ENODEV;
>+ }
>+
>+ bridge = pci_upstream_bridge(to_pci_dev(mdev->dev));
>+ if (!bridge) {
>+ dev_err((mdev)->dev, "Unable to find bridge\n");
>+ return -ENODEV;
>+ }
>+
>+ handle = ACPI_HANDLE(&bridge->dev);
>+ if (!handle) {
>+ dev_err((mdev)->dev, "Unsupported, acpi handle isn't found\n");
>+ return -ENODEV;
>+ }
>+ if (!acpi_has_method(handle, "PXP._OFF") ||
>+ !acpi_has_method(handle, "PXP._ON")) {
>+ dev_err((mdev)->dev, "Unsupported, pldr method isn't supported\n");
>+ return -ENODEV;
>+ }
>+ acpi_ret = acpi_evaluate_object(handle, "PXP._OFF", NULL, &buffer);
>+ if (ACPI_FAILURE(acpi_ret)) {
>+ dev_err((mdev)->dev, "Failed to execute _OFF method: %s\n",
>+ acpi_format_exception(acpi_ret));
>+ return -EFAULT;
>+ }
>+ msleep(500);

please dont use magic number
also where this value has been derived from?

>+ acpi_ret = acpi_evaluate_object(handle, "PXP._ON", NULL, &buffer);
>+ if (ACPI_FAILURE(acpi_ret)) {
>+ dev_err((mdev)->dev, "Failed to execute _ON method: %s\n",
>+ acpi_format_exception(acpi_ret));
>+ return -EFAULT;
>+ }
>+ acpi_os_free(buffer.pointer);

pleae add some newlines

>+
>+ return 0;
>+#else
>+ dev_err((mdev)->dev, "Unsupported, CONFIG ACPI hasn't been set to 'y'\n");
>+
>+ return -ENODEV;
>+#endif
>+}
>+
>+u32 mtk_pci_get_dev_cfg(struct mtk_md_dev *mdev)
>+{
>+ u32 val;
>+
>+ val = mtk_pci_mac_read32(mdev->hw_priv, REG_PCIE_DEBUG_DUMMY_4);
>+ return (val >> MTK_CFG_INFO_BIT_SHIFT);
>+}
>+
>+static int mtk_pci_dev_reset(struct mtk_md_dev *mdev, enum mtk_reset_type type)
>+{
>+ switch (type) {
>+ case RESET_MHCCIF:
>+ return mtk_pci_send_ext_evt(mdev, DEV_EVT_H2D_DEVICE_RESET);
>+ case RESET_FLDR:
>+ return mtk_pci_fldr(mdev);
>+ case RESET_PLDR:
>+ return mtk_pci_pldr(mdev);
>+ default:
>+ break;
>+ }
>+
>+ return -EINVAL;

please put return into default label

>+}
>+
>+int mtk_pci_reset(struct mtk_md_dev *mdev, enum mtk_reset_type type)
>+{
>+ return mtk_pci_dev_reset(mdev, type);
>+}
>+
>+bool mtk_pci_link_check(struct mtk_md_dev *mdev)
>+{
>+ return pci_device_is_present(to_pci_dev(mdev->dev));
>+}
>+
>+static void mtk_mhccif_isr_work(struct work_struct *work)
>+{
>+ struct mtk_pci_priv *priv = container_of(work, struct mtk_pci_priv, mhccif_work);

isn't this line > 80 chars?

>+ struct mtk_md_dev *mdev = priv->irq_desc->mdev;
>+ struct mtk_mhccif_cb *cb;
>+ u32 stat, mask, chs;
>+
>+ stat = mtk_pci_get_ext_evt_hw_status(mdev);
>+ mask = mtk_pci_read32(mdev, priv->cfg->mhccif_rc_base_addr
>+ + MHCCIF_EP2RC_SW_INT_EAP_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;
>+ chs = mtk_pci_ext_d2h_evt_chs(stat);
>+ spin_lock_bh(&priv->mhccif_lock);
>+ list_for_each_entry(cb, &priv->mhccif_cb_list, entry) {
>+ if (cb->chs & chs)
>+ cb->evt_cb(cb->chs & chs, cb->data);
>+ }
>+ spin_unlock_bh(&priv->mhccif_lock);
>+
>+ mtk_pci_clear_irq(mdev, priv->mhccif_irq_id);
>+ mtk_pci_unmask_irq(mdev, priv->mhccif_irq_id);
>+}
>+
>+static const struct pci_device_id t9xx_pci_table[] = {
>+ MTK_PCI_DEV_CFG(0x0900, mtk_dev_cfg_0900),
>+ CEI_PCI_DEV_CFG(0x01CA, mtk_dev_cfg_0900),
>+ {/* end: all zeroes */}
>+};
>+
>+MODULE_DEVICE_TABLE(pci, t9xx_pci_table);
>+
>+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;
>+ u32 bar[BAR_NUM];
>+ int i, ret;
>+
>+ for (i = 0; i < BAR_NUM; i++)
>+ pci_read_config_dword(to_pci_dev(mdev->dev),
>+ PCI_BASE_ADDRESS_0 + (i << 2), bar + i);
>+
>+ 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];
>+ if (!priv->mac_reg_base || !priv->bar23_addr) {
>+ dev_err((mdev)->dev, "Failed to init BAR.\n");
>+ return -EINVAL;
>+ }
>+ /* 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_region(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;

why cannot just return ret?

>+ }
>+ 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;

it's redundant

>+ }
>+
>+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;

are we sure irq_state cannot be 0?

>+ 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);
>+ 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, mdev->dev_str);
>+ 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);
>+
>+ 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 const struct mtk_dev_ops pci_hw_ops = {
>+ .get_dev_state = mtk_pci_get_dev_state,
>+ .ack_dev_state = mtk_pci_ack_dev_state,
>+ .get_dev_cfg = mtk_pci_get_dev_cfg,
>+ .register_dev_evt = mtk_pci_register_ext_evt,
>+ .unregister_dev_evt = mtk_pci_unregister_ext_evt,
>+ .mask_dev_evt = mtk_pci_mask_ext_evt,
>+ .unmask_dev_evt = mtk_pci_unmask_ext_evt,
>+ .clear_dev_evt = mtk_pci_clear_ext_evt,
>+ .send_dev_evt = mtk_pci_send_ext_evt,
>+};
>+
>+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 out;

as for the rest of the labels please name what is done
eg log_err

>+ }
>+ mdev->dev_ops = &pci_hw_ops;
>+ mdev->dev = dev;
>+
>+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>+ if (!priv) {
>+ ret = -ENOMEM;
>+ goto free_cntx_data;
>+ }
>+
>+ pci_set_drvdata(pdev, mdev);
>+ priv->cfg = (void *)id->driver_data;
>+ priv->mdev = mdev;
>+ mdev->hw_ver = pdev->device;
>+ 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));
>+ if (pdev->state_saved)
>+ pci_restore_state(pdev);
>+
>+ 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 = priv->cfg->atr_init(mdev);
>+ if (ret)
>+ goto free_bar;
>+
>+ ret = mtk_mhccif_init(mdev);
>+ if (ret)
>+ goto free_bar;
>+
>+ /* mask all irqs */
>+ if (priv->cfg->flag & MTK_CFG_IRQ_DFLT_MASK)
>+ mtk_pci_mac_write32(priv, REG_IMASK_HOST_MSIX_CLR_GRP0_0, U32_MAX);
>+
>+ ret = mtk_pci_request_irq(mdev);
>+ if (ret)
>+ goto free_mhccif;
>+
>+ pci_set_master(pdev);
>+ mtk_pci_unmask_irq(mdev, priv->mhccif_irq_id);
>+
>+ if (mtk_pci_link_check(mdev)) {
>+ pci_save_state(pdev);
>+ } else {
>+ ret = -EFAULT;
>+ goto clear_master;

#define EFAULT 14 /* Bad address */
does it suit here?

>+ }
>+
>+ priv->saved_state = pci_store_saved_state(pdev);
>+ if (!priv->saved_state) {
>+ ret = -EFAULT;
>+ goto clear_master;
>+ }
>+
>+ return 0;
>+
>+clear_master:
>+ pci_clear_master(pdev);
>+ mtk_pci_free_irq(mdev);
>+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);
>+out:
>+ dev_err(dev, "Failed to probe device, ret=%d\n", ret);
>+
>+ return ret;
>+}
>+

please also take a look on sashiko notes, there is some number of them