Hi Bjorn, We need at least four kinds of PCIe transactions: CFG0, CFG1, IO, MEM. The best solution is to assign 4 iATU to support them separately for example: ATU0- CFG0, ATU1-CFG1, ATU2-IO ATU3-MEM. So, for IO and MEM, we don't need to update iATU and for CFG, we only need to update a register to update BUS device and function number. Moreover, this can avoid any conflict. But There are some SOCs that only provides two iATUs. So the latest driver provides a solution: ATU0 is shared by CFG0 CFG1 and IO, ATU1 is dedicated to MEM. The driver should first set ATU0 as IO and ATU1 as MEM. When accessing CFG0 or CFG1, ATU0 will be changed to CFG0/CFG1, then back to IO setting. CFG0 CFG1 can sequenced by pci_lock. But CFG IO and MEM(may be generated by PCIe device's DMA) are completely independent. Because most PCIe device do not need IO transaction and in general CFG0 CFG1 and IO transaction is generated by PCIe device driver and rarely occurred simultaneously. So the current solution can minimize conflict. In fact, for the current pcie-designware driver, there is no way to avoid IO and CFG conflict. Thanks, Minghuan > -----Original Message----- > From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx] > Sent: Wednesday, January 06, 2016 6:42 AM > To: Jingoo Han <jingoohan1@xxxxxxxxx>; Pratyush Anand > <pratyush.anand@xxxxxxxxx> > Cc: linux-pci@xxxxxxxxxxxxxxx; Jisheng Zhang <jszhang@xxxxxxxxxxx>; > Minghuan.Lian@xxxxxxxxxxxxx > Subject: DesignWare ATU PCIE_ATU_TYPE_MEM usage > > Hi guys, > > This code in dw_pcie_host_init() looks wrong: > > if (!pp->ops->rd_other_conf) > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX1, > PCIE_ATU_TYPE_MEM, pp->mem_base, > pp->mem_bus_addr, pp->mem_size); > > dw_pcie_wr_own_conf(pp, PCI_BASE_ADDRESS_0, 4, 0); > ... > dw_pcie_rd_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, &val); > > Evidently you need to program the ATU with PCIE_ATU_TYPE_MEM before > doing config accesses on the root bus? > > If that's the case, what about other config accesses after these few in > dw_pcie_host_init()? I don't see anything that changes the ATU programming > for things coming through dw_pcie_wr_conf(). > > I assume you need some sort of locking around this sequence: > > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, type, ... > ret = dw_pcie_cfg_read(va_cfg_base + where, size, val); > dw_pcie_prog_outbound_atu(pp, PCIE_ATU_REGION_INDEX0, > PCIE_ATU_TYPE_IO, ... > > Are you relying on pci_lock? If so, a comment to that effect would be nice. > > The whole ATU management looks pretty inefficient. It's likely that there'll be > a whole sequence of operations where we don't need to touch the ATU, but > since we don't remember its current state, we configure the whole thing from > scratch each time, which is at least eight register writes (plus a read for the > new synchronization). > > Bjorn ��.n��������+%������w��{.n�����{���"�)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
