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

[Lucas Stach <l.stach@pengutronix.de>]

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.

> 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

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |