Re: [PATCH v2] PCI: dwc: rcar-gen4-ep: Mark BAR0 and BAR2 as Resizable BARs

[Manivannan Sadhasivam]

On Wed, Mar 04, 2026 at 02:32:30PM +0100, Niklas Cassel wrote:
> Hello Bjorn, Koichiro,
> 
> On Tue, Mar 03, 2026 at 02:12:05PM +0900, Koichiro Den wrote:
> > > > --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> > > > @@ -422,7 +422,9 @@ static int rcar_gen4_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > > >  static const struct pci_epc_features rcar_gen4_pcie_epc_features = {
> > > >  	DWC_EPC_COMMON_FEATURES,
> > > >  	.msi_capable = true,
> > > > +	.bar[BAR_0] = { .type = BAR_RESIZABLE, },
> > > >  	.bar[BAR_1] = { .type = BAR_RESERVED, },
> > > > +	.bar[BAR_2] = { .type = BAR_RESIZABLE, },
> > > 
> > > I guess the "Resizable" property is not something that is discoverable
> > > at runtime?
> > > 
> > > You mention the Resizable BAR Capability, so I wondered if the driver
> > > could use that to figure this out.
> > 
> > As I understand it, it should be discoverable on DesignWare EPC, since a BAR
> > configured with a resizable sizing scheme results in a corresponding Resizable
> > BAR capabilioty/control pair, and the BAR index is there.
> 
> It is no secret that the PCI endpoint framework has been largely designed
> with DWC based controllers in mind.
> 
> After all, 13 out of the 16 EPC drivers are for DWC based controllers.
> 
> Only 3 out of the 16 EPC drivers are for non-DWC based contollers:
> drivers/pci/controller/cadence/pcie-cadence-ep.c
> drivers/pci/controller/pcie-rcar-ep.c
> drivers/pci/controller/pcie-rockchip-ep.c
> 
> For DWC based contollers, the databook defines three sizing schemes:
> BARn_SIZING_SCHEME_N =Fixed Mask (0)
> BARn_SIZING_SCHEME_N =Programmable Mask (1)
> BARn_SIZING_SCHEME_N =Resizable BAR (2)
> 
> Each of these require different ways to program the BAR size/mask.
> 
> For Fixed, the BAR mask is fixed, so really nothing to program.
> 
> For Programmable, the BAR mask is writable.
> 
> For Resizable, the BAR mask is writable, but should be configured
> via the Resizable BAR Capability, since you also need to define
> which sizes the host side can change between. Right now the PCI endpoint
> framework only supports a single supported "Resizable BAR" size.
> So essentially it is like a programmable BAR, but still needs to be
> program the size via the Resizable BAR Capability (so different from
> a programmable BAR).
> 
> 
> 
> A Resizable BAR also has a minimum size of 1 MB, whereas the programmable
> BAR can be smaller.
> 
> A Resizable BAR also has the unfortunate property that the selected BAR
> size is a non-sticky register, so it gets reset during any type of reset.
> (Unlike the register used for programmable BARs, which is sticky, and thus
> keeps its value for most types of reset.)
> 
> Thus we already have code to detect and restore the BAR size for Resizable
> BARs (if an PCI endpoint function driver has configured a BAR size):
> https://github.com/torvalds/linux/blob/v7.0-rc2/drivers/pci/controller/dwc/pcie-designware-ep.c#L1075-L1104
> 
> This code works regardless of type configured in epc_features.
> 
> It would probably be possible to extend this code to detect that a BAR is
> Resizable, and then "override" any default configured type in epc_features.
> 
> 
> 
> Note that there are also other things in epc_features that might be able to
> be detected in runtime, e.g. msi_capable and msix_capable (by iterating the
> capabilities list and see if they are there).
> 
> Possibly, it done this way because certain endpoint controllers, specifically
> Qcom and Tegra, require a reference clock (i.e. PCI common clock, provided by
> the host) before they can access DBI registers, which is needed to iterate the
> capabilities list. (Most other EPC drivers provide their own reference clock,
> SRNS/SRIS, so they can access DBI registers as soon as the driver has probed.)
> 
> By having the Resizable BAR information in the driver, we can do certain
> things, e.g. verify that the requested/allocated BAR size, is at least 1 MB,
> see pci_epf_alloc_space() and pci_epf_get_required_bar_size() which both have
> a BARno param, even before the controller has been provided a reference clock.
> 
> So this is not something unique to BAR type/sizing scheme, the same arguments,
> for/against, also applies for msi_capable/msix_capable.
> 
> Manivannan, would it be possible to automatically initialize msi/msix_capable
> on Qcom once a reference is provided, or would this be too late?
> 

Yes, with a valid reference clock, it should be possible to read the
capabilities and detect these. But there is no way we can make sure that the
reference clock is always be present. Maybe we can do it using a devicetree
property that you tried upstreaming some time before:
https://lore.kernel.org/linux-pci/20250425092012.95418-2-cassel@xxxxxxxxxx

But still, if there is no SRIS/SRNS, EPC core cannot read the capabilities
reliably and it has to make use of on the static flag.

> I assume that when Kishon added msi_capable/msix_capable, he did so for a
> reason. But perhaps it was just to have a way do disable msi/msix for platforms
> where MSI/MSI-X is broken?
> 
> See e.g. a52587e0bee1 ("PCI: rockchip: Don't advertise MSI-X in PCIe capabilities")
> 
> But nowadays we tend to hide broken capabilities. Assuming that we hide the
> broken capabilities, before iterating the capabilities list to see which
> features are supported, I don't see the need for msi_capable/msix_capable.
> (Unless there is some issue for boards like Qcom and Tegra that don't provide
> their own reference clock.)
> 

If the EPC core can reliably read the config space, we don't need these static
flags. But unfortunately, that cannot be done on all platforms.

- Mani

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