RE: [PATCH] PCI: dwc: Separate CFG0 and CFG1 into different ATU regions

[Gustavo Pimentel <Gustavo.Pimentel@synopsys.com>]

On Thu, Jan 9, 2020 at 11:14:58, Shawn Guo <shawn.guo@linaro.org> wrote:

> Hi Gustavo,
> 
> Thanks for taking a look.
> 
> On Thu, Jan 09, 2020 at 10:37:14AM +0000, Gustavo Pimentel wrote:
> > Hi Shawn,
> > 
> > On Thu, Jan 9, 2020 at 6:6:57, Shawn Guo <shawn.guo@linaro.org> wrote:
> > 
> > > Some platform has 4 (or more) viewports.  In that case, CFG0 and CFG1
> > 
> > Remove double space before "In that..." 
> 
> Hmm, that was intentional.  My writing practice is using two spaces
> after a period and single space after a comma.  Is it a bad habit?

I thought it was a typo. I personally don't have anything against it, but 
I didn't see this style on the comments till now. To keep the coherence 
between all patches, I know that Bjorn and Lorenzo like to have it the 
most standardized possible. It is OK by Lorenzo and Bjorn, it's fine for 
me too.

> 
> @Lorenzo, @Bjorn, let me know if you guys think this should be fixed.
> 
> > 
> > > can be separated into different ATU regions.
> > > 
> > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
> > > ---
> > >  .../pci/controller/dwc/pcie-designware-host.c    | 16 +++++++++++++++-
> > >  drivers/pci/controller/dwc/pcie-designware.h     |  1 +
> > >  2 files changed, 16 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index 0f36a926059a..504d2a192bba 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -532,6 +532,7 @@ static int dw_pcie_access_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> > >  	u64 cpu_addr;
> > >  	void __iomem *va_cfg_base;
> > >  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > +	int index = PCIE_ATU_REGION_INDEX1;
> > >  
> > >  	busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) |
> > >  		 PCIE_ATU_FUNC(PCI_FUNC(devfn));
> > > @@ -548,7 +549,20 @@ static int dw_pcie_access_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> > >  		va_cfg_base = pp->va_cfg1_base;
> > >  	}
> > >  
> > > -	dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
> > > +	if (pci->num_viewport >= 4) {
> > > +		/*
> > > +		 * If there are 4 (or more) viewports, we can separate
> > > +		 * CFG0 and CFG1 into different ATU regions:
> > > +		 *  - region0: MEM
> > > +		 *  - region1: CFG0
> > > +		 *  - region2: IO
> > > +		 *  - region3: CFG1
> > > +		 */
> > > +		if (type == PCIE_ATU_TYPE_CFG1)
> > > +			index = PCIE_ATU_REGION_INDEX3;
> > > +	}
> > > +
> > > +	dw_pcie_prog_outbound_atu(pci, index,
> > >  				  type, cpu_addr,
> > >  				  busdev, cfg_size);
> > >  	if (write)
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > index 5a18e94e52c8..86225804f1e7 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > @@ -63,6 +63,7 @@
> > >  #define PCIE_ATU_VIEWPORT		0x900
> > >  #define PCIE_ATU_REGION_INBOUND		BIT(31)
> > >  #define PCIE_ATU_REGION_OUTBOUND	0
> > > +#define PCIE_ATU_REGION_INDEX3		0x3
> > >  #define PCIE_ATU_REGION_INDEX2		0x2
> > >  #define PCIE_ATU_REGION_INDEX1		0x1
> > >  #define PCIE_ATU_REGION_INDEX0		0x0
> > > -- 
> > > 2.17.1
> > 
> > With the description fix,
> > 
> > Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> 
> Thanks!
> 
> Shawn