Dear Minghuan, Bjorn, On Thu, 7 Jan 2016 07:11:59 +0000 Minghuan Lian wrote: > 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. IMHO, 3 ATUs is enough, cfg0 and cfg1 can be sequenced by pci_lock as you pointed out ;) > 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. The key is how to sequence IO and CFG, this may need pci subsystem to provide the support. > > > 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 -- 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
