Re: [PATCH] pcie-rcar: try setting PCIe speed to 5 GT/s at boot

[Sergei Shtylyov]

Hello.

On 09/14/2016 11:54 PM, Bjorn Helgaas wrote:


From: Grigory Kletsko 

Initially, the PCIe link speed is set up only at 2.5 GT/s.
For better performance,  we're trying to increase link speed to 5 GT/s.

[Sergei Shtylyov: indented the macro definitions with tabs, renamed the
SPCHG register bits for consistency, renamed the link speed field/values,
fixed too long lines, fixed redundancy in clearing the MACSR register bits,
fixed grammar/typos in  the comments/messages, removed unrelated/useless
changes, fixed bugs in rcar_rwm32() calls done to set the bits, removed
unneeded braces, removed non-informative comment, reworded the patch
summary/description.]

Signed-off-by: Grigory Kletsko 
Signed-off-by: Sergei Shtylyov 

---
The patch is against the 'next' branch of Bjorn Helgaas' 'pci.git' repo.

 drivers/pci/host/pcie-rcar.c |  103 +++
 1 file changed, 103 insertions(+)

Index: pci/drivers/pci/host/pcie-rcar.c
===
--- pci.orig/drivers/pci/host/pcie-rcar.c
+++ pci/drivers/pci/host/pcie-rcar.c

[...]

@@ -385,6 +402,51 @@ static int rcar_pcie_setup(struct list_h
return 1;
 }

+void rcar_pcie_force_speedup(struct rcar_pcie *pcie)
+{
+   u32 macsr;
+
+   dev_info(pcie->dev, "Trying speed up to 5 GT/s\n");
+
+   if ((rcar_pci_read_reg(pcie, MACSR) & SPCHGFIN) ||
+   (rcar_pci_read_reg(pcie, MACCTLR) & SPEED_CHANGE)) {
+   dev_err(pcie->dev, "Speed changing is in progress\n");


Are these messages all really errors?


I guess not. :-)


+   return;
+   }
+
+   if ((rcar_pci_read_reg(pcie, MACSR) & LINK_SPEED) ==
+   LINK_SPEED_5_0GTS) {
+   dev_err(pcie->dev, "Current link speed is already 5 GT/s\n");
+   return;
+   }
+
+   if ((rcar_pci_read_reg(pcie, MACS2R) & LINK_SPEED) !=
+   LINK_SPEED_5_0GTS) {
+   dev_err(pcie->dev,
+   "Current max support link speed not 5 GT/s\n");
+   return;
+   }

[...]

@@ -416,6 +478,9 @@ static int rcar_pcie_enable(struct rcar_

pci_bus_add_devices(bus);

+   /* Try setting 5 GT/s link speed */
+   rcar_pcie_force_speedup(pcie);


I assume it's safe to change the link speed even while the downstream
devices are in use?  As soon as we call pci_bus_add_devices(), drivers
can claim the devices and start using them.


   Not sure. OK, I'm going to move this call before pci_bus_add_devices() and 
try to make it synchronous, without using interrupt.



return 0;
 }

@@ -621,6 +686,44 @@ static irqreturn_t rcar_pcie_msi_irq(int
struct rcar_msi *msi = >msi;
unsigned long reg;

+   if (rcar_pci_read_reg(pcie, PCIEINTR) & PCIEINTMAC) {
+   dev_dbg(pcie->dev, "MAC interrupt received\n");


I guess you don't need to write PCIEINTR or any other register to
acknowledge the interrupt?


   You need to write MACSR -- which the code below does. However, it only 
clears the SPCHGFIN interrupt (which is only ever enabled).



+
+   rcar_rmw32(pcie, MACSR, SPCHGFIN, SPCHGFIN);
+
+   /* Disable this interrupt */
+   rcar_rmw32(pcie, MACINTENR, SPCHGFINE, 0);
+   rcar_rmw32(pcie, PCIEINTER, PCIEINTMACE, 0);



reg = rcar_pci_read_reg(pcie, PCIEMSIFR);

/* MSI & INTx share an interrupt - we only handle MSI here */


Is this patch adding handling for INTx events?


No.

MBR, Sergei

Re: [PATCH] pcie-rcar: try setting PCIe speed to 5 GT/s at boot

[Simon Horman]

On Wed, Sep 07, 2016 at 11:22:14PM +0300, Sergei Shtylyov wrote:
> From: Grigory Kletsko 
> 
> Initially, the PCIe link speed is set up only at 2.5 GT/s.
> For better performance,  we're trying to increase link speed to 5 GT/s.
> 
> [Sergei Shtylyov: indented the macro definitions with tabs, renamed the
> SPCHG register bits for consistency, renamed the link speed field/values,
> fixed too long lines, fixed redundancy in clearing the MACSR register bits,
> fixed grammar/typos in  the comments/messages, removed unrelated/useless
> changes, fixed bugs in rcar_rwm32() calls done to set the bits, removed
> unneeded braces, removed non-informative comment, reworded the patch
> summary/description.]
> 
> Signed-off-by: Grigory Kletsko 
> Signed-off-by: Sergei Shtylyov 

Acked-by: Simon Horman 

[PATCH] pcie-rcar: try setting PCIe speed to 5 GT/s at boot

[Sergei Shtylyov]

From: Grigory Kletsko 

Initially, the PCIe link speed is set up only at 2.5 GT/s.
For better performance,  we're trying to increase link speed to 5 GT/s.

[Sergei Shtylyov: indented the macro definitions with tabs, renamed the
SPCHG register bits for consistency, renamed the link speed field/values,
fixed too long lines, fixed redundancy in clearing the MACSR register bits,
fixed grammar/typos in  the comments/messages, removed unrelated/useless
changes, fixed bugs in rcar_rwm32() calls done to set the bits, removed
unneeded braces, removed non-informative comment, reworded the patch
summary/description.]

Signed-off-by: Grigory Kletsko 
Signed-off-by: Sergei Shtylyov 

---
The patch is against the 'next' branch of Bjorn Helgaas' 'pci.git' repo.

 drivers/pci/host/pcie-rcar.c |  103 +++
 1 file changed, 103 insertions(+)

Index: pci/drivers/pci/host/pcie-rcar.c
===
--- pci.orig/drivers/pci/host/pcie-rcar.c
+++ pci/drivers/pci/host/pcie-rcar.c
@@ -48,6 +48,10 @@
 #define  CFINIT1
 #define PCIETSTR   0x02004
 #define  DATA_LINK_ACTIVE  1
+#define PCIEINTR   0x02008
+#define  PCIEINTMAC(1 << 13)
+#define PCIEINTER  0x0200C
+#define  PCIEINTMACE   (1 << 13)
 #define PCIEERRFR  0x02020
 #define  UNSUPPORTED_REQUEST   (1 << 4)
 #define PCIEMSIFR  0x02044
@@ -84,8 +88,21 @@
 #define IDSETR10x011004
 #define TLCTLR 0x011048
 #define MACSR  0x011054
+#define  SPCHG (1 << 5)
+#define  SPCHGFIN  (1 << 4)
+#define  SPCHGSUC  (1 << 7)
+#define  SPCHGFAIL (1 << 6)
+#define  LINK_SPEED(0xf << 16)
+#define  LINK_SPEED_2_5GTS (1 << 16)
+#define  LINK_SPEED_5_0GTS (2 << 16)
 #define MACCTLR0x011058
+#define  SPEED_CHANGE  (1 << 24)
 #define  SCRAMBLE_DISABLE  (1 << 27)
+#define MACINTENR  0x01106C
+#define  SPCHGFINE (1 << 4)
+#define MACS2R 0x011078
+#define MACCGSPSETR0x011084
+#define  SPCNGRSN  (1 << 31)
 
 /* R-Car H1 PHY */
 #define H1_PCIEPHYADRR 0x04000c
@@ -385,6 +402,51 @@ static int rcar_pcie_setup(struct list_h
return 1;
 }
 
+void rcar_pcie_force_speedup(struct rcar_pcie *pcie)
+{
+   u32 macsr;
+
+   dev_info(pcie->dev, "Trying speed up to 5 GT/s\n");
+
+   if ((rcar_pci_read_reg(pcie, MACSR) & SPCHGFIN) ||
+   (rcar_pci_read_reg(pcie, MACCTLR) & SPEED_CHANGE)) {
+   dev_err(pcie->dev, "Speed changing is in progress\n");
+   return;
+   }
+
+   if ((rcar_pci_read_reg(pcie, MACSR) & LINK_SPEED) ==
+   LINK_SPEED_5_0GTS) {
+   dev_err(pcie->dev, "Current link speed is already 5 GT/s\n");
+   return;
+   }
+
+   if ((rcar_pci_read_reg(pcie, MACS2R) & LINK_SPEED) !=
+   LINK_SPEED_5_0GTS) {
+   dev_err(pcie->dev,
+   "Current max support link speed not 5 GT/s\n");
+   return;
+   }
+
+   /* Set target link speed to 5.0 GT/s */
+   rcar_rmw32(pcie, EXPCAP(12), PCI_EXP_LNKSTA_CLS,
+  PCI_EXP_LNKSTA_CLS_5_0GB);
+
+   /* Set speed change reason as intentional factor */
+   rcar_rmw32(pcie, MACCGSPSETR, SPCNGRSN, 0);
+
+   /* Clear SPCHGFIN, SPCHGSUC, and SPCHGFAIL */
+   macsr = rcar_pci_read_reg(pcie, MACSR);
+   if (macsr & (SPCHGFIN | SPCHGSUC | SPCHGFAIL))
+   rcar_pci_write_reg(pcie, macsr, MACSR);
+
+   /* Enable interrupt */
+   rcar_rmw32(pcie, MACINTENR, SPCHGFINE, SPCHGFINE);
+   rcar_rmw32(pcie, PCIEINTER, PCIEINTMACE, PCIEINTMACE);
+
+   /* Start link speed change */
+   rcar_rmw32(pcie, MACCTLR, SPEED_CHANGE, SPEED_CHANGE);
+}
+
 static int rcar_pcie_enable(struct rcar_pcie *pcie)
 {
struct pci_bus *bus, *child;
@@ -416,6 +478,9 @@ static int rcar_pcie_enable(struct rcar_
 
pci_bus_add_devices(bus);
 
+   /* Try setting 5 GT/s link speed */
+   rcar_pcie_force_speedup(pcie);
+
return 0;
 }
 
@@ -621,6 +686,44 @@ static irqreturn_t rcar_pcie_msi_irq(int
struct rcar_msi *msi = >msi;
unsigned long reg;
 
+   if (rcar_pci_read_reg(pcie, PCIEINTR) & PCIEINTMAC) {
+   dev_dbg(pcie->dev, "MAC interrupt received\n");
+
+   rcar_rmw32(pcie, MACSR, SPCHGFIN, SPCHGFIN);
+
+   /* Disable this interrupt */
+   rcar_rmw32(pcie, MACINTENR, SPCHGFINE, 0);
+   rcar_rmw32(pcie, PCIEINTER, PCIEINTMACE, 0);
+
+   if (rcar_pci_read_reg(pcie, MACSR) & SPCHGFAIL) {
+   dev_err(pcie->dev, "Speed change