Re: [PATCH 3/4] PCI: tegra194: Expose BAR2 (MSI-X) and BAR4 (DMA) as 64-bit BAR_RESERVED

[Niklas Cassel <cassel@kernel.org>]

On Mon, Feb 23, 2026 at 01:04:55AM +0530, Manikanta Maddireddy wrote:
> Tegra endpoint exposes three 64-bit BARs at indices 0, 2, and 4:
> - BAR0+BAR1: EPF test/data (programmable 64-bit BAR)
> - BAR2+BAR3: MSI-X table (hardware-backed)
> - BAR4+BAR5: DMA registers (hardware-backed)
> 
> Update tegra_pcie_epc_features so BAR2 is BAR_RESERVED with
> PCI_EPC_BAR_RSVD_MSIX_CTRL_MMIO (128KB), BAR3 is BAR_64BIT_UPPER,
> BAR4 is BAR_RESERVED with PCI_EPC_BAR_RSVD_DMA_CTRL_MMIO (4KB), and
> BAR5 is BAR_64BIT_UPPER. This keeps CONSECUTIVE_BAR_TEST working
> while allowing the host to use 64-bit BAR2 (MSI-X) and BAR4 (DMA).
> 
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> ---
>  drivers/pci/controller/dwc/pcie-tegra194.c | 38 +++++++++++++++++++---
>  1 file changed, 33 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index 3c84a230dc79..b5397a63461f 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -2000,16 +2000,44 @@ static int tegra_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>  	return 0;
>  }
>  
> -/* Tegra EP: BAR0 = 64-bit programmable BAR */
> +static const struct pci_epc_bar_rsvd_region tegra194_bar2_rsvd[] = {
> +	{
> +		/* MSI-X structure */
> +		.type = PCI_EPC_BAR_RSVD_MSIX_CTRL_RAM,
> +		.offset = 0x0,
> +		.size = SZ_128K,
> +	},
> +};
> +
> +static const struct pci_epc_bar_rsvd_region tegra194_bar4_rsvd[] = {
> +	{
> +		/* DMA_CAP (BAR4: DMA Port Logic Structure) */
> +		.type = PCI_EPC_BAR_RSVD_DMA_CTRL_MMIO,
> +		.offset = 0x0,
> +		.size = SZ_4K,
> +	},
> +};
> +
> +/* Tegra EP: BAR0 = 64-bit programmable BAR,  BAR2 = 64-bit MSI-X table, BAR4 = 64-bit DMA regs. */
>  static const struct pci_epc_features tegra_pcie_epc_features = {
>  	.linkup_notifier = true,
>  	.msi_capable = true,
>  	.bar[BAR_0] = { .type = BAR_PROGRAMMABLE, .only_64bit = true, },
>  	.bar[BAR_1] = { .type = BAR_64BIT_UPPER, },
> -	.bar[BAR_2] = { .type = BAR_DISABLED, },
> -	.bar[BAR_3] = { .type = BAR_DISABLED, },
> -	.bar[BAR_4] = { .type = BAR_DISABLED, },
> -	.bar[BAR_5] = { .type = BAR_DISABLED, },
> +	.bar[BAR_2] = {
> +		.type = BAR_RESERVED,
> +		.only_64bit = true,

Here you define a BAR of type BAR_RESERVED as .only_64bit.

In include/linux/pci-epc.h:
"only_64bit should not be set on a BAR of type BAR_RESERVED.
(If BARx is a 64-bit BAR that an EPF driver is not allowed to
reprogram, then both BARx and BARx+1 must be set to type
BAR_RESERVED.)"


However, if we look at pci_epc_get_next_free_bar(), it will
handle this perfectly fine already.


So I think it does make sense to allow a RERSERVED_BAR to have
only_64bit = true after all.

(E.g. if we in the future want to disable the RESERVED BAR,
it is good to know that it is a 64-bit BAR, even if it is
RESERVED, because if we disable it, the disable function will
know that it will need to clear the upper bits/adjecent BAR as
well.)

Perhaps just create a new commit in your series which simply removes
does:

@@ -243,11 +243,6 @@ struct pci_epc_bar_rsvd_region {
  *             should be configured as 32-bit or 64-bit, the EPF driver must
  *             configure this BAR as 64-bit. Additionally, the BAR succeeding
  *             this BAR must be set to type BAR_64BIT_UPPER.
- *
- *             only_64bit should not be set on a BAR of type BAR_RESERVED.
- *             (If BARx is a 64-bit BAR that an EPF driver is not allowed to
- *             reprogram, then both BARx and BARx+1 must be set to type
- *             BAR_RESERVED.)
  * @nr_rsvd_regions: number of fixed subregions described for BAR_RESERVED
  * @rsvd_regions: fixed subregions behind BAR_RESERVED
  */

from include/linux/pci-epc.h.

With the motivation that if the BAR is 64-bit by default, and it is
reserved, it makes sense to set it as only64bit, because that is the most
accurate description, and even if we don't have a disable_bar() function
to disable a BAR that we have not called set_bar() on today (so we can't
disable a reserved BAR today), we might implement it in the future, and
then the disable_bar() function will need to clear the adjecent BAR as
well, so better to describe it as correctly as possible already.


Kind regards,
Niklas