| Message ID | 1450358557-28376-3-git-send-email-phil.edworthy@renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Phil, > + /* Wait until we are in L1 */ > + while (!(val & L1FAEG)) > + val = rcar_pci_read_reg(pcie, PMSR); No timeout? Regards, Wolfram
On Thu, Dec 17, 2015 at 2:30 PM, Wolfram Sang <wsa@the-dreams.de> wrote: >> + /* Wait until we are in L1 */ >> + while (!(val & L1FAEG)) >> + val = rcar_pci_read_reg(pcie, PMSR); > > No timeout? And no cpu_relax() in each iteration. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Wolfram, On 17 December 2015 13:31, Wolfram Sang wrote: > Hi Phil, > > > + /* Wait until we are in L1 */ > > + while (!(val & L1FAEG)) > > + val = rcar_pci_read_reg(pcie, PMSR); > > No timeout? Since the hardware doesn't support hot plug, I believe this loop will always exit very quickly. Unless someone has taken a hammer to the HW of course. However, point taken. I'll add a timeout. > Regards, > > Wolfram -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
T24gMTcgRGVjZW1iZXIgMjAxNSAxMzo0MSwgR2VlcnQgVXl0dGVyaG9ldmVuIHdyb3RlOg0KPiBP biBUaHUsIERlYyAxNywgMjAxNSBhdCAyOjMwIFBNLCBXb2xmcmFtIFNhbmcgPHdzYUB0aGUtZHJl YW1zLmRlPiB3cm90ZToNCj4gPj4gKyAgICAgICAgICAgICAvKiBXYWl0IHVudGlsIHdlIGFyZSBp biBMMSAqLw0KPiA+PiArICAgICAgICAgICAgIHdoaWxlICghKHZhbCAmIEwxRkFFRykpDQo+ID4+ ICsgICAgICAgICAgICAgICAgICAgICB2YWwgPSByY2FyX3BjaV9yZWFkX3JlZyhwY2llLCBQTVNS KTsNCj4gPg0KPiA+IE5vIHRpbWVvdXQ/DQo+IA0KPiBBbmQgbm8gY3B1X3JlbGF4KCkgaW4gZWFj aCBpdGVyYXRpb24uDQpTdXJlLCBJJ2xsIGZpeCB0aGF0Lg0KDQpUaGFua3MNClBoaWwNCg0KIA0K PiBHcntvZXRqZSxlZXRpbmd9cywNCj4gDQo+ICAgICAgICAgICAgICAgICAgICAgICAgIEdlZXJ0 DQo+IA0KPiAtLQ0KPiBHZWVydCBVeXR0ZXJob2V2ZW4gLS0gVGhlcmUncyBsb3RzIG9mIExpbnV4 IGJleW9uZCBpYTMyIC0tIGdlZXJ0QGxpbnV4LW02OGsub3JnDQo+IA0KPiBJbiBwZXJzb25hbCBj b252ZXJzYXRpb25zIHdpdGggdGVjaG5pY2FsIHBlb3BsZSwgSSBjYWxsIG15c2VsZiBhIGhhY2tl ci4gQnV0DQo+IHdoZW4gSSdtIHRhbGtpbmcgdG8gam91cm5hbGlzdHMgSSBqdXN0IHNheSAicHJv Z3JhbW1lciIgb3Igc29tZXRoaW5nIGxpa2UgdGhhdC4NCj4gICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAtLSBMaW51cyBUb3J2YWxkcw0K -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Since the hardware doesn't support hot plug, I believe this loop will > always exit very quickly. Unless someone has taken a hammer to the HW > of course. I know what you mean. But since readl_poll_timeout() makes it easy, we should better be safe than sorry.
Hi Wolfram, On 18 December 2015 14:04, Wolfram Sang wrote: > > Since the hardware doesn't support hot plug, I believe this loop will > > always exit very quickly. Unless someone has taken a hammer to the HW > > of course. > > I know what you mean. But since readl_poll_timeout() makes it easy, we > should better be safe than sorry. I haven't see that one before, very handy! Thanks Phil -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Wolfram, On 18 December 2015 14:04, Wolfram Sang wrote: > > Since the hardware doesn't support hot plug, I believe this loop will > > always exit very quickly. Unless someone has taken a hammer to the HW > > of course. > > I know what you mean. But since readl_poll_timeout() makes it easy, we > should better be safe than sorry. Hmm, I changed the code, but now it doesn't come out of suspend unless sleep_us passed to readl_poll_timeout is 0. Any reason you can think of? To test, I am just using: Build the kernel with CONFIG_PM_DEBUG. echo platform > /sys/power/pm_test echo N > /sys/module/printk/parameters/console_suspend echo mem > /sys/power/state Thanks Phil -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Phil, (this time with full CC list) On Mon, Dec 21, 2015 at 11:52 AM, Phil Edworthy <phil.edworthy@renesas.com> wrote: > On 18 December 2015 14:04, Wolfram Sang wrote: >> > Since the hardware doesn't support hot plug, I believe this loop will >> > always exit very quickly. Unless someone has taken a hammer to the HW >> > of course. >> >> I know what you mean. But since readl_poll_timeout() makes it easy, we >> should better be safe than sorry. > Hmm, I changed the code, but now it doesn't come out of suspend unless > sleep_us passed to readl_poll_timeout is 0. Any reason you can think of? Timers or interrupts disabled? Does the might_sleep_if() scream if CONFIG_DEBUG_ATOMIC_SLEEP=y? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, On 21 December 2015 13:17, Geert Uytterhoeven wrote: > On Mon, Dec 21, 2015 at 11:52 AM, Phil Edworthy > <phil.edworthy@renesas.com> wrote: > > On 18 December 2015 14:04, Wolfram Sang wrote: > >> > Since the hardware doesn't support hot plug, I believe this loop will > >> > always exit very quickly. Unless someone has taken a hammer to the HW > >> > of course. > >> > >> I know what you mean. But since readl_poll_timeout() makes it easy, we > >> should better be safe than sorry. > > Hmm, I changed the code, but now it doesn't come out of suspend unless > > sleep_us passed to readl_poll_timeout is 0. Any reason you can think of? > > Timers or interrupts disabled? > > Does the might_sleep_if() scream if CONFIG_DEBUG_ATOMIC_SLEEP=y? Yes, it does indeed scream. Would you recommend to still use readl_poll_timeout with sleep_us set to 0? Thanks Phil
Hi Phil, On Mon, Jan 4, 2016 at 3:18 PM, Phil Edworthy <phil.edworthy@renesas.com> wrote: > On 21 December 2015 13:17, Geert Uytterhoeven wrote: >> On Mon, Dec 21, 2015 at 11:52 AM, Phil Edworthy >> <phil.edworthy@renesas.com> wrote: >> > On 18 December 2015 14:04, Wolfram Sang wrote: >> >> > Since the hardware doesn't support hot plug, I believe this loop will >> >> > always exit very quickly. Unless someone has taken a hammer to the HW >> >> > of course. >> >> >> >> I know what you mean. But since readl_poll_timeout() makes it easy, we >> >> should better be safe than sorry. >> > Hmm, I changed the code, but now it doesn't come out of suspend unless >> > sleep_us passed to readl_poll_timeout is 0. Any reason you can think of? >> >> Timers or interrupts disabled? >> >> Does the might_sleep_if() scream if CONFIG_DEBUG_ATOMIC_SLEEP=y? > Yes, it does indeed scream. Would you recommend to still use readl_poll_timeout > with sleep_us set to 0? Seems like people hit this before: use readl_poll_timeout_atomic(). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c index c72c0ae..4a4f8e1 100644 --- a/drivers/pci/host/pcie-rcar.c +++ b/drivers/pci/host/pcie-rcar.c @@ -83,6 +83,14 @@ #define MACSR 0x011054 #define MACCTLR 0x011058 #define SCRAMBLE_DISABLE (1 << 27) +#define PMSR 0x01105c +#define L1FAEG (1 << 31) +#define PM_ENTER_L1RX (1 << 23) +#define PMSTATE (7 << 16) +#define PMSTATE_L1 (3 << 16) +#define PMCTLR 0x011060 +#define L1_INIT (1 << 31) + /* R-Car H1 PHY */ #define H1_PCIEPHYADRR 0x04000c @@ -175,6 +183,7 @@ static int rcar_pcie_config_access(struct rcar_pcie *pcie, unsigned int devfn, int where, u32 *data) { int dev, func, reg, index; + u32 val; dev = PCI_SLOT(devfn); func = PCI_FUNC(devfn); @@ -216,6 +225,22 @@ static int rcar_pcie_config_access(struct rcar_pcie *pcie, if (pcie->root_bus_nr < 0) return PCIBIOS_DEVICE_NOT_FOUND; + /* + * If we are not in L1 link state but have received PM_ENTER_L1 DLLP, + * transition to L1 link state. The HW will handle coming out of L1. + */ + val = rcar_pci_read_reg(pcie, PMSR); + if ((val & PM_ENTER_L1RX) && ((val & PMSTATE) != PMSTATE_L1)) { + rcar_pci_write_reg(pcie, L1_INIT, PMCTLR); + + /* Wait until we are in L1 */ + while (!(val & L1FAEG)) + val = rcar_pci_read_reg(pcie, PMSR); + + /* Clear flags indicating link has transitioned to L1 */ + rcar_pci_write_reg(pcie, L1FAEG | PM_ENTER_L1RX, PMSR); + } + /* Clear errors */ rcar_pci_write_reg(pcie, rcar_pci_read_reg(pcie, PCIEERRFR), PCIEERRFR);
The R-Car PCIe host controller does not handle L1 ASPM. Instead, the hardware needs assistance to transition to L1. When the controller has received a PM_ENTER_L1 DLLP, we can't access a card's config regs until we have got it out of L1 link state. The host controller will handle this as long as it has also been transitioned to L1 link state. So, when attempting a config access, check to see if the card has gone into L1, and if so, do the same for the host controller. This is based on a patch by Hien Dang <hien.dang.eb@rvc.renesas.com> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> --- drivers/pci/host/pcie-rcar.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)