On 2021-01-13 11:39, Jianjun Wang wrote:
> Add MSI support for MediaTek Gen3 PCIe controller.
>
> This PCIe controller supports up to 256 MSI vectors, the MSI hardware
> block diagram is as follows:
>
> +-----+
> | GIC |
> +-----+
> ^
> |
> port->irq
> |
> +-+-+-+-+-+-+-+-+
> |0|1|2|3|4|5|6|7| (PCIe intc)
> +-+-+-+-+-+-+-+-+
> ^ ^ ^
> | | ... |
> +-------+ +------+ +-----------+
> | | |
> +-+-+---+--+--+ +-+-+---+--+--+ +-+-+---+--+--+
> |0|1|...|30|31| |0|1|...|30|31| |0|1|...|30|31| (MSI sets)
> +-+-+---+--+--+ +-+-+---+--+--+ +-+-+---+--+--+
> ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^ ^
> | | | | | | | | | | | | (MSI vectors)
> | | | | | | | | | | | |
>
> (MSI SET0) (MSI SET1) ... (MSI SET7)
>
> With 256 MSI vectors supported, the MSI vectors are composed of 8 sets,
> each set has its own address for MSI message, and supports 32 MSI
> vectors
> to generate interrupt.
>
> Signed-off-by: Jianjun Wang <jianjun.wang@xxxxxxxxxxxx>
> Acked-by: Ryder Lee <ryder.lee@xxxxxxxxxxxx>
> ---
> drivers/pci/controller/pcie-mediatek-gen3.c | 261 ++++++++++++++++++++
> 1 file changed, 261 insertions(+)
>
> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c
> b/drivers/pci/controller/pcie-mediatek-gen3.c
> index 7979a2856c35..471d97cd1ef9 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> @@ -14,6 +14,7 @@
> #include <linux/irqdomain.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/msi.h>
> #include <linux/of_address.h>
> #include <linux/of_clk.h>
> #include <linux/of_pci.h>
> @@ -52,11 +53,28 @@
> #define PCIE_LINK_STATUS_REG 0x154
> #define PCIE_PORT_LINKUP BIT(8)
>
> +#define PCIE_MSI_SET_NUM 8
> +#define PCIE_MSI_IRQS_PER_SET 32
> +#define PCIE_MSI_IRQS_NUM \
> + (PCIE_MSI_IRQS_PER_SET * (PCIE_MSI_SET_NUM))
Spurious inner bracketing.
> +
> #define PCIE_INT_ENABLE_REG 0x180
> +#define PCIE_MSI_ENABLE GENMASK(PCIE_MSI_SET_NUM + 8 - 1, 8)
> +#define PCIE_MSI_SHIFT 8
> #define PCIE_INTX_SHIFT 24
> #define PCIE_INTX_MASK GENMASK(27, 24)
>
> #define PCIE_INT_STATUS_REG 0x184
> +#define PCIE_MSI_SET_ENABLE_REG 0x190
> +#define PCIE_MSI_SET_ENABLE GENMASK(PCIE_MSI_SET_NUM - 1, 0)
> +
> +#define PCIE_MSI_SET_BASE_REG 0xc00
> +#define PCIE_MSI_SET_OFFSET 0x10
> +#define PCIE_MSI_SET_STATUS_OFFSET 0x04
> +#define PCIE_MSI_SET_ENABLE_OFFSET 0x08
> +
> +#define PCIE_MSI_SET_ADDR_HI_BASE 0xc80
> +#define PCIE_MSI_SET_ADDR_HI_OFFSET 0x04
>
> #define PCIE_TRANS_TABLE_BASE_REG 0x800
> #define PCIE_ATR_SRC_ADDR_MSB_OFFSET 0x4
> @@ -76,6 +94,18 @@
> #define PCIE_ATR_TLP_TYPE_MEM PCIE_ATR_TLP_TYPE(0)
> #define PCIE_ATR_TLP_TYPE_IO PCIE_ATR_TLP_TYPE(2)
>
> +/**
> + * struct mtk_pcie_msi - MSI information for each set
> + * @dev: pointer to PCIe device
> + * @base: IO mapped register base
> + * @msg_addr: MSI message address
> + */
> +struct mtk_msi_set {
> + struct device *dev;
> + void __iomem *base;
> + phys_addr_t msg_addr;
> +};
> +
> /**
> * struct mtk_pcie_port - PCIe port information
> * @dev: pointer to PCIe device
> @@ -88,6 +118,11 @@
> * @num_clks: PCIe clocks count for this port
> * @irq: PCIe controller interrupt number
> * @intx_domain: legacy INTx IRQ domain
> + * @msi_domain: MSI IRQ domain
> + * @msi_bottom_domain: MSI IRQ bottom domain
> + * @msi_sets: MSI sets information
> + * @lock: lock protecting IRQ bit map
> + * @msi_irq_in_use: bit map for assigned MSI IRQ
> */
> struct mtk_pcie_port {
> struct device *dev;
> @@ -101,6 +136,11 @@ struct mtk_pcie_port {
>
> int irq;
> struct irq_domain *intx_domain;
> + struct irq_domain *msi_domain;
> + struct irq_domain *msi_bottom_domain;
> + struct mtk_msi_set msi_sets[PCIE_MSI_SET_NUM];
> + struct mutex lock;
> + DECLARE_BITMAP(msi_irq_in_use, PCIE_MSI_IRQS_NUM);
> };
>
> /**
> @@ -243,6 +283,15 @@ static int mtk_pcie_startup_port(struct
> mtk_pcie_port *port)
> return err;
> }
>
> + /* Enable MSI */
> + val = readl_relaxed(port->base + PCIE_MSI_SET_ENABLE_REG);
> + val |= PCIE_MSI_SET_ENABLE;
> + writel_relaxed(val, port->base + PCIE_MSI_SET_ENABLE_REG);
> +
> + val = readl_relaxed(port->base + PCIE_INT_ENABLE_REG);
> + val |= PCIE_MSI_ENABLE;
> + writel_relaxed(val, port->base + PCIE_INT_ENABLE_REG);
> +
> /* Set PCIe translation windows */
> resource_list_for_each_entry(entry, &host->windows) {
> struct resource *res = entry->res;
> @@ -286,6 +335,129 @@ static int mtk_pcie_set_affinity(struct irq_data
> *data,
> return -EINVAL;
> }
>
> +static struct irq_chip mtk_msi_irq_chip = {
> + .name = "MSI",
> + .irq_ack = irq_chip_ack_parent,
> +};
> +
> +static struct msi_domain_info mtk_msi_domain_info = {
> + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_PCI_MSIX |
> + MSI_FLAG_USE_DEF_CHIP_OPS | MSI_FLAG_MULTI_PCI_MSI),
> + .chip = &mtk_msi_irq_chip,
> +};
> +
> +static void mtk_compose_msi_msg(struct irq_data *data, struct msi_msg
> *msg)
> +{
> + struct mtk_msi_set *msi_set = irq_data_get_irq_chip_data(data);
> + unsigned long hwirq;
> +
> + hwirq = data->hwirq % PCIE_MSI_IRQS_PER_SET;
> +
> + msg->address_hi = upper_32_bits(msi_set->msg_addr);
> + msg->address_lo = lower_32_bits(msi_set->msg_addr);
> + msg->data = hwirq;
> + dev_dbg(msi_set->dev, "msi#%#lx address_hi %#x address_lo %#x data
> %d\n",
> + hwirq, msg->address_hi, msg->address_lo, msg->data);
> +}
> +
> +static void mtk_msi_bottom_irq_ack(struct irq_data *data)
> +{
> + struct mtk_msi_set *msi_set = irq_data_get_irq_chip_data(data);
> + unsigned long hwirq;
> +
> + hwirq = data->hwirq % PCIE_MSI_IRQS_PER_SET;
> +
> + writel_relaxed(BIT(hwirq), msi_set->base +
> PCIE_MSI_SET_STATUS_OFFSET);
> +}
> +
> +static struct irq_chip mtk_msi_bottom_irq_chip = {
> + .irq_ack = mtk_msi_bottom_irq_ack,
> + .irq_compose_msi_msg = mtk_compose_msi_msg,
> + .irq_set_affinity = mtk_pcie_set_affinity,
> + .name = "PCIe",
nit: "MSI", rather than "PCIe".
> +};
> +
> +static int mtk_msi_bottom_domain_alloc(struct irq_domain *domain,
> + unsigned int virq, unsigned int nr_irqs,
> + void *arg)
> +{
> + struct mtk_pcie_port *port = domain->host_data;
> + struct mtk_msi_set *msi_set;
> + int i, hwirq, set_idx;
> +
> + mutex_lock(&port->lock);
> +
> + hwirq = bitmap_find_free_region(port->msi_irq_in_use,
> PCIE_MSI_IRQS_NUM,
> + order_base_2(nr_irqs));
> +
> + mutex_unlock(&port->lock);
> +
> + if (hwirq < 0)
> + return -ENOSPC;
> +
> + set_idx = hwirq / PCIE_MSI_IRQS_PER_SET;
> + msi_set = &port->msi_sets[set_idx];
> +
> + for (i = 0; i < nr_irqs; i++)
> + irq_domain_set_info(domain, virq + i, hwirq + i,
> + &mtk_msi_bottom_irq_chip, msi_set,
> + handle_edge_irq, NULL, NULL);
> +
> + return 0;
> +}
> +
> +static void mtk_msi_bottom_domain_free(struct irq_domain *domain,
> + unsigned int virq, unsigned int nr_irqs)
> +{
> + struct mtk_pcie_port *port = domain->host_data;
> + struct irq_data *data = irq_domain_get_irq_data(domain, virq);
> +
> + mutex_lock(&port->lock);
> +
> + bitmap_clear(port->msi_irq_in_use, data->hwirq, nr_irqs);
> +
> + mutex_unlock(&port->lock);
> +
> + irq_domain_free_irqs_common(domain, virq, nr_irqs);
> +}
> +
> +static int mtk_msi_bottom_domain_activate(struct irq_domain *domain,
> + struct irq_data *data, bool reserve)
> +{
> + struct mtk_msi_set *msi_set = irq_data_get_irq_chip_data(data);
> + unsigned long hwirq;
> + u32 val;
> +
> + hwirq = data->hwirq % PCIE_MSI_IRQS_PER_SET;
> +
> + val = readl_relaxed(msi_set->base + PCIE_MSI_SET_ENABLE_OFFSET);
> + val |= BIT(hwirq);
> + writel_relaxed(val, msi_set->base + PCIE_MSI_SET_ENABLE_OFFSET);
This isn't an activate. This is an unmask, which suffers from the same
issue as its INTx sibling.
> +
> + return 0;
> +}
> +
> +static void mtk_msi_bottom_domain_deactivate(struct irq_domain
> *domain,
> + struct irq_data *data)
> +{
> + struct mtk_msi_set *msi_set = irq_data_get_irq_chip_data(data);
> + unsigned long hwirq;
> + u32 val;
> +
> + hwirq = data->hwirq % PCIE_MSI_IRQS_PER_SET;
> +
> + val = readl_relaxed(msi_set->base + PCIE_MSI_SET_ENABLE_OFFSET);
> + val &= ~BIT(hwirq);
> + writel_relaxed(val, msi_set->base + PCIE_MSI_SET_ENABLE_OFFSET);
> +}
Same thing, this is a mask. I don't think this block requires any
activate/deactivate callbacks for its lower irqdomain.
As it stands, you can't mask a MSI at the low-level, which is
pretty bad (you need to mask them at the PCI source, which can
end-up disabling all vectors in the case of Multi-MSI).
