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

[Mingkai.Hu at freescale.com]

> -----Original Message-----
> From: Lucas Stach [mailto:l.stach at pengutronix.de]
> Sent: Friday, November 14, 2014 6:02 PM
> To: Lian Minghuan-B31939
> Cc: Srikanth Thokala; Lian Minghuan-B31939; linux-pci at vger.kernel.org;
> linux-arm-kernel at lists.infradead.org; Zang Roy-R61911; Hu Mingkai-B21284;
> Bjorn Helgaas
> Subject: Re: [PATCH v2] PCI: designware: Add support 4 ATUs assignment
> 
> Am Freitag, den 14.11.2014, 16:47 +0800 schrieb Lian Minghuan-B31939:
> > 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;
> >
> The number of ATUs is fixed for a specific implementation. If the number
> of ATUs changes in a newer implementation of a SoC then the silicon
> implementation of the DW PCIe core changed, so it should get a new
> compatible anyway.
> 

If there are multiple platforms to use the same SoC driver, there will be
multiple cases for different platforms as Minghuan has pointed out.

For the platform that only has 2 ATUs, ATU0 is used for CFG0 and MEM and
there will be conflict when MEM and CFG0 are accessed simultaneously which
has cause the SATA link issue. I think more ATUs will be added in the 
future design, then one driver maybe need to handle multiple platforms.

DTS is designed for to describe the hardware property, it's a general way
to put hardware related property into DTS.

Thanks,
Mingkai