Re: [PATCH 2/3] PCI: dwc: Add ECAM support with iATU configuration

[Bjorn Helgaas]

On Mon, Dec 09, 2024 at 10:00:06AM +0530, Krishna Chaitanya Chundru wrote:
> On 12/5/2024 3:47 AM, Bjorn Helgaas wrote:
> > On Wed, Dec 04, 2024 at 07:45:29AM +0530, Krishna Chaitanya Chundru wrote:
> > > On 12/4/2024 12:25 AM, Bjorn Helgaas wrote:
> > > > On Sun, Nov 17, 2024 at 03:30:19AM +0530, Krishna chaitanya chundru wrote:
> > > > > The current implementation requires iATU for every configuration
> > > > > space access which increases latency & cpu utilization.
> > > > > 
> > > > > Configuring iATU in config shift mode enables ECAM feature to access the
> > > > > config space, which avoids iATU configuration for every config access.
> > 
> > > > > +static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
> > > > > +{
> > > > > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > > > +	struct dw_pcie_ob_atu_cfg atu = {0};
> > > > > +	struct resource_entry *bus;
> > > > > +	int ret, bus_range_max;
> > > > > +
> > > > > +	bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
> > > > > +
> > > > > +	/*
> > > > > +	 * Bus 1 config space needs type 0 atu configuration
> > > > > +	 * Remaining buses need type 1 atu configuration
> > > > 
> > > > I'm confused about the bus numbering; you refer to "bus 1" and "bus
> > > > 2".  Is bus 1 the root bus, i.e., the primary bus of a Root Port?
> > > > 
> > > > The root bus number would typically be 0, not 1, and is sometimes
> > > > programmable.  I don't know how the DesignWare core works, but since
> > > > you have "bus" here, referring to "bus 1" and "bus 2" here seems
> > > > overly specific.
> > > > 
> > > root bus is bus 0 and we don't need any iATU configuration for it as
> > > its config space is accessible from the system memory, for usp port of
> > > the switch or the direct the endpoint i.e bus 1 we need to send
> > > Configuration Type 0 requests and for other buses we need to send
> > > Configuration Type 1 requests this is as per PCIe spec, I will try to
> > > include PCIe spec details in next patch.
> > 
> > I understand the Type 0/Type 1 differences.  The question is whether
> > the root bus number is hard-wired to 0.
> > 
> It is not hard-wired to 0, we can configure it though bus-range property
>
> > I don't think specifying "bus 1" really adds anything.  The point is
> > that we need Type 0 accesses for anything directly below a Root Port
> > (regardless of what the RP's secondary bus number is), and Type 1 for
> > things deeper.
>
> I will update the comment without mentioning the buses as suggested.
>
> > When DWC supports multiple Root Ports in a Root Complex, they will not
> > all have a secondary bus number of 1.
>
> mostly they should be in bus number 0 with different device numbers, but
> it mostly depends upon the design, currently we don't have any multiple
> root ports.

Say "root bus" instead of "bus 0", since you said above that the root
bus number is configurable.

Root Ports should all have a *primary* bus number of the root bus, but
if there are multiple Root Ports, they will all have different
secondary bus numbers.