On Fri, Mar 06, 2020 at 11:45:47AM +0000, Bharat Kumar Gogada wrote: > > From: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx> > > Sent: Friday, March 6, 2020 4:46 PM > > To: Bharat Kumar Gogada <bharatku@xxxxxxxxxx> > > Cc: linux-pci@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > > bhelgaas@xxxxxxxxxx; Ravikiran Gummaluri <rgummal@xxxxxxxxxx>; > > maz@xxxxxxxxxx > > Subject: Re: [PATCH v5 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver > > > > On Fri, Feb 28, 2020 at 12:48:48PM +0000, Bharat Kumar Gogada wrote: > > > > Subject: Re: [PATCH v5 2/2] PCI: xilinx-cpm: Add Versal CPM Root > > > > Port driver > > > > > > > > [+MarcZ, FHI] > > > > > > > > On Tue, Feb 25, 2020 at 02:39:56PM +0000, Bharat Kumar Gogada wrote: > > > > > > > > [...] > > > > > > > > > > > +/* ECAM definitions */ > > > > > > > +#define ECAM_BUS_NUM_SHIFT 20 > > > > > > > +#define ECAM_DEV_NUM_SHIFT 12 > > > > > > > > > > > > You don't need these ECAM_* defines, you can use > > pci_generic_ecam_ops. > > > > > Does this need separate ranges region for ECAM space ? > > > > > We have ECAM and controller space in same region. > > > > > > > > You can create an ECAM window with pci_ecam_create where *cfgres > > > > represent the ECAM area, I don't get what you mean by "same region". > > > > > > > > Do you mean "contiguous" ? Or something else ? > > > Yes, contiguous; within ECAM region some space is for controller registers. > > > > What does that mean ? I don't get it. Can you explain to me how this address > > space works please ? > > > Hi Lorenzo, > reg = <0x6 0x00000000 0x0 0x1000000>, This supports up to 16 busses (it is 16MB in size rather than full ECAM 256MB), right ? Please make sure that the bus-range property reflects that. > <0x0 0xFCA10000 0x0 0x1000>; > reg-names = "cfg", "cpm_slcr"; > > In the above cfg region some region of it reserved for bridge registers and rest for ECAM > address space transactions. The bridge registers are mapped at an unused offset in config space > of root port, when the offset hit it will access controller register space. > > This region is already being mapped > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cfg"); > port->reg_base = devm_ioremap_resource(dev, res); > > Does pci_ecam_create will work along with above API simultaneously ? Basically the bridge registers are accessible through the PCI config accessors (after enumeration), since they are in the bridge device specific config space (device specific area). IIUC the answer is yes and you can access the bridge registers through PCI config space accessors (after enumeration). Pre-enumeration you can map (and unmap) the region as you are doing now (+ the unmap) - since you need a pci_bus structure for PCI config accessors to work and you don't have it till the enumeration is actually executed. Lorenzo