* [PATCH] pcie-rcar: try setting PCIe speed to 5 GT/s at boot
@ 2016-09-07 20:22 Sergei Shtylyov
2016-09-13 15:19 ` Simon Horman
2016-09-14 20:54 ` Bjorn Helgaas
0 siblings, 2 replies; 4+ messages in thread
From: Sergei Shtylyov @ 2016-09-07 20:22 UTC (permalink / raw)
To: horms, bhelgaas, linux-pci, linux-renesas-soc
From: Grigory Kletsko <grigory.kletsko@cogentembedded.com>
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 <grigory.kletsko@cogentembedded.com>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
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 CFINIT 1
#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 IDSETR1 0x011004
#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 MACCTLR 0x011058
+#define SPEED_CHANGE (1 << 24)
#define SCRAMBLE_DISABLE (1 << 27)
+#define MACINTENR 0x01106C
+#define SPCHGFINE (1 << 4)
+#define MACS2R 0x011078
+#define MACCGSPSETR 0x011084
+#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 = &pcie->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 failed\n");
+
+ rcar_rmw32(pcie, MACSR, SPCHGFAIL, SPCHGFAIL);
+ /*
+ * TODO: if speed change failed we need to enable
+ * "L0 enter" interrupt and set "speed change disabled"
+ * state. After L0 interrupt rising, we must clear it,
+ * wait for 200 ms and set "speed change enabled" state
+ * according to the R-Car Series, 2nd Generation User's
+ * Manual, section 50.3.9.
+ */
+ return IRQ_HANDLED;
+ }
+
+ if (rcar_pci_read_reg(pcie, MACSR) & SPCHGSUC)
+ rcar_rmw32(pcie, MACSR, SPCHGSUC, SPCHGSUC);
+
+ /* Check speed */
+ if ((rcar_pci_read_reg(pcie, MACSR) & LINK_SPEED) ==
+ LINK_SPEED_5_0GTS)
+ dev_info(pcie->dev, "Current link speed now 5 GT/s\n");
+ else
+ dev_info(pcie->dev,
+ "Current link speed now 2.5 GT/s\n");
+
+ return IRQ_HANDLED;
+ }
+
reg = rcar_pci_read_reg(pcie, PCIEMSIFR);
/* MSI & INTx share an interrupt - we only handle MSI here */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pcie-rcar: try setting PCIe speed to 5 GT/s at boot
2016-09-07 20:22 [PATCH] pcie-rcar: try setting PCIe speed to 5 GT/s at boot Sergei Shtylyov
@ 2016-09-13 15:19 ` Simon Horman
2016-09-14 20:54 ` Bjorn Helgaas
1 sibling, 0 replies; 4+ messages in thread
From: Simon Horman @ 2016-09-13 15:19 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: bhelgaas, linux-pci, linux-renesas-soc
On Wed, Sep 07, 2016 at 11:22:14PM +0300, Sergei Shtylyov wrote:
> From: Grigory Kletsko <grigory.kletsko@cogentembedded.com>
>
> 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 <grigory.kletsko@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Acked-by: Simon Horman <horms+renesas@verge.net.au>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pcie-rcar: try setting PCIe speed to 5 GT/s at boot
2016-09-07 20:22 [PATCH] pcie-rcar: try setting PCIe speed to 5 GT/s at boot Sergei Shtylyov
2016-09-13 15:19 ` Simon Horman
@ 2016-09-14 20:54 ` Bjorn Helgaas
2016-09-20 18:13 ` Sergei Shtylyov
1 sibling, 1 reply; 4+ messages in thread
From: Bjorn Helgaas @ 2016-09-14 20:54 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: horms, bhelgaas, linux-pci, linux-renesas-soc
On Wed, Sep 07, 2016 at 11:22:14PM +0300, Sergei Shtylyov wrote:
> From: Grigory Kletsko <grigory.kletsko@cogentembedded.com>
>
> 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 <grigory.kletsko@cogentembedded.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> 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 CFINIT 1
> #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 IDSETR1 0x011004
> #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 MACCTLR 0x011058
> +#define SPEED_CHANGE (1 << 24)
> #define SCRAMBLE_DISABLE (1 << 27)
> +#define MACINTENR 0x01106C
> +#define SPCHGFINE (1 << 4)
> +#define MACS2R 0x011078
> +#define MACCGSPSETR 0x011084
> +#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");
Are these messages all really errors?
> + 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);
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.
> return 0;
> }
>
> @@ -621,6 +686,44 @@ static irqreturn_t rcar_pcie_msi_irq(int
> struct rcar_msi *msi = &pcie->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?
> +
> + 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 failed\n");
> +
> + rcar_rmw32(pcie, MACSR, SPCHGFAIL, SPCHGFAIL);
> + /*
> + * TODO: if speed change failed we need to enable
> + * "L0 enter" interrupt and set "speed change disabled"
> + * state. After L0 interrupt rising, we must clear it,
> + * wait for 200 ms and set "speed change enabled" state
> + * according to the R-Car Series, 2nd Generation User's
> + * Manual, section 50.3.9.
> + */
> + return IRQ_HANDLED;
> + }
> +
> + if (rcar_pci_read_reg(pcie, MACSR) & SPCHGSUC)
> + rcar_rmw32(pcie, MACSR, SPCHGSUC, SPCHGSUC);
> +
> + /* Check speed */
> + if ((rcar_pci_read_reg(pcie, MACSR) & LINK_SPEED) ==
> + LINK_SPEED_5_0GTS)
> + dev_info(pcie->dev, "Current link speed now 5 GT/s\n");
> + else
> + dev_info(pcie->dev,
> + "Current link speed now 2.5 GT/s\n");
> +
> + return IRQ_HANDLED;
> + }
> +
> 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? If so, it looks like
this comment is now out of date, and possibly the code should be along
these lines in case both MSI and INTx signal an interrupt
simultaneously:
irqreturn_t handled = IRQ_NONE;
reg = rcar_pci_read_reg(pcie, PCIEINTR) & PCIEINTMAC;
if (reg) {
handled = IRQ_HANDLED;
...
}
reg = rcar_pci_read_reg(pcie, PCIEMSIFR);
while (reg) {
handled = IRQ_HANDLED;
...
}
return handled;
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] pcie-rcar: try setting PCIe speed to 5 GT/s at boot
2016-09-14 20:54 ` Bjorn Helgaas
@ 2016-09-20 18:13 ` Sergei Shtylyov
0 siblings, 0 replies; 4+ messages in thread
From: Sergei Shtylyov @ 2016-09-20 18:13 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: horms, bhelgaas, linux-pci, linux-renesas-soc
Hello.
On 09/14/2016 11:54 PM, Bjorn Helgaas wrote:
>> From: Grigory Kletsko <grigory.kletsko@cogentembedded.com>
>>
>> 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 <grigory.kletsko@cogentembedded.com>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> 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 = &pcie->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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-20 18:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-07 20:22 [PATCH] pcie-rcar: try setting PCIe speed to 5 GT/s at boot Sergei Shtylyov
2016-09-13 15:19 ` Simon Horman
2016-09-14 20:54 ` Bjorn Helgaas
2016-09-20 18:13 ` Sergei Shtylyov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.