From: Marek Vasut <firstname.lastname@example.org> To: "Pali Rohár" <email@example.com>, "Lorenzo Pieralisi" <firstname.lastname@example.org> Cc: Bjorn Helgaas <email@example.com>, firstname.lastname@example.org, Bjorn Helgaas <email@example.com>, Geert Uytterhoeven <firstname.lastname@example.org>, Wolfram Sang <email@example.com>, Yoshihiro Shimoda <firstname.lastname@example.org>, email@example.com Subject: Re: [PATCH V6] PCI: rcar: Add L1 link state fix into data abort hook Date: Mon, 19 Jul 2021 20:39:13 +0200 [thread overview] Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <20210719172340.vvtnddbli2vgxndi@pali> On 7/19/21 7:23 PM, Pali Rohár wrote: [...] >>>> The R-Car PCIe controller is capable of handling L0s/L1 link states. >>>> While the controller can enter and exit L0s link state, and exit L1 >>>> link state, without any additional action from the driver, to enter >>>> L1 link state, the driver must complete the link state transition by >>>> issuing additional commands to the controller. >>>> >>>> The problem is, this transition is not atomic. The controller sets >>>> PMEL1RX bit in PMSR register upon reception of PM_ENTER_L1 DLLP from >>>> the PCIe card, but then the controller enters some sort of inbetween >>>> state. The driver must detect this condition and complete the link >>>> state transition, by setting L1IATN bit in PMCTLR and waiting for >>>> the link state transition to complete. >>>> >>>> If a PCIe access happens inside this window, where the controller >>>> is between L0 and L1 link states, the access generates a fault and >>>> the ARM 'imprecise external abort' handler is invoked. > > And if PCIe MMIO access does not happen, what fixes this issue? Then you have no problem because you don't hit this fault. > In this > patch is implemented only arm32 external abort hook handler (which is > called only when PCIe MMIO access happens and aborts). Yes, for the aarch64 rcar the same fix is implemented in atf (see below). >>>> Just like other PCI controller drivers, here we hook the fault handler, >>>> perform the fixup to help the controller enter L1 link state, and then >>>> restart the instruction which triggered the fault. Since the controller >>>> is in L1 link state now, the link can exit from L1 link state to L0 and >>>> successfully complete the access. > > Link cannot directly goes to L0 from L1. It first goes to Recovery state > and in this state card can "disconnect" or reset... > > What would happen if PCIe MMIO access is issued when link is not in some > L* state? (This can be manually triggered by PCIe Hot Reset - toggling > Secondary Bus Reset bit in Bridge Control register on parent PCIe Bridge > device) Is R-Car working in this case and does not crash? This seems to be exactly the situation the commit message describes -- the controller is stuck between L states and needs manual register write to proceed. [...] >>> To be clear, I'm not objecting to the patch. It's a hardware problem >>> and we should work around it as best we can. > > I'm not sure if current API of hook_fault_code or rather whole usage of > it is prepared to expand into more and more drivers. Last time I looked > at this arm32 part, it was possible to register only one callback from > driver. So extending usage of this hook API can result that two drivers > start fighting who register it earlier... There doesn't seem to be much ongoing HW development on the arm32 r-car, so I don't expect this list of hooks to grow much on this platform. [...]
next prev parent reply other threads:[~2021-07-19 22:04 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-14 20:05 marek.vasut 2021-05-17 7:39 ` Geert Uytterhoeven 2021-07-17 17:33 ` Bjorn Helgaas 2021-07-17 18:14 ` Marek Vasut 2021-07-19 8:59 ` Lorenzo Pieralisi 2021-07-19 15:38 ` Marek Vasut 2021-07-19 17:23 ` Pali Rohár 2021-07-19 18:39 ` Marek Vasut [this message] 2021-07-22 20:31 ` Pali Rohár 2021-07-19 22:06 ` Bjorn Helgaas 2021-07-27 16:11 ` Lorenzo Pieralisi 2021-07-27 16:16 ` Geert Uytterhoeven 2021-07-26 14:47 ` Geert Uytterhoeven 2021-07-26 17:49 ` Bjorn Helgaas 2021-07-27 16:32 ` Lorenzo Pieralisi 2021-08-05 18:30 ` Pali Rohár 2021-07-27 17:08 ` Marek Vasut 2021-08-04 11:06 ` Lorenzo Pieralisi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH V6] PCI: rcar: Add L1 link state fix into data abort hook' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).