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

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

[Shawn Guo]

Some platform has 4 (or more) viewports.  In that case, CFG0 and CFG1
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

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

[Shawn Guo]

Some platform has 4 (or more) viewports.  In that case, CFG0 and CFG1
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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Gustavo Pimentel]

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..." 

> 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>

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

[Gustavo Pimentel]

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..." 

> 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>



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Shawn Guo]

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?

@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

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

[Shawn Guo]

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?

@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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Russell King - ARM Linux admin]

On Thu, Jan 09, 2020 at 07:14:58PM +0800, Shawn Guo 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?

Mine too.  It is not a bad habit, it's just a different style, but
some people can't cope with other people having different styles
and feel an urge to try and make them conform to their own ideals.
At the end of the day, it is personal style, and should *not* be
commented on, IMHO.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

[Russell King - ARM Linux admin]

On Thu, Jan 09, 2020 at 07:14:58PM +0800, Shawn Guo 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?

Mine too.  It is not a bad habit, it's just a different style, but
some people can't cope with other people having different styles
and feel an urge to try and make them conform to their own ideals.
At the end of the day, it is personal style, and should *not* be
commented on, IMHO.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Gustavo Pimentel]

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

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

[Gustavo Pimentel]

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



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Vidya Sagar]

On 1/9/2020 11:36 AM, Shawn Guo wrote:
> External email: Use caution opening links or attachments
> 
> 
> Some platform has 4 (or more) viewports.  In that case, CFG0 and CFG1
> can be separated into different ATU regions.
Is there any specific benefit with this scheme?

- Vidya Sagar
> 
> 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
> 

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

[Vidya Sagar]

On 1/9/2020 11:36 AM, Shawn Guo wrote:
> External email: Use caution opening links or attachments
> 
> 
> Some platform has 4 (or more) viewports.  In that case, CFG0 and CFG1
> can be separated into different ATU regions.
Is there any specific benefit with this scheme?

- Vidya Sagar
> 
> 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
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Bjorn Helgaas]

On Thu, Jan 09, 2020 at 12:24:17PM +0000, Gustavo Pimentel wrote:
> 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.

Eagle eyes!  I was taught in the dark ages of typewriters to use two
spaces after a period, but I don't really care either way.  If I
rework a commit log for other reasons I might use two spaces, and I
frequently use vim 'gq' to reformat paragraphs to use the whole line
width, and I think that inserts two spaces (by default), so I try to
be consistent at least within each commit log.  But either is really
fine with me.

Thanks for taking the time to read and pay attention to commit logs!

Bjorn

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

[Bjorn Helgaas]

On Thu, Jan 09, 2020 at 12:24:17PM +0000, Gustavo Pimentel wrote:
> 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.

Eagle eyes!  I was taught in the dark ages of typewriters to use two
spaces after a period, but I don't really care either way.  If I
rework a commit log for other reasons I might use two spaces, and I
frequently use vim 'gq' to reformat paragraphs to use the whole line
width, and I think that inserts two spaces (by default), so I try to
be consistent at least within each commit log.  But either is really
fine with me.

Thanks for taking the time to read and pay attention to commit logs!

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Shawn Guo]

Hi Vidya,

On Thu, Jan 09, 2020 at 11:04:01PM +0530, Vidya Sagar wrote:
> 
> 
> On 1/9/2020 11:36 AM, Shawn Guo wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > Some platform has 4 (or more) viewports.  In that case, CFG0 and CFG1
> > can be separated into different ATU regions.
> Is there any specific benefit with this scheme?

Thanks much for the question which leads me to go back to vendor for
checking design details of 4 (or more) viewports.

It turns out the patch is not complete.  We need more code change to
get the benefit of using separate ATU region for CFG0 and CFG1, that
is the dw_pcie_prog_outbound_atu() call in dw_pcie_access_other_conf()
function can  be saved.  But in the meanwhile, we need to pass
'va_cfg_base | busdev' as the first argument to dw_pcie_write/read()
in there.

@Lorenzo, @Bjorn,

Please ignore this patch.

Shawn

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

[Shawn Guo]

Hi Vidya,

On Thu, Jan 09, 2020 at 11:04:01PM +0530, Vidya Sagar wrote:
> 
> 
> On 1/9/2020 11:36 AM, Shawn Guo wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > Some platform has 4 (or more) viewports.  In that case, CFG0 and CFG1
> > can be separated into different ATU regions.
> Is there any specific benefit with this scheme?

Thanks much for the question which leads me to go back to vendor for
checking design details of 4 (or more) viewports.

It turns out the patch is not complete.  We need more code change to
get the benefit of using separate ATU region for CFG0 and CFG1, that
is the dw_pcie_prog_outbound_atu() call in dw_pcie_access_other_conf()
function can  be saved.  But in the meanwhile, we need to pass
'va_cfg_base | busdev' as the first argument to dw_pcie_write/read()
in there.

@Lorenzo, @Bjorn,

Please ignore this patch.

Shawn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel