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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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,

On Wed, Nov 12, 2014 at 3:39 PM, Lian Minghuan-B31939
<B31939@xxxxxxxxxxxxx> wrote:
Hi Srikanth,

please see my comments inline.

Thanks,
Minghuan


On 2014年11月12日 17:01, Srikanth Thokala wrote:
Hi Minghuan,

On Wed, Nov 12, 2014 at 12:44 PM, Lian Minghuan-B31939
<B31939@xxxxxxxxxxxxx> wrote:
Hi  Srikanth,

Thanks for your comments, please see my reply inline.


On 2014年11月12日 14:22, Srikanth Thokala wrote:
Hi,

On Tue, Nov 11, 2014 at 10:37 AM, Minghuan Lian
<Minghuan.Lian@xxxxxxxxxxxxx> wrote:
Currently, pcie-designware.c only supports two ATUs, ATU0 is used
for CFG0 and MEM, ATU1 is used for CFG1 and IO. There is a conflict
when MEM and CFG0 are accessed simultaneously. The patch adds
'num-atus' property to PCIe dts node to describe the number of
PCIe controller's ATUs. If num_atus is bigger than or equal to 4,
we will change ATUs assignment: ATU0 for CFG0, ATU1 for CFG1,
ATU2 for MEM, ATU3 for IO.

Signed-off-by: Minghuan Lian <Minghuan.Lian@xxxxxxxxxxxxx>
---
change log:
v1-v2:
1. add the default value to property num-atus description
2. Use atu_idx[] instead of single values
3. initialize num_atus to 2

    .../devicetree/bindings/pci/designware-pcie.txt    |  1 +
    drivers/pci/host/pcie-designware.c                 | 53
++++++++++++++++------
    drivers/pci/host/pcie-designware.h                 |  9 ++++
    3 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index 9f4faa8..64d0533 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -26,3 +26,4 @@ Optional properties:
    - bus-range: PCI bus numbers covered (it is recommended for new
devicetrees to
      specify this property, to keep backwards compatibility a range of
0x00-0xff
      is assumed if not present)
+- num-atus: number of ATUs. The default value is 2 if not present.
diff --git a/drivers/pci/host/pcie-designware.c
b/drivers/pci/host/pcie-designware.c
index dfed00a..46a609d 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -48,6 +48,8 @@
    #define PCIE_ATU_VIEWPORT              0x900
    #define PCIE_ATU_REGION_INBOUND                (0x1 << 31)
    #define PCIE_ATU_REGION_OUTBOUND       (0x0 << 31)
+#define PCIE_ATU_REGION_INDEX3         (0x3 << 0)
+#define PCIE_ATU_REGION_INDEX2         (0x2 << 0)
    #define PCIE_ATU_REGION_INDEX1         (0x1 << 0)
    #define PCIE_ATU_REGION_INDEX0         (0x0 << 0)
    #define PCIE_ATU_CR1                   0x904
@@ -346,7 +348,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
           struct of_pci_range range;
           struct of_pci_range_parser parser;
           struct resource *cfg_res;
-       u32 val, na, ns;
+       u32 num_atus = 2, val, na, ns;
I think this can be u8?
[Minghuan] I define num-atus like this: num-atus = <6> (our controller
supports 6 outbound ATUs)
So, num_atus should be u32 type.
If we use u8 type to define num_atus, the dts node should be changed to
num_atus = /bits/ 8 <8>.
I prefer the previous definition num-atus = <6> which is more simple and
clear.
Yes, I agree.  But as it holds only 6 possible distinct values, I
prefer to use u8.
[Minghuan] PCIe Designware IP supports more than 6 ATUs. But our
current PCIe controller only supports 6 outbound ATUs and 6 inbound ATUs.
The next PCIe controller may supports more ATUs. I think u32 can be better
compatible with hardware upgrade.

I inquired dts, almost all dts property use u32 type.
I don't think this property will have values > 255, but if you think
so you could
use u16 and then u32.

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.

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




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux