| Message ID | 20191112155926.16476-6-nsaenzjulienne@suse.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Raspberry Pi 4 PCIe support | expand |
On 2019-11-12 17:08, Nicolas Saenz Julienne wrote: > From: Jim Quinlan <james.quinlan@broadcom.com> > > This commit adds MSI to the Broadcom STB PCIe host controller. It > does > not add MSIX since that functionality is not in the HW. The MSI > controller is physically located within the PCIe block, however, > there > is no reason why the MSI controller could not be moved elsewhere in > the future. > > Since the internal Brcmstb MSI controller is intertwined with the > PCIe > controller, it is not its own platform device but rather part of the > PCIe platform device. > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > Co-developed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Reviewed-by: Marc Zyngier <maz@kernel.org> M.
On Tue, Nov 12, 2019 at 04:59:24PM +0100, Nicolas Saenz Julienne wrote: > From: Jim Quinlan <james.quinlan@broadcom.com> > > This commit adds MSI to the Broadcom STB PCIe host controller. It does > not add MSIX since that functionality is not in the HW. The MSI > controller is physically located within the PCIe block, however, there > is no reason why the MSI controller could not be moved elsewhere in > the future. > > Since the internal Brcmstb MSI controller is intertwined with the PCIe > controller, it is not its own platform device but rather part of the > PCIe platform device. > > Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com> > Co-developed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> > > --- > > Changes since v1: > - Move revision code and some registers to this patch > - Use PCIE_MSI_IRQ_DOMAIN in Kconfig > - Remove redundant register read from ISR > - Fail probe on MSI init error > - Get rid of msi_internal > - Use bitmap family of functions > - Use edge triggered setup > - Add comment regarding MultiMSI > - Simplify compose_msi_msg to avoid reg read > > This is based on Jim's original submission[1] with some slight changes > regarding how pcie->msi_target_addr is decided. > > [1] https://patchwork.kernel.org/patch/10605955/ > > drivers/pci/controller/Kconfig | 1 + > drivers/pci/controller/pcie-brcmstb.c | 326 +++++++++++++++++++++++++- > 2 files changed, 326 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig > index 486d4d570233..eb9d973ad8e5 100644 > --- a/drivers/pci/controller/Kconfig > +++ b/drivers/pci/controller/Kconfig > @@ -285,6 +285,7 @@ config PCIE_BRCMSTB > bool "Broadcom Brcmstb PCIe host controller" > depends on ARCH_BCM2835 || COMPILE_TEST > depends on OF > + depends on PCI_MSI_IRQ_DOMAIN > help > Say Y here to enable PCIe host controller support for > Broadcom STB based SoCs, like the Raspberry Pi 4. > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index 8d81c5ab5979..35afd82107a7 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -1,6 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0+ > /* Copyright (C) 2009 - 2019 Broadcom */ > > +#include <linux/bitops.h> > #include <linux/clk.h> > #include <linux/compiler.h> > #include <linux/delay.h> > @@ -8,11 +9,13 @@ > #include <linux/interrupt.h> > #include <linux/io.h> > #include <linux/ioport.h> > +#include <linux/irqchip/chained_irq.h> > #include <linux/irqdomain.h> > #include <linux/kernel.h> > #include <linux/list.h> > #include <linux/log2.h> > #include <linux/module.h> > +#include <linux/msi.h> > #include <linux/of_address.h> > #include <linux/of_irq.h> > #include <linux/of_pci.h> > @@ -46,13 +49,18 @@ > #define PCIE_MISC_RC_BAR2_CONFIG_LO 0x4034 > #define PCIE_MISC_RC_BAR2_CONFIG_HI 0x4038 > #define PCIE_MISC_RC_BAR3_CONFIG_LO 0x403c > +#define PCIE_MISC_MSI_BAR_CONFIG_LO 0x4044 > +#define PCIE_MISC_MSI_BAR_CONFIG_HI 0x4048 > +#define PCIE_MISC_MSI_DATA_CONFIG 0x404c > #define PCIE_MISC_PCIE_CTRL 0x4064 > #define PCIE_MISC_PCIE_STATUS 0x4068 > +#define PCIE_MISC_REVISION 0x406c > #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT 0x4070 > #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI 0x4080 > #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI 0x4084 > #define PCIE_MISC_HARD_PCIE_HARD_DEBUG 0x4204 > #define PCIE_INTR2_CPU_BASE 0x4300 > +#define PCIE_MSI_INTR2_BASE 0x4500 > > /* > * Broadcom STB PCIe Register Field shift and mask info. The names are from the > @@ -87,6 +95,8 @@ > #define PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_SHIFT 0x4 > #define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK 0x40 > #define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_SHIFT 0x6 > +#define PCIE_MISC_REVISION_MAJMIN_MASK 0xffff > +#define PCIE_MISC_REVISION_MAJMIN_SHIFT 0 I don't think these two are used. > #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK 0xfff00000 > #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_SHIFT 0x14 > #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK 0xfff0 > @@ -104,6 +114,11 @@ > #define PCIE_RGR1_SW_INIT_1_PERST_SHIFT 0x0 > > #define BRCM_NUM_PCIE_OUT_WINS 0x4 > +#define BRCM_INT_PCI_MSI_NR 32 > +#define BRCM_PCIE_HW_REV_33 0x0303 > + > +#define BRCM_MSI_TARGET_ADDR_LT_4GB 0x0fffffffcULL > +#define BRCM_MSI_TARGET_ADDR_GT_4GB 0xffffffffcULL > > #define BURST_SIZE_128 0 > > @@ -182,6 +197,33 @@ struct brcm_window { > dma_addr_t size; > }; > > +struct brcm_msi { > + struct device *dev; > + void __iomem *base; > + struct device_node *np; > + struct irq_domain *msi_domain; > + struct irq_domain *inner_domain; > + struct mutex lock; /* guards the alloc/free operations */ > + u64 target_addr; > + int irq; > + > + /* intr_base is the base pointer for interrupt status/set/clr regs */ > + void __iomem *intr_base; > + > + /* intr_legacy_mask indicates how many bits are MSI interrupts */ > + u32 intr_legacy_mask; > + > + /* > + * intr_legacy_offset indicates bit position of MSI_01. It is > + * to map the register bit position to a hwirq that starts at 0. > + */ > + u32 intr_legacy_offset; > + > + /* used indicates which MSI interrupts have been alloc'd */ > + unsigned long used; > + unsigned int rev; > +}; > + > /* Internal PCIe Host Controller Information.*/ > struct brcm_pcie { > struct device *dev; > @@ -191,7 +233,10 @@ struct brcm_pcie { > struct device_node *np; > bool ssc; > int gen; > + u64 msi_target_addr; > struct brcm_window out_wins[BRCM_NUM_PCIE_OUT_WINS]; > + struct brcm_msi *msi; > + unsigned int rev; > const int *reg_offsets; > const int *reg_field_info; > enum pcie_type type; > @@ -433,6 +478,259 @@ static void brcm_pcie_set_outbound_win(struct brcm_pcie *pcie, > LIMIT, tmp); > } > > +static struct irq_chip brcm_msi_irq_chip = { > + .name = "BRCM STB PCIe MSI", > + .irq_ack = irq_chip_ack_parent, > + .irq_mask = pci_msi_mask_irq, > + .irq_unmask = pci_msi_unmask_irq, > +}; > + > +static struct msi_domain_info brcm_msi_domain_info = { > + /* TODO: Multi MSI is technically supported by the controller */ As I understand we're not supporting MSI_FLAG_MULTI_PCI_MSI, not because there is no hardware capability, but because the current use-case (RPi EPs) have no need for it. It wouldn't be a stretch to add this support (compare your alloc implementation with nwl_irq_domain_alloc from pcie-xilinx-nwl.c) - though I appreciate the difficulity you may have with testing. I'd probably replace the TODO line with: /* Multi MSI is supported by the controller, but not by this driver */ > + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | > + MSI_FLAG_PCI_MSIX), Why the MSIX flag if the commit message says "It does not add MSIX since that functionality is not in the HW." in the commit message? > + .chip = &brcm_msi_irq_chip, > +}; > + > +static void brcm_pcie_msi_isr(struct irq_desc *desc) > +{ > + struct irq_chip *chip = irq_desc_get_chip(desc); > + unsigned long status, virq; > + struct brcm_msi *msi; > + u32 mask, bit, hwirq; > + struct device *dev; > + > + chained_irq_enter(chip, desc); > + msi = irq_desc_get_handler_data(desc); > + mask = msi->intr_legacy_mask; NIT: As you only use this variable once, you could get rid of it. > + dev = msi->dev; > + > + status = bcm_readl(msi->intr_base + STATUS) & mask; > + for_each_set_bit(bit, &status, BRCM_INT_PCI_MSI_NR) { > + > + /* Account for legacy interrupt offset */ > + hwirq = bit - msi->intr_legacy_offset; > + virq = irq_find_mapping(msi->inner_domain, hwirq); > + if (virq) > + generic_handle_irq(virq); > + else > + dev_dbg(dev, "unexpected MSI\n"); > + } > + chained_irq_exit(chip, desc); > +} > + > +static void brcm_msi_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) > +{ > + struct brcm_msi *msi = irq_data_get_irq_chip_data(data); > + > + msg->address_lo = lower_32_bits(msi->target_addr); > + msg->address_hi = upper_32_bits(msi->target_addr); > + msg->data = 0x6540 | data->hwirq; > +} > + > +static int brcm_msi_set_affinity(struct irq_data *irq_data, > + const struct cpumask *mask, bool force) > +{ > + return -EINVAL; > +} > + > +static void brcm_msi_ack_irq(struct irq_data *data) > +{ > + struct brcm_msi *msi = irq_data_get_irq_chip_data(data); > + u32 bit = data->hwirq + msi->intr_legacy_offset; > + > + bcm_writel(1 << bit, msi->intr_base + CLR); > +} > + > + > +static struct irq_chip brcm_msi_bottom_irq_chip = { > + .name = "BRCM STB MSI", > + .irq_compose_msi_msg = brcm_msi_compose_msi_msg, > + .irq_set_affinity = brcm_msi_set_affinity, > + .irq_ack = brcm_msi_ack_irq, > +}; > + > +static int brcm_msi_alloc(struct brcm_msi *msi) > +{ > + int hwirq; > + > + mutex_lock(&msi->lock); > + hwirq = bitmap_find_free_region(&msi->used, BRCM_INT_PCI_MSI_NR, 0); > + if (hwirq >= 0) > + hwirq = hwirq - msi->intr_legacy_offset; > + mutex_unlock(&msi->lock); > + > + return hwirq; > +} > + > +static void brcm_msi_free(struct brcm_msi *msi, unsigned long hwirq) > +{ > + mutex_lock(&msi->lock); > + bitmap_release_region(&msi->used, hwirq + msi->intr_legacy_offset, 0); > + mutex_unlock(&msi->lock); > +} > + > +static int brcm_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, > + unsigned int nr_irqs, void *args) > +{ > + struct brcm_msi *msi = domain->host_data; > + int hwirq; > + > + hwirq = brcm_msi_alloc(msi); > + > + if (hwirq < 0) > + return hwirq; > + > + irq_domain_set_info(domain, virq, (irq_hw_number_t)hwirq, > + &brcm_msi_bottom_irq_chip, domain->host_data, > + handle_edge_irq, NULL, NULL); > + return 0; > +} > + > +static void brcm_irq_domain_free(struct irq_domain *domain, > + unsigned int virq, unsigned int nr_irqs) > +{ > + struct irq_data *d = irq_domain_get_irq_data(domain, virq); > + struct brcm_msi *msi = irq_data_get_irq_chip_data(d); > + > + brcm_msi_free(msi, d->hwirq); > +} > + > +static const struct irq_domain_ops msi_domain_ops = { > + .alloc = brcm_irq_domain_alloc, > + .free = brcm_irq_domain_free, > +}; > + > +static int brcm_allocate_domains(struct brcm_msi *msi) > +{ > + struct fwnode_handle *fwnode = of_node_to_fwnode(msi->np); > + struct device *dev = msi->dev; > + > + msi->inner_domain = irq_domain_add_linear(NULL, BRCM_INT_PCI_MSI_NR, > + &msi_domain_ops, msi); > + if (!msi->inner_domain) { > + dev_err(dev, "failed to create IRQ domain\n"); > + return -ENOMEM; > + } > + > + msi->msi_domain = pci_msi_create_irq_domain(fwnode, > + &brcm_msi_domain_info, > + msi->inner_domain); > + if (!msi->msi_domain) { > + dev_err(dev, "failed to create MSI domain\n"); > + irq_domain_remove(msi->inner_domain); > + return -ENOMEM; > + } > + > + return 0; > +} > + > +static void brcm_free_domains(struct brcm_msi *msi) > +{ > + irq_domain_remove(msi->msi_domain); > + irq_domain_remove(msi->inner_domain); > +} > + > +static void brcm_msi_remove(struct brcm_pcie *pcie) > +{ > + struct brcm_msi *msi = pcie->msi; > + > + if (!msi) > + return; > + irq_set_chained_handler(msi->irq, NULL); > + irq_set_handler_data(msi->irq, NULL); > + brcm_free_domains(msi); > +} > + > +static void brcm_msi_set_regs(struct brcm_msi *msi) > +{ > + u32 data_val, msi_lo, msi_hi; > + > + if (msi->rev >= BRCM_PCIE_HW_REV_33) { > + /* > + * ffe0 -- least sig 5 bits are 0 indicating 32 msgs > + * 6540 -- this is our arbitrary unique data value > + */ > + data_val = 0xffe06540; > + } else { > + /* > + * fff8 -- least sig 3 bits are 0 indicating 8 msgs > + * 6540 -- this is our arbitrary unique data value > + */ > + data_val = 0xfff86540; > + } > + > + /* Unmaks MSIs */ > + bcm_writel(0xffffffff & msi->intr_legacy_mask, > + msi->intr_base + MASK_CLR); > + > + msi_lo = lower_32_bits(msi->target_addr); > + msi_hi = upper_32_bits(msi->target_addr); > + /* > + * The 0 bit of PCIE_MISC_MSI_BAR_CONFIG_LO is repurposed to MSI > + * enable, which we set to 1. > + */ > + bcm_writel(msi_lo | 1, msi->base + PCIE_MISC_MSI_BAR_CONFIG_LO); > + bcm_writel(msi_hi, msi->base + PCIE_MISC_MSI_BAR_CONFIG_HI); > + bcm_writel(data_val, msi->base + PCIE_MISC_MSI_DATA_CONFIG); > +} > + > +static int brcm_pcie_enable_msi(struct brcm_pcie *pcie) > +{ > + struct brcm_msi *msi; > + int irq, ret; > + struct device *dev = pcie->dev; > + > + irq = irq_of_parse_and_map(dev->of_node, 1); > + if (irq <= 0) { > + dev_err(dev, "cannot map msi intr\n"); NIT: I think we can spare a few more characters and replace intr with interrupt. > + return -ENODEV; > + } > + > + msi = devm_kzalloc(dev, sizeof(struct brcm_msi), GFP_KERNEL); > + if (!msi) > + return -ENOMEM; > + > + msi->dev = dev; > + msi->base = pcie->base; > + msi->rev = pcie->rev; > + msi->np = pcie->np; > + msi->target_addr = pcie->msi_target_addr; > + msi->irq = irq; > + > + ret = brcm_allocate_domains(msi); > + if (ret) > + return ret; > + > + irq_set_chained_handler_and_data(msi->irq, brcm_pcie_msi_isr, msi); > + > + if (msi->rev >= BRCM_PCIE_HW_REV_33) { > + msi->intr_base = msi->base + PCIE_MSI_INTR2_BASE; > + /* > + * This version of PCIe hw has only 32 intr bits > + * starting at bit position 0. > + */ > + msi->intr_legacy_mask = 0xffffffff; > + msi->intr_legacy_offset = 0x0; > + msi->used = 0x0; > + > + } else { > + msi->intr_base = msi->base + PCIE_INTR2_CPU_BASE; > + /* > + * This version of PCIe hw has only 8 intr bits starting > + * at bit position 24. > + */ > + msi->intr_legacy_mask = 0xff000000; > + msi->intr_legacy_offset = 24; > + msi->used = 0x00ffffff; > + } > + > + brcm_msi_set_regs(msi); > + pcie->msi = msi; > + > + return 0; > +} > + > /* Configuration space read/write support */ > static int brcm_pcie_cfg_index(int busnr, int devfn, int reg) > { > @@ -584,6 +882,10 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie) > /* Wait for SerDes to be stable */ > usleep_range(100, 200); > > + /* Grab the PCIe hw revision number */ > + tmp = bcm_readl(base + PCIE_MISC_REVISION); > + pcie->rev = EXTRACT_FIELD(tmp, PCIE_MISC_REVISION, MAJMIN); > + > /* Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN */ > tmp = INSERT_FIELD(0, PCIE_MISC_MISC_CTRL, SCB_ACCESS_EN, 1); > tmp = INSERT_FIELD(tmp, PCIE_MISC_MISC_CTRL, CFG_READ_UR_MODE, 1); > @@ -596,6 +898,18 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie) > if (ret) > return ret; > > + /* > + * We ideally want the MSI target address to be located in the 32bit > + * addressable memory area. Some devices might depend on it. This is > + * possible either when the inbound window is located above the lower > + * 4GB or when the inbound and outbound areas fit in the lower 4GB of > + * memory. > + */ > + if (rc_bar2_offset >= SZ_4G || (rc_bar2_size + rc_bar2_offset) <= SZ_4G) > + pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_LT_4GB; > + else > + pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_GT_4GB; > + Can this above hunk me moved into brcm_pcie_enable_msi? You could then avoid having the pcie->msi_target_addr and just have a single msi->target_addr variable? Thanks, Andrew Murray > tmp = lower_32_bits(rc_bar2_offset); > tmp = INSERT_FIELD(tmp, PCIE_MISC_RC_BAR2_CONFIG_LO, SIZE, > brcm_pcie_encode_ibar_size(rc_bar2_size)); > @@ -734,6 +1048,7 @@ static void brcm_pcie_turn_off(struct brcm_pcie *pcie) > > static void __brcm_pcie_remove(struct brcm_pcie *pcie) > { > + brcm_msi_remove(pcie); > brcm_pcie_turn_off(pcie); > clk_disable_unprepare(pcie->clk); > clk_put(pcie->clk); > @@ -758,7 +1073,7 @@ MODULE_DEVICE_TABLE(of, brcm_pcie_match); > > static int brcm_pcie_probe(struct platform_device *pdev) > { > - struct device_node *np = pdev->dev.of_node; > + struct device_node *np = pdev->dev.of_node, *msi_np; > const struct pcie_cfg_data *data; > struct pci_host_bridge *bridge; > struct brcm_pcie *pcie; > @@ -813,6 +1128,15 @@ static int brcm_pcie_probe(struct platform_device *pdev) > if (ret) > goto fail; > > + msi_np = of_parse_phandle(pcie->np, "msi-parent", 0); > + if (pci_msi_enabled() && msi_np == pcie->np) { > + ret = brcm_pcie_enable_msi(pcie); > + if (ret) { > + dev_err(pcie->dev, "probe of internal MSI failed"); > + goto fail; > + } > + } > + > bridge->dev.parent = &pdev->dev; > bridge->busnr = 0; > bridge->ops = &brcm_pcie_ops; > -- > 2.24.0 >
On Thu, 2019-11-21 at 15:38 +0000, Andrew Murray wrote: > > #define PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_SHIFT 0x4 > > #define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK 0x40 > > #define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_SHIFT 0x6 > > +#define PCIE_MISC_REVISION_MAJMIN_MASK 0xffff > > +#define PCIE_MISC_REVISION_MAJMIN_SHIFT 0 > > I don't think these two are used. Yes, in brcm_pcie_setup(), when we grab the PCIe hw revision number. [...] > > +static struct msi_domain_info brcm_msi_domain_info = { > > + /* TODO: Multi MSI is technically supported by the controller */ > > As I understand we're not supporting MSI_FLAG_MULTI_PCI_MSI, not because there > is no hardware capability, but because the current use-case (RPi EPs) have no > need for it. It wouldn't be a stretch to add this support (compare your alloc > implementation with nwl_irq_domain_alloc from pcie-xilinx-nwl.c) - though I > appreciate the difficulity you may have with testing. > > I'd probably replace the TODO line with: > > /* Multi MSI is supported by the controller, but not by this driver */ I'll replace the comment for now. I've already seen people who unsoldered the XHCI chip on the RPi4 to then add a proper PCI connector. If someone shows up with such setup, I'll be happy to work out the MultiMSI support. [...] > > + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | > > + MSI_FLAG_PCI_MSIX), > > Why the MSIX flag if the commit message says "It does not add MSIX since that > functionality is not in the HW." in the commit message? That's plain wrong, sorry. [...] > > + .chip = &brcm_msi_irq_chip, > > +}; > > + > > +static void brcm_pcie_msi_isr(struct irq_desc *desc) > > +{ > > + struct irq_chip *chip = irq_desc_get_chip(desc); > > + unsigned long status, virq; > > + struct brcm_msi *msi; > > + u32 mask, bit, hwirq; > > + struct device *dev; > > + > > + chained_irq_enter(chip, desc); > > + msi = irq_desc_get_handler_data(desc); > > + mask = msi->intr_legacy_mask; > > NIT: As you only use this variable once, you could get rid of it. OK [...] > > + > > +static int brcm_pcie_enable_msi(struct brcm_pcie *pcie) > > +{ > > + struct brcm_msi *msi; > > + int irq, ret; > > + struct device *dev = pcie->dev; > > + > > + irq = irq_of_parse_and_map(dev->of_node, 1); > > + if (irq <= 0) { > > + dev_err(dev, "cannot map msi intr\n"); > > NIT: I think we can spare a few more characters and replace intr with > interrupt. Of course. [...] > > + /* > > + * We ideally want the MSI target address to be located in the 32bit > > + * addressable memory area. Some devices might depend on it. This is > > + * possible either when the inbound window is located above the lower > > + * 4GB or when the inbound and outbound areas fit in the lower 4GB of > > + * memory. > > + */ > > + if (rc_bar2_offset >= SZ_4G || (rc_bar2_size + rc_bar2_offset) <= SZ_4G) > > + pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_LT_4GB; > > + else > > + pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_GT_4GB; > > + > > Can this above hunk me moved into brcm_pcie_enable_msi? You could then avoid > having the pcie->msi_target_addr and just have a single msi->target_addr > variable? As it depends on rc_bar2_offset and rc_bar2_size it's not really possible without having to store those values in exchange, which IMO amounts to negative benefit. Regards, Nicolas
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig index 486d4d570233..eb9d973ad8e5 100644 --- a/drivers/pci/controller/Kconfig +++ b/drivers/pci/controller/Kconfig @@ -285,6 +285,7 @@ config PCIE_BRCMSTB bool "Broadcom Brcmstb PCIe host controller" depends on ARCH_BCM2835 || COMPILE_TEST depends on OF + depends on PCI_MSI_IRQ_DOMAIN help Say Y here to enable PCIe host controller support for Broadcom STB based SoCs, like the Raspberry Pi 4. diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index 8d81c5ab5979..35afd82107a7 100644 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0+ /* Copyright (C) 2009 - 2019 Broadcom */ +#include <linux/bitops.h> #include <linux/clk.h> #include <linux/compiler.h> #include <linux/delay.h> @@ -8,11 +9,13 @@ #include <linux/interrupt.h> #include <linux/io.h> #include <linux/ioport.h> +#include <linux/irqchip/chained_irq.h> #include <linux/irqdomain.h> #include <linux/kernel.h> #include <linux/list.h> #include <linux/log2.h> #include <linux/module.h> +#include <linux/msi.h> #include <linux/of_address.h> #include <linux/of_irq.h> #include <linux/of_pci.h> @@ -46,13 +49,18 @@ #define PCIE_MISC_RC_BAR2_CONFIG_LO 0x4034 #define PCIE_MISC_RC_BAR2_CONFIG_HI 0x4038 #define PCIE_MISC_RC_BAR3_CONFIG_LO 0x403c +#define PCIE_MISC_MSI_BAR_CONFIG_LO 0x4044 +#define PCIE_MISC_MSI_BAR_CONFIG_HI 0x4048 +#define PCIE_MISC_MSI_DATA_CONFIG 0x404c #define PCIE_MISC_PCIE_CTRL 0x4064 #define PCIE_MISC_PCIE_STATUS 0x4068 +#define PCIE_MISC_REVISION 0x406c #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT 0x4070 #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI 0x4080 #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI 0x4084 #define PCIE_MISC_HARD_PCIE_HARD_DEBUG 0x4204 #define PCIE_INTR2_CPU_BASE 0x4300 +#define PCIE_MSI_INTR2_BASE 0x4500 /* * Broadcom STB PCIe Register Field shift and mask info. The names are from the @@ -87,6 +95,8 @@ #define PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_SHIFT 0x4 #define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK 0x40 #define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_SHIFT 0x6 +#define PCIE_MISC_REVISION_MAJMIN_MASK 0xffff +#define PCIE_MISC_REVISION_MAJMIN_SHIFT 0 #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK 0xfff00000 #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_SHIFT 0x14 #define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK 0xfff0 @@ -104,6 +114,11 @@ #define PCIE_RGR1_SW_INIT_1_PERST_SHIFT 0x0 #define BRCM_NUM_PCIE_OUT_WINS 0x4 +#define BRCM_INT_PCI_MSI_NR 32 +#define BRCM_PCIE_HW_REV_33 0x0303 + +#define BRCM_MSI_TARGET_ADDR_LT_4GB 0x0fffffffcULL +#define BRCM_MSI_TARGET_ADDR_GT_4GB 0xffffffffcULL #define BURST_SIZE_128 0 @@ -182,6 +197,33 @@ struct brcm_window { dma_addr_t size; }; +struct brcm_msi { + struct device *dev; + void __iomem *base; + struct device_node *np; + struct irq_domain *msi_domain; + struct irq_domain *inner_domain; + struct mutex lock; /* guards the alloc/free operations */ + u64 target_addr; + int irq; + + /* intr_base is the base pointer for interrupt status/set/clr regs */ + void __iomem *intr_base; + + /* intr_legacy_mask indicates how many bits are MSI interrupts */ + u32 intr_legacy_mask; + + /* + * intr_legacy_offset indicates bit position of MSI_01. It is + * to map the register bit position to a hwirq that starts at 0. + */ + u32 intr_legacy_offset; + + /* used indicates which MSI interrupts have been alloc'd */ + unsigned long used; + unsigned int rev; +}; + /* Internal PCIe Host Controller Information.*/ struct brcm_pcie { struct device *dev; @@ -191,7 +233,10 @@ struct brcm_pcie { struct device_node *np; bool ssc; int gen; + u64 msi_target_addr; struct brcm_window out_wins[BRCM_NUM_PCIE_OUT_WINS]; + struct brcm_msi *msi; + unsigned int rev; const int *reg_offsets; const int *reg_field_info; enum pcie_type type; @@ -433,6 +478,259 @@ static void brcm_pcie_set_outbound_win(struct brcm_pcie *pcie, LIMIT, tmp); } +static struct irq_chip brcm_msi_irq_chip = { + .name = "BRCM STB PCIe MSI", + .irq_ack = irq_chip_ack_parent, + .irq_mask = pci_msi_mask_irq, + .irq_unmask = pci_msi_unmask_irq, +}; + +static struct msi_domain_info brcm_msi_domain_info = { + /* TODO: Multi MSI is technically supported by the controller */ + .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | + MSI_FLAG_PCI_MSIX), + .chip = &brcm_msi_irq_chip, +}; + +static void brcm_pcie_msi_isr(struct irq_desc *desc) +{ + struct irq_chip *chip = irq_desc_get_chip(desc); + unsigned long status, virq; + struct brcm_msi *msi; + u32 mask, bit, hwirq; + struct device *dev; + + chained_irq_enter(chip, desc); + msi = irq_desc_get_handler_data(desc); + mask = msi->intr_legacy_mask; + dev = msi->dev; + + status = bcm_readl(msi->intr_base + STATUS) & mask; + for_each_set_bit(bit, &status, BRCM_INT_PCI_MSI_NR) { + + /* Account for legacy interrupt offset */ + hwirq = bit - msi->intr_legacy_offset; + virq = irq_find_mapping(msi->inner_domain, hwirq); + if (virq) + generic_handle_irq(virq); + else + dev_dbg(dev, "unexpected MSI\n"); + } + chained_irq_exit(chip, desc); +} + +static void brcm_msi_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) +{ + struct brcm_msi *msi = irq_data_get_irq_chip_data(data); + + msg->address_lo = lower_32_bits(msi->target_addr); + msg->address_hi = upper_32_bits(msi->target_addr); + msg->data = 0x6540 | data->hwirq; +} + +static int brcm_msi_set_affinity(struct irq_data *irq_data, + const struct cpumask *mask, bool force) +{ + return -EINVAL; +} + +static void brcm_msi_ack_irq(struct irq_data *data) +{ + struct brcm_msi *msi = irq_data_get_irq_chip_data(data); + u32 bit = data->hwirq + msi->intr_legacy_offset; + + bcm_writel(1 << bit, msi->intr_base + CLR); +} + + +static struct irq_chip brcm_msi_bottom_irq_chip = { + .name = "BRCM STB MSI", + .irq_compose_msi_msg = brcm_msi_compose_msi_msg, + .irq_set_affinity = brcm_msi_set_affinity, + .irq_ack = brcm_msi_ack_irq, +}; + +static int brcm_msi_alloc(struct brcm_msi *msi) +{ + int hwirq; + + mutex_lock(&msi->lock); + hwirq = bitmap_find_free_region(&msi->used, BRCM_INT_PCI_MSI_NR, 0); + if (hwirq >= 0) + hwirq = hwirq - msi->intr_legacy_offset; + mutex_unlock(&msi->lock); + + return hwirq; +} + +static void brcm_msi_free(struct brcm_msi *msi, unsigned long hwirq) +{ + mutex_lock(&msi->lock); + bitmap_release_region(&msi->used, hwirq + msi->intr_legacy_offset, 0); + mutex_unlock(&msi->lock); +} + +static int brcm_irq_domain_alloc(struct irq_domain *domain, unsigned int virq, + unsigned int nr_irqs, void *args) +{ + struct brcm_msi *msi = domain->host_data; + int hwirq; + + hwirq = brcm_msi_alloc(msi); + + if (hwirq < 0) + return hwirq; + + irq_domain_set_info(domain, virq, (irq_hw_number_t)hwirq, + &brcm_msi_bottom_irq_chip, domain->host_data, + handle_edge_irq, NULL, NULL); + return 0; +} + +static void brcm_irq_domain_free(struct irq_domain *domain, + unsigned int virq, unsigned int nr_irqs) +{ + struct irq_data *d = irq_domain_get_irq_data(domain, virq); + struct brcm_msi *msi = irq_data_get_irq_chip_data(d); + + brcm_msi_free(msi, d->hwirq); +} + +static const struct irq_domain_ops msi_domain_ops = { + .alloc = brcm_irq_domain_alloc, + .free = brcm_irq_domain_free, +}; + +static int brcm_allocate_domains(struct brcm_msi *msi) +{ + struct fwnode_handle *fwnode = of_node_to_fwnode(msi->np); + struct device *dev = msi->dev; + + msi->inner_domain = irq_domain_add_linear(NULL, BRCM_INT_PCI_MSI_NR, + &msi_domain_ops, msi); + if (!msi->inner_domain) { + dev_err(dev, "failed to create IRQ domain\n"); + return -ENOMEM; + } + + msi->msi_domain = pci_msi_create_irq_domain(fwnode, + &brcm_msi_domain_info, + msi->inner_domain); + if (!msi->msi_domain) { + dev_err(dev, "failed to create MSI domain\n"); + irq_domain_remove(msi->inner_domain); + return -ENOMEM; + } + + return 0; +} + +static void brcm_free_domains(struct brcm_msi *msi) +{ + irq_domain_remove(msi->msi_domain); + irq_domain_remove(msi->inner_domain); +} + +static void brcm_msi_remove(struct brcm_pcie *pcie) +{ + struct brcm_msi *msi = pcie->msi; + + if (!msi) + return; + irq_set_chained_handler(msi->irq, NULL); + irq_set_handler_data(msi->irq, NULL); + brcm_free_domains(msi); +} + +static void brcm_msi_set_regs(struct brcm_msi *msi) +{ + u32 data_val, msi_lo, msi_hi; + + if (msi->rev >= BRCM_PCIE_HW_REV_33) { + /* + * ffe0 -- least sig 5 bits are 0 indicating 32 msgs + * 6540 -- this is our arbitrary unique data value + */ + data_val = 0xffe06540; + } else { + /* + * fff8 -- least sig 3 bits are 0 indicating 8 msgs + * 6540 -- this is our arbitrary unique data value + */ + data_val = 0xfff86540; + } + + /* Unmaks MSIs */ + bcm_writel(0xffffffff & msi->intr_legacy_mask, + msi->intr_base + MASK_CLR); + + msi_lo = lower_32_bits(msi->target_addr); + msi_hi = upper_32_bits(msi->target_addr); + /* + * The 0 bit of PCIE_MISC_MSI_BAR_CONFIG_LO is repurposed to MSI + * enable, which we set to 1. + */ + bcm_writel(msi_lo | 1, msi->base + PCIE_MISC_MSI_BAR_CONFIG_LO); + bcm_writel(msi_hi, msi->base + PCIE_MISC_MSI_BAR_CONFIG_HI); + bcm_writel(data_val, msi->base + PCIE_MISC_MSI_DATA_CONFIG); +} + +static int brcm_pcie_enable_msi(struct brcm_pcie *pcie) +{ + struct brcm_msi *msi; + int irq, ret; + struct device *dev = pcie->dev; + + irq = irq_of_parse_and_map(dev->of_node, 1); + if (irq <= 0) { + dev_err(dev, "cannot map msi intr\n"); + return -ENODEV; + } + + msi = devm_kzalloc(dev, sizeof(struct brcm_msi), GFP_KERNEL); + if (!msi) + return -ENOMEM; + + msi->dev = dev; + msi->base = pcie->base; + msi->rev = pcie->rev; + msi->np = pcie->np; + msi->target_addr = pcie->msi_target_addr; + msi->irq = irq; + + ret = brcm_allocate_domains(msi); + if (ret) + return ret; + + irq_set_chained_handler_and_data(msi->irq, brcm_pcie_msi_isr, msi); + + if (msi->rev >= BRCM_PCIE_HW_REV_33) { + msi->intr_base = msi->base + PCIE_MSI_INTR2_BASE; + /* + * This version of PCIe hw has only 32 intr bits + * starting at bit position 0. + */ + msi->intr_legacy_mask = 0xffffffff; + msi->intr_legacy_offset = 0x0; + msi->used = 0x0; + + } else { + msi->intr_base = msi->base + PCIE_INTR2_CPU_BASE; + /* + * This version of PCIe hw has only 8 intr bits starting + * at bit position 24. + */ + msi->intr_legacy_mask = 0xff000000; + msi->intr_legacy_offset = 24; + msi->used = 0x00ffffff; + } + + brcm_msi_set_regs(msi); + pcie->msi = msi; + + return 0; +} + /* Configuration space read/write support */ static int brcm_pcie_cfg_index(int busnr, int devfn, int reg) { @@ -584,6 +882,10 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie) /* Wait for SerDes to be stable */ usleep_range(100, 200); + /* Grab the PCIe hw revision number */ + tmp = bcm_readl(base + PCIE_MISC_REVISION); + pcie->rev = EXTRACT_FIELD(tmp, PCIE_MISC_REVISION, MAJMIN); + /* Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN */ tmp = INSERT_FIELD(0, PCIE_MISC_MISC_CTRL, SCB_ACCESS_EN, 1); tmp = INSERT_FIELD(tmp, PCIE_MISC_MISC_CTRL, CFG_READ_UR_MODE, 1); @@ -596,6 +898,18 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie) if (ret) return ret; + /* + * We ideally want the MSI target address to be located in the 32bit + * addressable memory area. Some devices might depend on it. This is + * possible either when the inbound window is located above the lower + * 4GB or when the inbound and outbound areas fit in the lower 4GB of + * memory. + */ + if (rc_bar2_offset >= SZ_4G || (rc_bar2_size + rc_bar2_offset) <= SZ_4G) + pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_LT_4GB; + else + pcie->msi_target_addr = BRCM_MSI_TARGET_ADDR_GT_4GB; + tmp = lower_32_bits(rc_bar2_offset); tmp = INSERT_FIELD(tmp, PCIE_MISC_RC_BAR2_CONFIG_LO, SIZE, brcm_pcie_encode_ibar_size(rc_bar2_size)); @@ -734,6 +1048,7 @@ static void brcm_pcie_turn_off(struct brcm_pcie *pcie) static void __brcm_pcie_remove(struct brcm_pcie *pcie) { + brcm_msi_remove(pcie); brcm_pcie_turn_off(pcie); clk_disable_unprepare(pcie->clk); clk_put(pcie->clk); @@ -758,7 +1073,7 @@ MODULE_DEVICE_TABLE(of, brcm_pcie_match); static int brcm_pcie_probe(struct platform_device *pdev) { - struct device_node *np = pdev->dev.of_node; + struct device_node *np = pdev->dev.of_node, *msi_np; const struct pcie_cfg_data *data; struct pci_host_bridge *bridge; struct brcm_pcie *pcie; @@ -813,6 +1128,15 @@ static int brcm_pcie_probe(struct platform_device *pdev) if (ret) goto fail; + msi_np = of_parse_phandle(pcie->np, "msi-parent", 0); + if (pci_msi_enabled() && msi_np == pcie->np) { + ret = brcm_pcie_enable_msi(pcie); + if (ret) { + dev_err(pcie->dev, "probe of internal MSI failed"); + goto fail; + } + } + bridge->dev.parent = &pdev->dev; bridge->busnr = 0; bridge->ops = &brcm_pcie_ops;