Re: [PATCH v3 01/14] PCI: tegra: Convert to MSI domains
Hi Marc,
On 20/04/2021 09:39, Marc Zyngier wrote:
...
> The following should hopefully cure it (compile tested only). Please
> let me know.
>
> M.
>
> diff --git a/drivers/pci/controller/pci-tegra.c
> b/drivers/pci/controller/pci-tegra.c
> index eaba7b2fab4a..507b23d43ad1 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -1802,13 +1802,19 @@ static void tegra_pcie_enable_msi(struct tegra_pcie
> *pcie)
> {
> const struct tegra_pcie_soc *soc = pcie->soc;
> struct tegra_msi *msi = >msi;
> - u32 reg;
> + u32 reg, msi_state[INT_PCI_MSI_NR / 32];
> + int i;
>
> afi_writel(pcie, msi->phys >> soc->msi_base_shift, AFI_MSI_FPCI_BAR_ST);
> afi_writel(pcie, msi->phys, AFI_MSI_AXI_BAR_ST);
> /* this register is in 4K increments */
> afi_writel(pcie, 1, AFI_MSI_BAR_SZ);
>
> + /* Restore the MSI allocation state */
> + bitmap_to_arr32(msi_state, msi->used, INT_PCI_MSI_NR);
> + for (i = 0; i < ARRAY_SIZE(msi_state); i++)
> + afi_writel(pcie, msi_state[i], AFI_MSI_EN_VEC(i));
> +
> /* and unmask the MSI interrupt */
> reg = afi_readl(pcie, AFI_INTR_MASK);
> reg |= AFI_INTR_MASK_MSI_MASK;
>
Thanks, that does fix it indeed!
Tested-by: Jon Hunter
Cheers
Jon
--
nvpublic
Re: [PATCH v3 01/14] PCI: tegra: Convert to MSI domains
On 19/04/2021 20:19, Jon Hunter wrote:
> Hi Marc,
>
> On 30/03/2021 16:11, Marc Zyngier wrote:
>> In anticipation of the removal of the msi_controller structure, convert
>> the Tegra host controller driver to MSI domains.
>>
>> We end-up with the usual two domain structure, the top one being a
>> generic PCI/MSI domain, the bottom one being Tegra-specific and handling
>> the actual HW interrupt allocation.
>>
>> While at it, convert the normal interrupt handler to a chained handler,
>> handle the controller's MSI IRQ edge triggered, support multiple MSIs
>> per device and use the AFI_MSI_EN_VEC* registers to provide MSI masking.
>>
>> Acked-by: Bjorn Helgaas
>> [tred...@nvidia.com: fix, clean up and address TODOs from Marc's draft]
>> Signed-off-by: Thierry Reding
>> Signed-off-by: Marc Zyngier
>
>
> This change is breaking a suspend test that we are running on Tegra124
> Jetson-TK1. The Tegra124 Jetson TK1 uses a PCI based ethernet device ...
>
> $ lspci
> 00:02.0 PCI bridge: NVIDIA Corporation TegraK1 PCIe x1 Bridge (rev a1)
> 01:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
> RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 0c)
>
> After resuming from suspend, networking is no longer working. The reason
> why this breaks our suspend test is because that setup is using NFS for
> the rootfs. I am looking into it, but if anyone has any thoughts please
> let me know.
So the following does appear to fix it ...
diff --git a/drivers/pci/controller/pci-tegra.c
b/drivers/pci/controller/pci-tegra.c
index eaba7b2fab4a..558f02e0693d 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -1802,13 +1802,17 @@ static void tegra_pcie_enable_msi(struct
tegra_pcie *pcie)
{
const struct tegra_pcie_soc *soc = pcie->soc;
struct tegra_msi *msi = >msi;
- u32 reg;
+ u32 i, reg;
afi_writel(pcie, msi->phys >> soc->msi_base_shift,
AFI_MSI_FPCI_BAR_ST);
afi_writel(pcie, msi->phys, AFI_MSI_AXI_BAR_ST);
/* this register is in 4K increments */
afi_writel(pcie, 1, AFI_MSI_BAR_SZ);
+ /* enable all MSI vectors */
+ for (i = 0; i < 8; i++)
+ afi_writel(pcie, 0x, AFI_MSI_EN_VEC(i));
+
/* and unmask the MSI interrupt */
reg = afi_readl(pcie, AFI_INTR_MASK);
reg |= AFI_INTR_MASK_MSI_MASK;
@@ -1837,13 +1841,17 @@ static void tegra_pcie_msi_teardown(struct
tegra_pcie *pcie)
static int tegra_pcie_disable_msi(struct tegra_pcie *pcie)
{
- u32 value;
+ u32 i, value;
/* mask the MSI interrupt */
value = afi_readl(pcie, AFI_INTR_MASK);
value &= ~AFI_INTR_MASK_MSI_MASK;
afi_writel(pcie, value, AFI_INTR_MASK);
+ /* disable all MSI vectors */
+ for (i = 0; i < 8; i++)
+ afi_writel(pcie, 0, AFI_MSI_EN_VEC(i));
+
return 0;
}
Any reason why that code was removed?
Thanks
Jon
--
nvpublic
Re: [PATCH v3 01/14] PCI: tegra: Convert to MSI domains
Hi Marc, On 30/03/2021 16:11, Marc Zyngier wrote: > In anticipation of the removal of the msi_controller structure, convert > the Tegra host controller driver to MSI domains. > > We end-up with the usual two domain structure, the top one being a > generic PCI/MSI domain, the bottom one being Tegra-specific and handling > the actual HW interrupt allocation. > > While at it, convert the normal interrupt handler to a chained handler, > handle the controller's MSI IRQ edge triggered, support multiple MSIs > per device and use the AFI_MSI_EN_VEC* registers to provide MSI masking. > > Acked-by: Bjorn Helgaas > [tred...@nvidia.com: fix, clean up and address TODOs from Marc's draft] > Signed-off-by: Thierry Reding > Signed-off-by: Marc Zyngier This change is breaking a suspend test that we are running on Tegra124 Jetson-TK1. The Tegra124 Jetson TK1 uses a PCI based ethernet device ... $ lspci 00:02.0 PCI bridge: NVIDIA Corporation TegraK1 PCIe x1 Bridge (rev a1) 01:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller (rev 0c) After resuming from suspend, networking is no longer working. The reason why this breaks our suspend test is because that setup is using NFS for the rootfs. I am looking into it, but if anyone has any thoughts please let me know. Jon -- nvpublic
[PATCH v3 01/14] PCI: tegra: Convert to MSI domains
In anticipation of the removal of the msi_controller structure, convert
the Tegra host controller driver to MSI domains.
We end-up with the usual two domain structure, the top one being a
generic PCI/MSI domain, the bottom one being Tegra-specific and handling
the actual HW interrupt allocation.
While at it, convert the normal interrupt handler to a chained handler,
handle the controller's MSI IRQ edge triggered, support multiple MSIs
per device and use the AFI_MSI_EN_VEC* registers to provide MSI masking.
Acked-by: Bjorn Helgaas
[tred...@nvidia.com: fix, clean up and address TODOs from Marc's draft]
Signed-off-by: Thierry Reding
Signed-off-by: Marc Zyngier
---
drivers/pci/controller/Kconfig | 1 -
drivers/pci/controller/pci-tegra.c | 343 -
2 files changed, 185 insertions(+), 159 deletions(-)
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 5aa8977d7b0f..be8f9ff512a0 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -41,7 +41,6 @@ config PCI_TEGRA
bool "NVIDIA Tegra PCIe controller"
depends on ARCH_TEGRA || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
- select PCI_MSI_ARCH_FALLBACKS
help
Say Y here if you want support for the PCIe host controller found
on NVIDIA Tegra SoCs.
diff --git a/drivers/pci/controller/pci-tegra.c
b/drivers/pci/controller/pci-tegra.c
index 8fcabed7c6a6..eaba7b2fab4a 100644
--- a/drivers/pci/controller/pci-tegra.c
+++ b/drivers/pci/controller/pci-tegra.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -78,23 +79,8 @@
#define AFI_MSI_FPCI_BAR_ST0x64
#define AFI_MSI_AXI_BAR_ST 0x68
-#define AFI_MSI_VEC0 0x6c
-#define AFI_MSI_VEC1 0x70
-#define AFI_MSI_VEC2 0x74
-#define AFI_MSI_VEC3 0x78
-#define AFI_MSI_VEC4 0x7c
-#define AFI_MSI_VEC5 0x80
-#define AFI_MSI_VEC6 0x84
-#define AFI_MSI_VEC7 0x88
-
-#define AFI_MSI_EN_VEC00x8c
-#define AFI_MSI_EN_VEC10x90
-#define AFI_MSI_EN_VEC20x94
-#define AFI_MSI_EN_VEC30x98
-#define AFI_MSI_EN_VEC40x9c
-#define AFI_MSI_EN_VEC50xa0
-#define AFI_MSI_EN_VEC60xa4
-#define AFI_MSI_EN_VEC70xa8
+#define AFI_MSI_VEC(x) (0x6c + ((x) * 4))
+#define AFI_MSI_EN_VEC(x) (0x8c + ((x) * 4))
#define AFI_CONFIGURATION 0xac
#define AFI_CONFIGURATION_EN_FPCI (1 << 0)
@@ -280,10 +266,10 @@
#define LINK_RETRAIN_TIMEOUT 10 /* in usec */
struct tegra_msi {
- struct msi_controller chip;
DECLARE_BITMAP(used, INT_PCI_MSI_NR);
struct irq_domain *domain;
- struct mutex lock;
+ struct mutex map_lock;
+ spinlock_t mask_lock;
void *virt;
dma_addr_t phys;
int irq;
@@ -333,11 +319,6 @@ struct tegra_pcie_soc {
} ectl;
};
-static inline struct tegra_msi *to_tegra_msi(struct msi_controller *chip)
-{
- return container_of(chip, struct tegra_msi, chip);
-}
-
struct tegra_pcie {
struct device *dev;
@@ -372,6 +353,11 @@ struct tegra_pcie {
struct dentry *debugfs;
};
+static inline struct tegra_pcie *msi_to_pcie(struct tegra_msi *msi)
+{
+ return container_of(msi, struct tegra_pcie, msi);
+}
+
struct tegra_pcie_port {
struct tegra_pcie *pcie;
struct device_node *np;
@@ -1432,7 +1418,6 @@ static void tegra_pcie_phys_put(struct tegra_pcie *pcie)
}
}
-
static int tegra_pcie_get_resources(struct tegra_pcie *pcie)
{
struct device *dev = pcie->dev;
@@ -1509,6 +1494,7 @@ static int tegra_pcie_get_resources(struct tegra_pcie
*pcie)
phys_put:
if (soc->program_uphy)
tegra_pcie_phys_put(pcie);
+
return err;
}
@@ -1551,161 +1537,227 @@ static void tegra_pcie_pme_turnoff(struct
tegra_pcie_port *port)
afi_writel(pcie, val, AFI_PCIE_PME);
}
-static int tegra_msi_alloc(struct tegra_msi *chip)
-{
- int msi;
-
- mutex_lock(>lock);
-
- msi = find_first_zero_bit(chip->used, INT_PCI_MSI_NR);
- if (msi < INT_PCI_MSI_NR)
- set_bit(msi, chip->used);
- else
- msi = -ENOSPC;
-
- mutex_unlock(>lock);
-
- return msi;
-}
-
-static void tegra_msi_free(struct tegra_msi *chip, unsigned long irq)
+static void tegra_pcie_msi_irq(struct irq_desc *desc)
{
- struct device *dev = chip->chip.dev;
-
- mutex_lock(>lock);
-
- if (!test_bit(irq, chip->used))
- dev_err(dev, "trying to free unused MSI#%lu\n", irq);
- else
- clear_bit(irq, chip->used);
-
- mutex_unlock(>lock);
-}
-
-static irqreturn_t tegra_pcie_msi_irq(int irq, void *data)
-{
- struct tegra_pcie *pcie = data;
- struct device *dev = pcie->dev;
+ struct
