On Thu, Jan 9, 2020 at 11:14:58, Shawn Guo <shawn.guo@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > ---
> > > .../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@xxxxxxxxxxxx>
>
> Thanks!
>
> Shawn
