RE: [PATCH 01/11] net: wwan: t9xx: Add PCIe core
flat view
From: Jagielski, Jedrzej <hidden>
Date: 2026-06-01 11:18:19
Also in:
linux-arm-kernel, linux-doc, linux-mediatek, lkml
From: Jack Wu via B4 Relay <devnull+jackbb_wu.compal.com@kernel.org> Sent: Friday, May 29, 2026 12:32 PM
quoted hunk ↗ jump to hunk
From: Jack Wu <redacted> 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 <redacted> --- 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_T7XXIf 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 endmenudiff --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.oobj-$(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.odiff --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
quoted hunk ↗ jump to hunk
+#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?
quoted hunk ↗ jump to hunk
+ (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
quoted hunk ↗ jump to hunk
+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
quoted hunk ↗ jump to hunk
+ } + + /* 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
quoted hunk ↗ jump to hunk
+ + /* 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
quoted hunk ↗ jump to hunk
+ + 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?
quoted hunk ↗ jump to hunk
+ + 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?
quoted hunk ↗ jump to hunk
+ + 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
quoted hunk ↗ jump to hunk
+ 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
quoted hunk ↗ jump to hunk
+ + 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
quoted hunk ↗ jump to hunk
+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
quoted hunk ↗ jump to hunk
+ + 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 */
quoted hunk ↗ jump to hunk
+ dev_err((mdev)->dev, "Unsupported, CONFIG ACPI hasn't been set to 'y'\n"); + + return -ENODEV; +#endif
#endif /* CONFIG_ACPI */
quoted hunk ↗ jump to hunk
+} + +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?
quoted hunk ↗ jump to hunk
+ 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
quoted hunk ↗ jump to hunk
+ + 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
quoted hunk ↗ jump to hunk
+} + +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?
quoted hunk ↗ jump to hunk
+ 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?
quoted hunk ↗ jump to hunk
+ } + 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
quoted hunk ↗ jump to hunk
+ } + +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?
quoted hunk ↗ jump to hunk
+ 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
quoted hunk ↗ jump to hunk
+ } + 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?
quoted hunk ↗ jump to hunk
+ } + + 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