[PATCH v2 1/3] PCI: tegra194: Fix broken tegra_pcie_ep_raise_msi_irq()

[PATCH v2 1/3] PCI: tegra194: Fix broken tegra_pcie_ep_raise_msi_irq()

[Niklas Cassel]

The pci_epc_raise_irq() supplies a MSI or MSI-X interrupt number in range
(1-N), see kdoc for pci_epc_raise_irq().

Thus, for MSI pci_epc_raise_irq() will supply interrupt number 1-32.

Convert the interrupt number to an MSI vector.

With this, the PCI endpoint kselftest test case MSI_TEST passes.

Also, set msi_capable to true, as the driver obviously supports MSI.
This helps pci_endpoint_test to use the optimal IRQ type when using
PCITEST_IRQ_TYPE_AUTO.

Cc: stable@vger.kernel.org
Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pcie-tegra194.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 7c295ec6f0f16..63d310e5335f4 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1969,10 +1969,10 @@ static int tegra_pcie_ep_raise_intx_irq(struct tegra_pcie_dw *pcie, u16 irq)
 
 static int tegra_pcie_ep_raise_msi_irq(struct tegra_pcie_dw *pcie, u16 irq)
 {
-	if (unlikely(irq > 31))
+	if (unlikely(irq > 32))
 		return -EINVAL;
 
-	appl_writel(pcie, BIT(irq), APPL_MSI_CTRL_1);
+	appl_writel(pcie, BIT(irq - 1), APPL_MSI_CTRL_1);
 
 	return 0;
 }
@@ -2012,6 +2012,7 @@ static int tegra_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
 
 static const struct pci_epc_features tegra_pcie_epc_features = {
 	.linkup_notifier = true,
+	.msi_capable = true,
 	.bar[BAR_0] = { .type = BAR_FIXED, .fixed_size = SZ_1M,
 			.only_64bit = true, },
 	.bar[BAR_1] = { .type = BAR_RESERVED, },
-- 
2.51.0

[PATCH v2 2/3] PCI: tegra194: Reset BARs when running in PCIe endpoint mode

[Niklas Cassel]

Tegra already defines all BARs expect for BAR0 as BAR_RESERVED.
This is sufficient for pci-epf-test to not allocate backing memory and to
not call set_bar() for those BARs. However, marking a BAR as BAR_RESERVED
does not mean that the BAR get disabled.

The host side driver, pci_endpoint_test, simply does an ioremap for all
enabled BARs, and will run tests against all enabled BARs. (I.e. it will
run tests also against the BARs marked as BAR_RESERVED.)

After running the BARs tests (which will write to all enabled BARs), the
inbound address translation is broken.
This is because the tegra controller exposes the ATU Port Logic Structure
in BAR4. So when BAR4 is written, the inbound address translation settings
get overwritten.

To avoid this, implement the dw_pcie_ep_ops .init() callback and start off
by disabling all BARs (pci-epf-test will later enable/configure BARs that
are not defined as BAR_RESERVED).

This matches the behavior of other PCIe endpoint drivers:
dra7xx, imx6, layerscape-ep, artpec6, dw-rockchip, qcom-ep, rcar-gen4, and
uniphier-ep.

With this, the PCI endpoint kselftest test case CONSECUTIVE_BAR_TEST
(which was specifically made to detect address translation issues) passes.

Cc: stable@vger.kernel.org
Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pcie-tegra194.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 63d310e5335f4..7eb48cc13648e 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1955,6 +1955,15 @@ static irqreturn_t tegra_pcie_ep_pex_rst_irq(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+static void tegra_pcie_ep_init(struct dw_pcie_ep *ep)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	enum pci_barno bar;
+
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
+		dw_pcie_ep_reset_bar(pci, bar);
+};
+
 static int tegra_pcie_ep_raise_intx_irq(struct tegra_pcie_dw *pcie, u16 irq)
 {
 	/* Tegra194 supports only INTA */
@@ -2030,6 +2039,7 @@ tegra_pcie_ep_get_features(struct dw_pcie_ep *ep)
 }
 
 static const struct dw_pcie_ep_ops pcie_ep_ops = {
+	.init = tegra_pcie_ep_init,
 	.raise_irq = tegra_pcie_ep_raise_irq,
 	.get_features = tegra_pcie_ep_get_features,
 };
-- 
2.51.0

[PATCH v2 3/3] PCI: tegra194: Handle errors in BPMP response

[Niklas Cassel]

From: Vidya Sagar <vidyas@nvidia.com>

The return value from tegra_bpmp_transfer() indicates the success or
failure of the IPC transaction with BPMP. If the transaction
succeeded, we also need to check the actual command's result code.

If we don't have error handling for tegra_bpmp_transfer(), we will
set the pcie->ep_state to EP_STATE_ENABLED (even though the
tegra_bpmp_transfer() command failed). Thus, the pcie->ep_state will
get out of sync with reality, and any further PERST# assert + deassert
will be a no-op (will not trigger the hardware initialization sequence).

This is because pex_ep_event_pex_rst_deassert() checks the current
pcie->ep_state, and does nothing if the current state is already
EP_STATE_ENABLED.

Thus, it is important to have error handling for tegra_bpmp_transfer(),
such that the pcie->ep_state can not get out of sync with reality, so
that we will try to initialize the hardware not only during the first
PERST# assert + deassert, but also during any succeeding PERST# assert +
deassert.

One example where this fix is needed is when using a rock5b as host.
During the initial PERST# assert + deassert (triggered by the bootloader
on the rock5b) pex_ep_event_pex_rst_deassert() will get called, but for
some unknown reason, the tegra_bpmp_transfer() call to initialize the PHY
fails. Once Linux has been loaded on the rock5b, the PCIe driver will once
again assert + deassert PERST#. However, without tegra_bpmp_transfer()
error handling, this second PERST# assert + deassert will not trigger the
hardware initialization sequence.

With tegra_bpmp_transfer() error handling, the second PERST# assert +
deassert will once again trigger the hardware to be initialized and this
time the tegra_bpmp_transfer() succeeds.

Cc: stable@vger.kernel.org
Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
[cassel: improve commit log]
Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pcie-tegra194.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 7eb48cc13648e..c4265b3f72048 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1223,6 +1223,7 @@ static int tegra_pcie_bpmp_set_ctrl_state(struct tegra_pcie_dw *pcie,
 	struct mrq_uphy_response resp;
 	struct tegra_bpmp_message msg;
 	struct mrq_uphy_request req;
+	int err;
 
 	/*
 	 * Controller-5 doesn't need to have its state set by BPMP-FW in
@@ -1245,7 +1246,13 @@ static int tegra_pcie_bpmp_set_ctrl_state(struct tegra_pcie_dw *pcie,
 	msg.rx.data = &resp;
 	msg.rx.size = sizeof(resp);
 
-	return tegra_bpmp_transfer(pcie->bpmp, &msg);
+	err = tegra_bpmp_transfer(pcie->bpmp, &msg);
+	if (err)
+		return err;
+	if (msg.rx.ret)
+		return -EINVAL;
+
+	return 0;
 }
 
 static int tegra_pcie_bpmp_set_pll_state(struct tegra_pcie_dw *pcie,
@@ -1254,6 +1261,7 @@ static int tegra_pcie_bpmp_set_pll_state(struct tegra_pcie_dw *pcie,
 	struct mrq_uphy_response resp;
 	struct tegra_bpmp_message msg;
 	struct mrq_uphy_request req;
+	int err;
 
 	memset(&req, 0, sizeof(req));
 	memset(&resp, 0, sizeof(resp));
@@ -1273,7 +1281,13 @@ static int tegra_pcie_bpmp_set_pll_state(struct tegra_pcie_dw *pcie,
 	msg.rx.data = &resp;
 	msg.rx.size = sizeof(resp);
 
-	return tegra_bpmp_transfer(pcie->bpmp, &msg);
+	err = tegra_bpmp_transfer(pcie->bpmp, &msg);
+	if (err)
+		return err;
+	if (msg.rx.ret)
+		return -EINVAL;
+
+	return 0;
 }
 
 static void tegra_pcie_downstream_dev_to_D0(struct tegra_pcie_dw *pcie)
-- 
2.51.0

Re: [PATCH v2 1/3] PCI: tegra194: Fix broken tegra_pcie_ep_raise_msi_irq()

[Manivannan Sadhasivam]

On Mon, Sep 22, 2025 at 04:08:24PM +0200, Niklas Cassel wrote:
> The pci_epc_raise_irq() supplies a MSI or MSI-X interrupt number in range
> (1-N), see kdoc for pci_epc_raise_irq().
> 
> Thus, for MSI pci_epc_raise_irq() will supply interrupt number 1-32.
> 
> Convert the interrupt number to an MSI vector.
> 
> With this, the PCI endpoint kselftest test case MSI_TEST passes.
> 
> Also, set msi_capable to true, as the driver obviously supports MSI.
> This helps pci_endpoint_test to use the optimal IRQ type when using
> PCITEST_IRQ_TYPE_AUTO.
> 
> Cc: stable@vger.kernel.org
> Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-tegra194.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 7c295ec6f0f16..63d310e5335f4 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1969,10 +1969,10 @@ static int tegra_pcie_ep_raise_intx_irq(struct tegra_pcie_dw *pcie, u16 irq)
>  
>  static int tegra_pcie_ep_raise_msi_irq(struct tegra_pcie_dw *pcie, u16 irq)
>  {
> -	if (unlikely(irq > 31))
> +	if (unlikely(irq > 32))
>  		return -EINVAL;
>  
> -	appl_writel(pcie, BIT(irq), APPL_MSI_CTRL_1);
> +	appl_writel(pcie, BIT(irq - 1), APPL_MSI_CTRL_1);
>  
>  	return 0;
>  }
> @@ -2012,6 +2012,7 @@ static int tegra_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>  
>  static const struct pci_epc_features tegra_pcie_epc_features = {
>  	.linkup_notifier = true,
> +	.msi_capable = true,

This change is unrelated to the above tegra_pcie_ep_raise_msi_irq() fix. So this
change should be in a separate patch.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 1/3] PCI: tegra194: Fix broken tegra_pcie_ep_raise_msi_irq()

[Manivannan Sadhasivam]

On Wed, Sep 24, 2025 at 09:24:09PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Sep 22, 2025 at 04:08:24PM +0200, Niklas Cassel wrote:
> > The pci_epc_raise_irq() supplies a MSI or MSI-X interrupt number in range
> > (1-N), see kdoc for pci_epc_raise_irq().
> > 
> > Thus, for MSI pci_epc_raise_irq() will supply interrupt number 1-32.
> > 
> > Convert the interrupt number to an MSI vector.
> > 
> > With this, the PCI endpoint kselftest test case MSI_TEST passes.
> > 
> > Also, set msi_capable to true, as the driver obviously supports MSI.
> > This helps pci_endpoint_test to use the optimal IRQ type when using
> > PCITEST_IRQ_TYPE_AUTO.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
> > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-tegra194.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> > index 7c295ec6f0f16..63d310e5335f4 100644
> > --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> > @@ -1969,10 +1969,10 @@ static int tegra_pcie_ep_raise_intx_irq(struct tegra_pcie_dw *pcie, u16 irq)
> >  
> >  static int tegra_pcie_ep_raise_msi_irq(struct tegra_pcie_dw *pcie, u16 irq)
> >  {
> > -	if (unlikely(irq > 31))
> > +	if (unlikely(irq > 32))
> >  		return -EINVAL;
> >  
> > -	appl_writel(pcie, BIT(irq), APPL_MSI_CTRL_1);
> > +	appl_writel(pcie, BIT(irq - 1), APPL_MSI_CTRL_1);
> >  
> >  	return 0;
> >  }
> > @@ -2012,6 +2012,7 @@ static int tegra_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> >  
> >  static const struct pci_epc_features tegra_pcie_epc_features = {
> >  	.linkup_notifier = true,
> > +	.msi_capable = true,
> 
> This change is unrelated to the above tegra_pcie_ep_raise_msi_irq() fix. So this
> change should be in a separate patch.
> 

I did the split and applied the series, thanks!

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 1/3] PCI: tegra194: Fix broken tegra_pcie_ep_raise_msi_irq()

[Niklas Cassel]

On Wed, Sep 24, 2025 at 09:58:20PM +0530, Manivannan Sadhasivam wrote:
> > > @@ -2012,6 +2012,7 @@ static int tegra_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > >  
> > >  static const struct pci_epc_features tegra_pcie_epc_features = {
> > >  	.linkup_notifier = true,
> > > +	.msi_capable = true,
> > 
> > This change is unrelated to the above tegra_pcie_ep_raise_msi_irq() fix. So this
> > change should be in a separate patch.
> > 
> 
> I did the split and applied the series, thanks!

Thank you Mani!


Kind regards,
Niklas

Re: [PATCH v2 2/3] PCI: tegra194: Reset BARs when running in PCIe endpoint mode

[Manikanta Maddireddy]

Hi Niklas,

Tegra PCIe exposes only DMA register over BAR4, not iATU.
So, the issue described in this commit message is not applicable.
This patch is disabling BAR2 and BAR4, after enumeration I see
only BAR0. I think we should revert this patch.
Please share your inputs on this.

Thanks,
Manikanta


On 22/09/25 7:38 pm, Niklas Cassel wrote:
> Tegra already defines all BARs expect for BAR0 as BAR_RESERVED.
> This is sufficient for pci-epf-test to not allocate backing memory and to
> not call set_bar() for those BARs. However, marking a BAR as BAR_RESERVED
> does not mean that the BAR get disabled.
>
> The host side driver, pci_endpoint_test, simply does an ioremap for all
> enabled BARs, and will run tests against all enabled BARs. (I.e. it will
> run tests also against the BARs marked as BAR_RESERVED.)
>
> After running the BARs tests (which will write to all enabled BARs), the
> inbound address translation is broken.
> This is because the tegra controller exposes the ATU Port Logic Structure
> in BAR4. So when BAR4 is written, the inbound address translation settings
> get overwritten.
>
> To avoid this, implement the dw_pcie_ep_ops .init() callback and start off
> by disabling all BARs (pci-epf-test will later enable/configure BARs that
> are not defined as BAR_RESERVED).
>
> This matches the behavior of other PCIe endpoint drivers:
> dra7xx, imx6, layerscape-ep, artpec6, dw-rockchip, qcom-ep, rcar-gen4, and
> uniphier-ep.
>
> With this, the PCI endpoint kselftest test case CONSECUTIVE_BAR_TEST
> (which was specifically made to detect address translation issues) passes.
>
> Cc: stable@vger.kernel.org
> Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>   drivers/pci/controller/dwc/pcie-tegra194.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 63d310e5335f4..7eb48cc13648e 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1955,6 +1955,15 @@ static irqreturn_t tegra_pcie_ep_pex_rst_irq(int irq, void *arg)
>   	return IRQ_HANDLED;
>   }
>   
> +static void tegra_pcie_ep_init(struct dw_pcie_ep *ep)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	enum pci_barno bar;
> +
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
> +		dw_pcie_ep_reset_bar(pci, bar);
> +};
> +
>   static int tegra_pcie_ep_raise_intx_irq(struct tegra_pcie_dw *pcie, u16 irq)
>   {
>   	/* Tegra194 supports only INTA */
> @@ -2030,6 +2039,7 @@ tegra_pcie_ep_get_features(struct dw_pcie_ep *ep)
>   }
>   
>   static const struct dw_pcie_ep_ops pcie_ep_ops = {
> +	.init = tegra_pcie_ep_init,
>   	.raise_irq = tegra_pcie_ep_raise_irq,
>   	.get_features = tegra_pcie_ep_get_features,
>   };

Re: [PATCH v2 2/3] PCI: tegra194: Reset BARs when running in PCIe endpoint mode

[Manikanta Maddireddy]

Hi Niklas,

Tegra PCIe controller exposes only DMA registers over BAR4, iATU is never

exposed over BAR. So, this bug is not applicable for Tegra PCIe.

However, because of this change, BAR2 & BAR4 is never enabled and

I don't see BAR2 and BAR4 when host enumerates the Endpoint controller.

I think we should revert this patch, let me know your opinion on this.


Thanks,

Manikanta

On 22/09/25 7:38 pm, Niklas Cassel wrote:
> Tegra already defines all BARs expect for BAR0 as BAR_RESERVED.
> This is sufficient for pci-epf-test to not allocate backing memory and to
> not call set_bar() for those BARs. However, marking a BAR as BAR_RESERVED
> does not mean that the BAR get disabled.
>
> The host side driver, pci_endpoint_test, simply does an ioremap for all
> enabled BARs, and will run tests against all enabled BARs. (I.e. it will
> run tests also against the BARs marked as BAR_RESERVED.)
>
> After running the BARs tests (which will write to all enabled BARs), the
> inbound address translation is broken.
> This is because the tegra controller exposes the ATU Port Logic Structure
> in BAR4. So when BAR4 is written, the inbound address translation settings
> get overwritten.
>
> To avoid this, implement the dw_pcie_ep_ops .init() callback and start off
> by disabling all BARs (pci-epf-test will later enable/configure BARs that
> are not defined as BAR_RESERVED).
>
> This matches the behavior of other PCIe endpoint drivers:
> dra7xx, imx6, layerscape-ep, artpec6, dw-rockchip, qcom-ep, rcar-gen4, and
> uniphier-ep.
>
> With this, the PCI endpoint kselftest test case CONSECUTIVE_BAR_TEST
> (which was specifically made to detect address translation issues) passes.
>
> Cc: stable@vger.kernel.org
> Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>   drivers/pci/controller/dwc/pcie-tegra194.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 63d310e5335f4..7eb48cc13648e 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1955,6 +1955,15 @@ static irqreturn_t tegra_pcie_ep_pex_rst_irq(int irq, void *arg)
>   	return IRQ_HANDLED;
>   }
>   
> +static void tegra_pcie_ep_init(struct dw_pcie_ep *ep)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	enum pci_barno bar;
> +
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++)
> +		dw_pcie_ep_reset_bar(pci, bar);
> +};
> +
>   static int tegra_pcie_ep_raise_intx_irq(struct tegra_pcie_dw *pcie, u16 irq)
>   {
>   	/* Tegra194 supports only INTA */
> @@ -2030,6 +2039,7 @@ tegra_pcie_ep_get_features(struct dw_pcie_ep *ep)
>   }
>   
>   static const struct dw_pcie_ep_ops pcie_ep_ops = {
> +	.init = tegra_pcie_ep_init,
>   	.raise_irq = tegra_pcie_ep_raise_irq,
>   	.get_features = tegra_pcie_ep_get_features,
>   };

Re: [PATCH v2 2/3] PCI: tegra194: Reset BARs when running in PCIe endpoint mode

[Niklas Cassel]

On Sun, Feb 08, 2026 at 11:41:42PM +0530, Manikanta Maddireddy wrote:
> Hi Niklas,
> 
> Tegra PCIe exposes only DMA register over BAR4, not iATU.
> So, the issue described in this commit message is not applicable.
> This patch is disabling BAR2 and BAR4, after enumeration I see
> only BAR0. I think we should revert this patch.
> Please share your inputs on this.

If you look at this commit, it only disables all BARs by default,
which brings the tegra driver in-line with all other DWC based
endpoint drivers: dra7xx, imx6, layerscape-ep, artpec6, dw-rockchip,
qcom-ep, rcar-gen4, and uniphier-ep.

A PCI endpoint function (EPF) driver will still be able to enable a
BAR that was disabled in .init().
However, an EPF driver will not be able to use/enable a reserved BAR.

Look at e.g. the code in pci-epf-test. It will not allocate backing
memory for a BAR that is reserved, so having a BAR enabled that we
have not allocated backing memory for is wrong.

Commit c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint
mode in Tegra194") is the commit that marked all BARs other than
BAR0 as reserved, so if you want to test BARs other than BAR0,
talk to the author of that commit.

If you revert this patch, tools/testing/selftests/pci_endpoint/pci_endpoint_test
will once again fail the consecutive BAR test, so I think it would
be wrong to revert this patch.

If it is ATU registers or eDMA registers exposed in BAR4 does not
really matter. The end result is that you overwrite eDMA registers
that you should not be overwriting when you run the BAR tests.
(So BAR4 should absolutely be marked as reserved).

I don't recall, but if you overwrite the eDMA registers, then in
addition to the consecutive BAR test failing, most likely the DMA
test cases will also fail.

Have you tried running
tools/testing/selftests/pci_endpoint/pci_endpoint_test
?


Kind regards,
Niklas

Re: [PATCH v2 2/3] PCI: tegra194: Reset BARs when running in PCIe endpoint mode

[Manikanta Maddireddy]

On 09/02/26 11:57 pm, Niklas Cassel wrote:

> On Sun, Feb 08, 2026 at 11:41:42PM +0530, Manikanta Maddireddy wrote:
>> Hi Niklas,
>>
>> Tegra PCIe exposes only DMA register over BAR4, not iATU.
>> So, the issue described in this commit message is not applicable.
>> This patch is disabling BAR2 and BAR4, after enumeration I see
>> only BAR0. I think we should revert this patch.
>> Please share your inputs on this.
> If you look at this commit, it only disables all BARs by default,
> which brings the tegra driver in-line with all other DWC based
> endpoint drivers: dra7xx, imx6, layerscape-ep, artpec6, dw-rockchip,
> qcom-ep, rcar-gen4, and uniphier-ep.
>
> A PCI endpoint function (EPF) driver will still be able to enable a
> BAR that was disabled in .init().
> However, an EPF driver will not be able to use/enable a reserved BAR.
>
> Look at e.g. the code in pci-epf-test. It will not allocate backing
> memory for a BAR that is reserved, so having a BAR enabled that we
> have not allocated backing memory for is wrong.
>
> Commit c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint
> mode in Tegra194") is the commit that marked all BARs other than
> BAR0 as reserved, so if you want to test BARs other than BAR0,
> talk to the author of that commit.
>
> If you revert this patch, tools/testing/selftests/pci_endpoint/pci_endpoint_test
> will once again fail the consecutive BAR test, so I think it would
> be wrong to revert this patch.
>
> If it is ATU registers or eDMA registers exposed in BAR4 does not
> really matter. The end result is that you overwrite eDMA registers
> that you should not be overwriting when you run the BAR tests.
> (So BAR4 should absolutely be marked as reserved).
>
> I don't recall, but if you overwrite the eDMA registers, then in
> addition to the consecutive BAR test failing, most likely the DMA
> test cases will also fail.
>
> Have you tried running
> tools/testing/selftests/pci_endpoint/pci_endpoint_test
> ?
>
>
> Kind regards,
> Niklas

Hi Niklas,

In Tegra234 PCIe, BAR1 is MSI-X table and BAR2 is DMA registers backed

by PCIe HW RAM and registers. EPF driver shouldn't allocate memory for

these two BARs. This is the reason for marking them as reserved in

Tegra PCIe driver. DMA registers are exposed over BAR2 to allow

PCI client driver in host to transfer data from host to endpoint

using endpoint remote DMA read functionality. BAR test fails on this

because not all register bits are writable. Consider NVMe example

which has RO capability bits at the start of the BAR, it is not correct

to add BAR test on these bits.


I think following fixes are required to address this issue,

1. BAR test in pci_endpoint_test should skip MSI-X table.

2. BAR test in pci_endpoint_test should provide option to

skip this test on known reserved BARs, maybe we can use

pci_endpoint_test_data for this.

3. EPC driver should provide BAR_DISABLED enum to disable

unused BARs.

4. Tegra PCIe driver should disable only BAR_DISABLED bars and

leave BAR_RESERVED untouched.

5. Return NO_BAR for both BAR_DISABLED and BAR_RESERVED in

pci_epc_get_next_free_bar()


Let me know your opinion on this.

Re: [PATCH v2 2/3] PCI: tegra194: Reset BARs when running in PCIe endpoint mode

[Niklas Cassel]

Hello Manikanta,

On Tue, Feb 10, 2026 at 09:40:44AM +0530, Manikanta Maddireddy wrote:
> > On Sun, Feb 08, 2026 at 11:41:42PM +0530, Manikanta Maddireddy wrote:
> > > Hi Niklas,
> > > 
> > > Tegra PCIe exposes only DMA register over BAR4, not iATU.

Here you claim that DMA registers are exposed in BAR4.


> Hi Niklas,
> 
> In Tegra234 PCIe, BAR1 is MSI-X table and BAR2 is DMA registers backed
> by PCIe HW RAM and registers.

Here you claim that DMA registers are exposed in BAR2.
Which one is it?


> EPF driver shouldn't allocate memory for
> these two BARs. This is the reason for marking them as reserved in
> Tegra PCIe driver. DMA registers are exposed over BAR2 to allow
> PCI client driver in host to transfer data from host to endpoint
> using endpoint remote DMA read functionality. BAR test fails on this
> because not all register bits are writable. Consider NVMe example
> which has RO capability bits at the start of the BAR, it is not correct
> to add BAR test on these bits.

Have you tried running
tools/testing/selftests/pci_endpoint/pci_endpoint_test
?

pci_endpoint_test will run BAR tests against all BARs that are enabled by
default, regardless of the BAR being marked as RESERVED or not, see:
https://github.com/torvalds/linux/blob/v6.19/drivers/misc/pci_endpoint_test.c#L1052-L1061

In the case of nvidia,tegra234-pcie-ep, before my commit 42f9c66a6d0c
("PCI: tegra194: Reset BARs when running in PCIe endpoint mode"):
pcie-tegra194.c marked all BARs except BAR0 as reserved.
pci_endpoint_test would run tests against BAR0, BAR2, BAR3, BAR4, BAR5
(it would not run against BAR1, because BAR0 is marked as "only_64bit").

After my commit 42f9c66a6d0c ("PCI: tegra194: Reset BARs when running in
PCIe endpoint mode"):
pcie-tegra194.c still marks all BARs except BAR0 as reserved (I did not
change this).
pci_endpoint_test would run tests only against BAR0.
I.e. the only BAR that is not marked as reserved.


> 
> I think following fixes are required to address this issue,

Could you please define "this issue", because right now I honestly don't
see the issue.

To me it seems like you want pci_endpoint_test to run against BARs that are
marked as reserved (I assume that Vidya marked them as reserved for a good
reason, most likely because all of them map to MMIO registers) and thus you
want multiple test cases in pci_endpoint_test to fail instead of being skipped?


> 1. BAR test in pci_endpoint_test should skip MSI-X table.
> 2. BAR test in pci_endpoint_test should provide option to
> skip this test on known reserved BARs, maybe we can use
> pci_endpoint_test_data for this.

pci_endpoint_test already skips disabled BARs by default.

They way it works is that you disable all BARs in you EPC driver's init()
callback (i.e. what my patch does), pci-epf-test will then allocate backing
memory + enable all BARs that are not marked as RESERVED.


> 3. EPC driver should provide BAR_DISABLED enum to disable
> unused BARs.

BAR_RESERVED already means disabled, it just assumes that an EPC driver
disables all BARs by default, which is the case for:
pci-dra7xx.c, pci-imx6.c, pci-layerscape-ep.c, pcie-artpec6.c,
pcie-designware-plat.c, pcie-dw-rockchip.c, pcie-qcom-ep.c, pcie-rcar-gen4.c,
pcie-stm32-ep.c, pcie-uniphier-ep.c.
(All drivers which disables all BARs by default in the init() callback using
dw_pcie_ep_reset_bar(). pci-epf-test will later enable all BARs that are not
marked as BAR_RESERVED.)

That leaves: pcie-keembay.c, pci-keystone.c, pcie-tegra194.c (before my patch).

For pcie-keembay.c, this is not a problem, because BAR0, BAR2, BAR4 are marked
as only_64bit, so pci-epf-test configure these BARs as 64-bit BARs, and thus
BAR1, BAR3, and BAR5 will get disabled implicitly.

For pci-keystone.c, this is the only driver that is a bit weird, it marks
BAR0 and BAR1 as reserved, but does not disable them in the init() callback.
It seems force set BAR0 as a 32-bit BAR in the init() callback.

Thus, for all drivers except for pci-keystone.c, BAR_RESERVED does mean
BAR_DISABLED. Feel free to send a patch that renames BAR_RESERVED to
BAR_DISABLED.

If you send such a patch, perhaps you also want to modify the PCI endpoint
core to call reset_bar() for all BARs marked as BAR_RESERVED/BAR_DISABLED,
instead of each EPC driver doing so in the init() callback. I think the main
reason why this is not done already is that thare is no reset_bar() op in
struct pci_epc_ops epc_ops, there is only clear_bar() which clears an BAR
enabled by an EPF driver. (So you would most likely also need to add a
.disable_bar() op in struct pci_epc_ops epc_ops.)


> 4. Tegra PCIe driver should disable only BAR_DISABLED bars and
> leave BAR_RESERVED untouched.
> 5. Return NO_BAR for both BAR_DISABLED and BAR_RESERVED in
> pci_epc_get_next_free_bar()

A BAR marked as BAR_RESERVED will never be returned by
pci_epc_get_next_free_bar(), so this is the case already.


Kind regards,
Niklas

Re: [PATCH v2 2/3] PCI: tegra194: Reset BARs when running in PCIe endpoint mode

[Niklas Cassel]

On Tue, Feb 10, 2026 at 11:06:04AM +0100, Niklas Cassel wrote:
> BAR_RESERVED already means disabled, it just assumes that an EPC driver
> disables all BARs by default, which is the case for:
> pci-dra7xx.c, pci-imx6.c, pci-layerscape-ep.c, pcie-artpec6.c,
> pcie-designware-plat.c, pcie-dw-rockchip.c, pcie-qcom-ep.c, pcie-rcar-gen4.c,
> pcie-stm32-ep.c, pcie-uniphier-ep.c.
> (All drivers which disables all BARs by default in the init() callback using
> dw_pcie_ep_reset_bar(). pci-epf-test will later enable all BARs that are not
> marked as BAR_RESERVED.)
> 
> That leaves: pcie-keembay.c, pci-keystone.c, pcie-tegra194.c (before my patch).
> 
> For pcie-keembay.c, this is not a problem, because BAR0, BAR2, BAR4 are marked
> as only_64bit, so pci-epf-test configure these BARs as 64-bit BARs, and thus
> BAR1, BAR3, and BAR5 will get disabled implicitly.
> 
> For pci-keystone.c, this is the only driver that is a bit weird, it marks
> BAR0 and BAR1 as reserved, but does not disable them in the init() callback.
> It seems force set BAR0 as a 32-bit BAR in the init() callback.
> 
> Thus, for all drivers except for pci-keystone.c, BAR_RESERVED does mean
> BAR_DISABLED. Feel free to send a patch that renames BAR_RESERVED to
> BAR_DISABLED.
> 
> If you send such a patch, perhaps you also want to modify the PCI endpoint
> core to call reset_bar() for all BARs marked as BAR_RESERVED/BAR_DISABLED,
> instead of each EPC driver doing so in the init() callback. I think the main
> reason why this is not done already is that thare is no reset_bar() op in
> struct pci_epc_ops epc_ops, there is only clear_bar() which clears an BAR
> enabled by an EPF driver. (So you would most likely also need to add a
> .disable_bar() op in struct pci_epc_ops epc_ops.)

Aksh (on To:),

since you have a @ti.com email, perhaps you can explain how pci-keystone.c
can pass all the pci-epf-test test cases, considering that this is the only
driver that has BARs (BAR0 and BAR1) marked as BAR_RESERVED but do not also
disable the BARs (using dw_pcie_ep_reset_bar()) in the init() callback.

Or, perhaps the simple answer is that pci-keystone.c does not pass all
pci-epf-test test cases?


Kind regards,
Niklas

Re: [PATCH v2 2/3] PCI: tegra194: Reset BARs when running in PCIe endpoint mode

[Niklas Cassel]

On Tue, Feb 10, 2026 at 04:09:05PM +0530, Manikanta Maddireddy wrote:
> > For pci-keystone.c, this is the only driver that is a bit weird, it marks
> > BAR0 and BAR1 as reserved, but does not disable them in the init() callback.
> > It seems force set BAR0 as a 32-bit BAR in the init() callback.
> > 
> > Thus, for all drivers except for pci-keystone.c, BAR_RESERVED does mean
> > BAR_DISABLED. Feel free to send a patch that renames BAR_RESERVED to
> > BAR_DISABLED.
> > 
> > If you send such a patch, perhaps you also want to modify the PCI endpoint
> > core to call reset_bar() for all BARs marked as BAR_RESERVED/BAR_DISABLED,
> > instead of each EPC driver doing so in the init() callback. I think the main
> > reason why this is not done already is that thare is no reset_bar() op in
> > struct pci_epc_ops epc_ops, there is only clear_bar() which clears an BAR
> > enabled by an EPF driver. (So you would most likely also need to add a
> > .disable_bar() op in struct pci_epc_ops epc_ops.)
> > 
> pci-epc.h defined
> 
>  * @BAR_RESERVED: The BAR should not be touched by an EPF driver.
> 
> I believe you are interpreting this as unused BAR?
> 
> In Tegra PCIe, BAR2 and BAR4 are backed by PCIe HW memory which
> 
> shouldn't be touched by EPF, but should be kept enabled.
> 
> This support is not available. I suggested to add BAR_DISABLED
> 
> for unused and use BAR_RESERVED for bars like above.

I understand what you want. You want to have a BAR_RESERVED and a
BAR_DISABLED.

Sounds like a nice feature. Feel free to add that.


But like I mentioned in my reply, for all existing drivers, except for
pci-keystone.c, in practice BAR_RESERVED actually means BAR_DISABLED.
(Since all drivers except for pci-keystone.c call reset_bar() in init(),
and pci-epf-test will not enable BARs marked as BAR_RESERVED).

So if you add a BAR_DISABLED, make sure that you convert all existing
uses of BAR_RESERVED (except for pci-keystone.c) to BAR_DISABLED.

pci-keystone.c and pcie-tegra194.c can then be the only drivers that
have BARs marked as BAR_RESERVED (all other drivers would use BAR_DISABLED).

Please just make sure that you don't regress the amount of currently passing
pci_endpoint_test test cases that are passing for pcie-tegra194.c.
(Which would be the case if you revert the patch in $subject without first
adding your proposed new BAR_DISABLED, such that we can have a distinction
between BAR_DISABLED and BAR_RESERVED.)


Kind regards,
Niklas

Re: [PATCH v2 2/3] PCI: tegra194: Reset BARs when running in PCIe endpoint mode

[Aksh Garg]

+ Kishon

On 10/02/26 16:09, Niklas Cassel wrote:
>> BAR_RESERVED already means disabled, it just assumes that an EPC driver
>> disables all BARs by default, which is the case for:
>> pci-dra7xx.c, pci-imx6.c, pci-layerscape-ep.c, pcie-artpec6.c,
>> pcie-designware-plat.c, pcie-dw-rockchip.c, pcie-qcom-ep.c, pcie-rcar-gen4.c,
>> pcie-stm32-ep.c, pcie-uniphier-ep.c.
>> (All drivers which disables all BARs by default in the init() callback using
>> dw_pcie_ep_reset_bar(). pci-epf-test will later enable all BARs that are not
>> marked as BAR_RESERVED.)
>> 
>> That leaves: pcie-keembay.c, pci-keystone.c, pcie-tegra194.c (before my patch).
>> 
>> For pcie-keembay.c, this is not a problem, because BAR0, BAR2, BAR4 are marked
>> as only_64bit, so pci-epf-test configure these BARs as 64-bit BARs, and thus
>> BAR1, BAR3, and BAR5 will get disabled implicitly.
>> 
>> For pci-keystone.c, this is the only driver that is a bit weird, it marks
>> BAR0 and BAR1 as reserved, but does not disable them in the init() callback.
>> It seems force set BAR0 as a 32-bit BAR in the init() callback.
>> 
>> Thus, for all drivers except for pci-keystone.c, BAR_RESERVED does mean
>> BAR_DISABLED. Feel free to send a patch that renames BAR_RESERVED to
>> BAR_DISABLED.
>> 
>> If you send such a patch, perhaps you also want to modify the PCI endpoint
>> core to call reset_bar() for all BARs marked as BAR_RESERVED/BAR_DISABLED,
>> instead of each EPC driver doing so in the init() callback. I think the main
>> reason why this is not done already is that thare is no reset_bar() op in
>> struct pci_epc_ops epc_ops, there is only clear_bar() which clears an BAR
>> enabled by an EPF driver. (So you would most likely also need to add a
>> .disable_bar() op in struct pci_epc_ops epc_ops.)
> 
> Aksh (on To:),
> 
> since you have a @ti.com email, perhaps you can explain how pci-keystone.c
> can pass all the pci-epf-test test cases, considering that this is the only
> driver that has BARs (BAR0 and BAR1) marked as BAR_RESERVED but do not also
> disable the BARs (using dw_pcie_ep_reset_bar()) in the init() callback.
> 
> Or, perhaps the simple answer is that pci-keystone.c does not pass all
> pci-epf-test test cases?
> 
> 
> Kind regards,
> Niklas

Hi Niklas,

I just joined the organization and have no context on why the
pci-keystone.c have BAR0 and BAR1 as reserved, without disabling the
bars using dw_pcie_ep_reset_bar() in the .init() callback. Because the
AM65 do not use any BARs for any purpose like Tegra194 does (ATU
registers or eDMA registers exposed in BAR4 for example), there would
be no issue if the BAR0 and BAR1 are overwritten.

This was introduced in the driver the time the EP support was added to
the driver by Kishon in commit 23284ad677a9 ("PCI: keystone: Add support
for PCIe EP in AM654x Platforms"), where no context is provided in the
comments or commit message why the BAR0/1 are marked as reserved in the
features. Perhaps Kishon can provide a better insight over this.

Regards,
Aksh Garg

> 

Re: [PATCH v2 2/3] PCI: tegra194: Reset BARs when running in PCIe endpoint mode

[Niklas Cassel]

On Thu, Feb 12, 2026 at 05:40:59PM +0530, Aksh Garg wrote:
> > since you have a @ti.com email, perhaps you can explain how pci-keystone.c
> > can pass all the pci-epf-test test cases, considering that this is the only
> > driver that has BARs (BAR0 and BAR1) marked as BAR_RESERVED but do not also
> > disable the BARs (using dw_pcie_ep_reset_bar()) in the init() callback.
> > 
> > Or, perhaps the simple answer is that pci-keystone.c does not pass all
> > pci-epf-test test cases?
> 
> Hi Niklas,
> 
> I just joined the organization and have no context on why the
> pci-keystone.c have BAR0 and BAR1 as reserved, without disabling the
> bars using dw_pcie_ep_reset_bar() in the .init() callback. Because the
> AM65 do not use any BARs for any purpose like Tegra194 does (ATU
> registers or eDMA registers exposed in BAR4 for example), there would
> be no issue if the BAR0 and BAR1 are overwritten.
> 
> This was introduced in the driver the time the EP support was added to
> the driver by Kishon in commit 23284ad677a9 ("PCI: keystone: Add support
> for PCIe EP in AM654x Platforms"), where no context is provided in the
> comments or commit message why the BAR0/1 are marked as reserved in the
> features. Perhaps Kishon can provide a better insight over this.

It is extra confusing, since the older driver from TI:
pci-dra7xx.c does have the dw_pcie_ep_reset_bar() calls in init()
(git blame shows added by Kishon), so it is a bit surprising that
the newer driver (pci-keystone.c) does not.

(And like I explained, currently all DWC drivers except keystone and
pcie-keembay.c do have the dw_pcie_ep_reset_bar() calls in init().)


Kind regards,
Niklas

Re: [PATCH v2 2/3] PCI: tegra194: Reset BARs when running in PCIe endpoint mode

[Aksh Garg]

On 2/12/2026 5:50 PM, Niklas Cassel wrote:
> On Thu, Feb 12, 2026 at 05:40:59PM +0530, Aksh Garg wrote:
>> > since you have a @ti.com email, perhaps you can explain how pci-keystone.c
>> > can pass all the pci-epf-test test cases, considering that this is the only
>> > driver that has BARs (BAR0 and BAR1) marked as BAR_RESERVED but do not also
>> > disable the BARs (using dw_pcie_ep_reset_bar()) in the init() callback.
>> > 
>> > Or, perhaps the simple answer is that pci-keystone.c does not pass all
>> > pci-epf-test test cases?
>> 
>> Hi Niklas,
>> 
>> I just joined the organization and have no context on why the
>> pci-keystone.c have BAR0 and BAR1 as reserved, without disabling the
>> bars using dw_pcie_ep_reset_bar() in the .init() callback. Because the
>> AM65 do not use any BARs for any purpose like Tegra194 does (ATU
>> registers or eDMA registers exposed in BAR4 for example), there would
>> be no issue if the BAR0 and BAR1 are overwritten.
>> 
>> This was introduced in the driver the time the EP support was added to
>> the driver by Kishon in commit 23284ad677a9 ("PCI: keystone: Add support
>> for PCIe EP in AM654x Platforms"), where no context is provided in the
>> comments or commit message why the BAR0/1 are marked as reserved in the
>> features. Perhaps Kishon can provide a better insight over this.
> 
> It is extra confusing, since the older driver from TI:
> pci-dra7xx.c does have the dw_pcie_ep_reset_bar() calls in init()
> (git blame shows added by Kishon), so it is a bit surprising that
> the newer driver (pci-keystone.c) does not.
> 
> (And like I explained, currently all DWC drivers except keystone and
> pcie-keembay.c do have the dw_pcie_ep_reset_bar() calls in init().)
> 

Yes, true. I tried to backtrace why this is so for pci-keystone.c, but
couldn't find anything on this.

> Kind regards,
> Niklas
>