Hi Lucas,
On 2014年11月14日 19:42, Lucas Stach wrote:
Am Freitag, den 14.11.2014, 11:30 +0000 schrieb
Mingkai.Hu@xxxxxxxxxxxxx:
-----Original Message-----
From: Lucas Stach [mailto:l.stach@xxxxxxxxxxxxxx]
Sent: Friday, November 14, 2014 6:02 PM
To: Lian Minghuan-B31939
Cc: Srikanth Thokala; Lian Minghuan-B31939; linux-pci@xxxxxxxxxxxxxxx;
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; 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.
If another platform uses the same driver you should still have a
specific compatible for that platform. Exactly to differentiate those
silicon implementation differences.
If you add 2 more ATUs to the layerscape pcie in a future design it is
not fsl,ls1021a-pcie anymore, but for example fsl,ls1100-pcie. Also you
don't need the code construct as seen above. The number of ATU can be
added to implementation specific data.
Please look at the Tegra PCI driver and especially struct
tegra_pcie_soc_data on how to properly abstract properties that are
defined by a specific silicon implementation of a core.
[Minghuan] Yes, we can added specific data for different implementation.
But we need to add hard coded information and assignment statement to
the layerscape PCIe driver even to every PCIe driver based on
pcie-designware.c.
Why not use dts? Nothing needs to be added to specific implementation
driver code.
The Device Tree is a data structure for describing hardware. Rather than
hard coding every detail of a device into an operating system, many
aspect of the hardware can be described in a data structure that is
passed to the operating system at boot time.
Thanks,
Minghuan
Regards,
Lucas
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
