RE: [PATCH v5 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver

[Bharat Kumar Gogada]

> Subject: Re: [PATCH v5 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver
> 
> 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).
> 
Hi Lorenzo,

The bridge register access  are not using config space accessors, we have local pcie_read/write
api for accessing bridge registers.
The hardware logic have details of offsets to which it will not send config accesses it will do local
AXI read and write accesses when the bridge registers are accessed.

Regards,
Bharat