Message ID | 20220524132827.8837-2-kabel@kernel.org (mailing list archive) |
---|---|
State | Accepted |
Commit | 1326b49636065a29b0aa3832ac963df2a9cece08 |
Headers | show |
Series | PCI: aardvark controller changes BATCH 5 (subset) | expand |
On Tue, May 24, 2022 at 03:28:26PM +0200, Marek Behún wrote: > From: Pali Rohár <pali@kernel.org> > > Aardvark controller supports Advanced Error Reporting configuration > registers. > > Export these registers on the emulated root bridge via the new .read_ext > and .write_ext methods. > > Note that in the Advanced Error Reporting Capability header the offset > to the next Extended Capability header is set, but it is not documented > in Armada 3700 Functional Specification. Since this change adds support > only for Advanced Error Reporting, explicitly clear PCI_EXT_CAP_NEXT > bits in AER capability header. > > Now the pcieport driver correctly detects AER support and allows PCIe > AER driver to start receiving ERR interrupts. Kernel log now says: > > [ 4.358401] pcieport 0000:00:00.0: AER: enabled with IRQ 52 > > Signed-off-by: Pali Rohár <pali@kernel.org> > Signed-off-by: Marek Behún <kabel@kernel.org> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Did you mean Reviewed-by? Signed-off-by is only correct if Lorenzo applied or rewrote these. If he applied them, then Bjorn will pick them up. Rob
On Thu, 26 May 2022 15:38:01 -0500 Rob Herring <robh@kernel.org> wrote: > On Tue, May 24, 2022 at 03:28:26PM +0200, Marek Behún wrote: > > From: Pali Rohár <pali@kernel.org> > > > > Aardvark controller supports Advanced Error Reporting configuration > > registers. > > > > Export these registers on the emulated root bridge via the new .read_ext > > and .write_ext methods. > > > > Note that in the Advanced Error Reporting Capability header the offset > > to the next Extended Capability header is set, but it is not documented > > in Armada 3700 Functional Specification. Since this change adds support > > only for Advanced Error Reporting, explicitly clear PCI_EXT_CAP_NEXT > > bits in AER capability header. > > > > Now the pcieport driver correctly detects AER support and allows PCIe > > AER driver to start receiving ERR interrupts. Kernel log now says: > > > > [ 4.358401] pcieport 0000:00:00.0: AER: enabled with IRQ 52 > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > Signed-off-by: Marek Behún <kabel@kernel.org> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > Did you mean Reviewed-by? Signed-off-by is only correct if Lorenzo > applied or rewrote these. If he applied them, then Bjorn will pick them > up. Hmm. Well, Lorenzo applied the subset I am sending (patches 3 and 5) to his tree, with SOB, meaning to send it to Bjorn [1]. Then we discovered that patch 4 is also required for the _SHIFT macros, which was discussed previously that we want to avoid those, and use FIELD_PREP() / FIELD_GET() instead [2]. So I updated the second patch to use FIELD_PREP() / FIELD_GET() instead of the _SHIFT macros. I guess this version isn't SOB by Lorenzo, but the first version was... I should probably change it to Reviewed-by for both patches anyway, right? Marek [1] https://lore.kernel.org/linux-arm-kernel/165288925279.7950.90687082853412954.b4-ty@arm.com/ [2] https://lore.kernel.org/linux-arm-kernel/20220518202729.GA4606@bhelgaas/
On Sun, May 29, 2022 at 12:08:13PM +0200, Marek Behún wrote: > On Thu, 26 May 2022 15:38:01 -0500 > Rob Herring <robh@kernel.org> wrote: > > > On Tue, May 24, 2022 at 03:28:26PM +0200, Marek Behún wrote: > > > From: Pali Rohár <pali@kernel.org> > > > > > > Aardvark controller supports Advanced Error Reporting configuration > > > registers. > > > > > > Export these registers on the emulated root bridge via the new .read_ext > > > and .write_ext methods. > > > > > > Note that in the Advanced Error Reporting Capability header the offset > > > to the next Extended Capability header is set, but it is not documented > > > in Armada 3700 Functional Specification. Since this change adds support > > > only for Advanced Error Reporting, explicitly clear PCI_EXT_CAP_NEXT > > > bits in AER capability header. > > > > > > Now the pcieport driver correctly detects AER support and allows PCIe > > > AER driver to start receiving ERR interrupts. Kernel log now says: > > > > > > [ 4.358401] pcieport 0000:00:00.0: AER: enabled with IRQ 52 > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > Signed-off-by: Marek Behún <kabel@kernel.org> > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > > Did you mean Reviewed-by? Signed-off-by is only correct if Lorenzo > > applied or rewrote these. If he applied them, then Bjorn will pick them > > up. > > Hmm. Well, Lorenzo applied the subset I am sending (patches 3 and 5) to > his tree, with SOB, meaning to send it to Bjorn [1]. > > Then we discovered that patch 4 is also required for the _SHIFT > macros, which was discussed previously that we want to avoid those, and > use FIELD_PREP() / FIELD_GET() instead [2]. > > So I updated the second patch to use FIELD_PREP() / FIELD_GET() instead > of the _SHIFT macros. I guess this version isn't SOB by Lorenzo, but > the first version was... I should probably change it to Reviewed-by for > both patches anyway, right? I would suggest you send these without either (unless Lorenzo actually gave a Reviewed-by) and just state that Lorenzo applied these, but then you had to make another change as you described above. But if Bjorn applies the original and doesn't want to rebase (he usually will rebase if needed), then an incremental patch will be needed. Rob
diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c index ffec82c8a523..d71c9bc95934 100644 --- a/drivers/pci/controller/pci-aardvark.c +++ b/drivers/pci/controller/pci-aardvark.c @@ -33,6 +33,7 @@ #define PCIE_CORE_CMD_STATUS_REG 0x4 #define PCIE_CORE_DEV_REV_REG 0x8 #define PCIE_CORE_PCIEXP_CAP 0xc0 +#define PCIE_CORE_PCIERR_CAP 0x100 #define PCIE_CORE_ERR_CAPCTL_REG 0x118 #define PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX BIT(5) #define PCIE_CORE_ERR_CAPCTL_ECRC_CHK_TX_EN BIT(6) @@ -944,11 +945,87 @@ advk_pci_bridge_emul_pcie_conf_write(struct pci_bridge_emul *bridge, } } +static pci_bridge_emul_read_status_t +advk_pci_bridge_emul_ext_conf_read(struct pci_bridge_emul *bridge, + int reg, u32 *value) +{ + struct advk_pcie *pcie = bridge->data; + + switch (reg) { + case 0: + *value = advk_readl(pcie, PCIE_CORE_PCIERR_CAP + reg); + /* + * PCI_EXT_CAP_NEXT bits are set to offset 0x150, but Armada + * 3700 Functional Specification does not document registers + * at those addresses. + * Thus we clear PCI_EXT_CAP_NEXT bits to make Advanced Error + * Reporting Capability header the last of Extended + * Capabilities. (If we obtain documentation for those + * registers in the future, this can be changed.) + */ + *value &= 0x000fffff; + return PCI_BRIDGE_EMUL_HANDLED; + + case PCI_ERR_UNCOR_STATUS: + case PCI_ERR_UNCOR_MASK: + case PCI_ERR_UNCOR_SEVER: + case PCI_ERR_COR_STATUS: + case PCI_ERR_COR_MASK: + case PCI_ERR_CAP: + case PCI_ERR_HEADER_LOG + 0: + case PCI_ERR_HEADER_LOG + 4: + case PCI_ERR_HEADER_LOG + 8: + case PCI_ERR_HEADER_LOG + 12: + case PCI_ERR_ROOT_COMMAND: + case PCI_ERR_ROOT_STATUS: + case PCI_ERR_ROOT_ERR_SRC: + *value = advk_readl(pcie, PCIE_CORE_PCIERR_CAP + reg); + return PCI_BRIDGE_EMUL_HANDLED; + + default: + return PCI_BRIDGE_EMUL_NOT_HANDLED; + } +} + +static void +advk_pci_bridge_emul_ext_conf_write(struct pci_bridge_emul *bridge, + int reg, u32 old, u32 new, u32 mask) +{ + struct advk_pcie *pcie = bridge->data; + + switch (reg) { + /* These are W1C registers, so clear other bits */ + case PCI_ERR_UNCOR_STATUS: + case PCI_ERR_COR_STATUS: + case PCI_ERR_ROOT_STATUS: + new &= mask; + fallthrough; + + case PCI_ERR_UNCOR_MASK: + case PCI_ERR_UNCOR_SEVER: + case PCI_ERR_COR_MASK: + case PCI_ERR_CAP: + case PCI_ERR_HEADER_LOG + 0: + case PCI_ERR_HEADER_LOG + 4: + case PCI_ERR_HEADER_LOG + 8: + case PCI_ERR_HEADER_LOG + 12: + case PCI_ERR_ROOT_COMMAND: + case PCI_ERR_ROOT_ERR_SRC: + advk_writel(pcie, new, PCIE_CORE_PCIERR_CAP + reg); + break; + + default: + break; + } +} + static const struct pci_bridge_emul_ops advk_pci_bridge_emul_ops = { .read_base = advk_pci_bridge_emul_base_conf_read, .write_base = advk_pci_bridge_emul_base_conf_write, .read_pcie = advk_pci_bridge_emul_pcie_conf_read, .write_pcie = advk_pci_bridge_emul_pcie_conf_write, + .read_ext = advk_pci_bridge_emul_ext_conf_read, + .write_ext = advk_pci_bridge_emul_ext_conf_write, }; /*