Re: [PATCH v2] PCI: designware: Add support 4 ATUs assignment

[Lian Minghuan-B31939 <B31939@freescale.com>]

Hi Lucas and all,

On 2014年11月13日 19:09, Lucas Stach wrote:
> Am Donnerstag, den 13.11.2014, 18:52 +0800 schrieb Lian Minghuan-B31939:
>> Hi Lucas,
>>
>> On 2014年11月13日 18:20, Lucas Stach wrote:
>>> Am Donnerstag, den 13.11.2014, 18:02 +0800 schrieb Lian Minghuan-B31939:
>>>> Hi Lucas,
>>>>
>>>> Please see my comments inline.
>>>>
>>>> Thanks,
>>>> Minghuan
>>>>
>>>> On 2014年11月13日 00:32, Lucas Stach wrote:
>>>>> Am Mittwoch, den 12.11.2014, 21:53 +0530 schrieb Srikanth Thokala:
>>>>>> Hi Minghuan,
>>> [...]
>>>
>>>>> Using a smaller type complicates the DT for little to no benefit. I
>>>>> think it's ok to use u32 here, which is a common standard for integer
>>>>> values in DT.
>>>>>
>>>>> Though this discussion lead me to the question if we even need to have
>>>>> this property in the DT at all. Isn't this a property that is fixed for
>>>>> a specific silicon implementation of the DW core? In that case we could
>>>>> just infer the number of ATUs from the DT compatible, so this should
>>>>> probably just be added to struct pcie_port and properly initialized by
>>>>> the SoC glue drivers.
>>>> [Minghuan]  As far as I know, exynos implements only 2 ATUs, this is why
>>>> pcie-designware only supports 2 ATU. iMX implements 4 ATUs and LS1021A
>>>> implements 6 ATUs.
>>>>
>>> Right so we don't need an additional property in the DT at all. The
>>> number of ATUs is fixed for a specific core compatible and can be passed
>>> in by the respective exynos, imx and ls1021 glue drivers.
>>>
>>> You may ask the Keystone and Spear maintainers to get the correct number
>>> of ATUs for those implementations.
>>>
>>> Regards,
>>> Lucas
>> [Minghuan] Yes. This a way that specific core driver passes the ATU
>> number to
>> pci-designware. But I perfer to adding dts node for the following reasons:
>> 1. ATU number is hardware attribute, so it can be added to DTS.
> But it is a duplication of information that can be inferred from the DT
> compatible alone, which is usually frowned upon.
>
> Also in contrast to the num-lanes property I don't see a use-case to
> reduce the number of used ATUs in a specific system, so num-atus is
> basically fixed for a specific implementation.
[Minghuan] 4ATUs provides better support for multiple-function and 
SR-IOV PCIe devices.
Can anyone tell me if there is the company will increase ATUs number?
If yes, we should avoid the following code:

if (of_device_is_compatible(pcie->dev->of_node, "xxx,r1-pcie"))
     atu_num = 2;

if (of_device_is_compatible(pcie->dev->of_node, "xxx,r2-pcie"))
     atu_num = 4;


>> 2. That pci-designware common code parses the 'num-atus' can avoid every
>> specific controller driver to define and pass num-atus, so can reduce
>> code size and simplify the specific controller driver implementation.
>>
> I don't think the code reduction matters here and the simplification is
> minimal.
>
> Regards,
> Lucas
>