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

[Krishna Chaitanya Chundru <quic_krichai@quicinc.com>]

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.

- Krishna Chaitanya.
> 
> Bjorn