* [PATCH V4 0/4] PCI: Loongson-related pci quirks
@ 2021-06-28 10:10 Huacai Chen
2021-06-28 10:10 ` [PATCH V4 1/4] PCI/portdrv: Don't disable device during shutdown Huacai Chen
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Huacai Chen @ 2021-06-28 10:10 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Xuefeng Li, Huacai Chen, Jiaxun Yang, Huacai Chen,
Tiezhu Yang, Jianmin Lv
This patchset resolve some Loongson-related pci problems (however, some
of them affect not only Loongon platform). The first patch has been sent
before, and nearly all reviewers' questions had been answered at that
time [1]. The second patch move some LS7A quirks to quirks.c, where can
be shared by multi architectures. The third patch improve the mrrs quirk
for LS7A chipset, and the fourth patch add a new quirk for LS7A chipset.
[1] http://patchwork.ozlabs.org/project/linux-pci/patch/1600680138-10949-1-git-send-email-chenhc@lemote.com/
V1 -> V2:
1, Rework the 4th patch;
2, Improve commit messages;
3, Remove the last patch since there is better solutions.
V2 -> V3:
1, Add more affected device ids for the 4th patch;
2, Improve commit messages to describe root causes;
V3 -> V4:
1, Rework the MRRS quirk patch;
2, Improve commit messages to describe root causes, again;
Huacai Chen, Tiezhu Yang and Jianmin Lv(4):
PCI/portdrv: Don't disable device during shutdown.
PCI: Move loongson pci quirks to quirks.c.
PCI: Improve the MRRS quirk for LS7A.
PCI: Add quirk for multifunction devices of LS7A.
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
---
drivers/pci/controller/pci-loongson.c | 69 -----------------------------------
drivers/pci/pci.c | 5 +++
drivers/pci/pcie/portdrv.h | 2 +-
drivers/pci/pcie/portdrv_core.c | 6 ++-
drivers/pci/pcie/portdrv_pci.c | 15 +++++++-
drivers/pci/quirks.c | 64 ++++++++++++++++++++++++++++++++
include/linux/pci.h | 1 +
include/linux/pci_ids.h | 10 +++++
8 files changed, 98 insertions(+), 74 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH V4 1/4] PCI/portdrv: Don't disable device during shutdown
2021-06-28 10:10 [PATCH V4 0/4] PCI: Loongson-related pci quirks Huacai Chen
@ 2021-06-28 10:10 ` Huacai Chen
2021-06-28 10:10 ` [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c Huacai Chen
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Huacai Chen @ 2021-06-28 10:10 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Xuefeng Li, Huacai Chen, Jiaxun Yang, Huacai Chen,
Sinan Kaya, Tiezhu Yang
Use separate remove()/shutdown() callback, and don't disable PCI device
during shutdown. This can avoid some poweroff/reboot failures.
The poweroff/reboot failures could easily be reproduced on Loongson
platforms. I think this is not a Loongson-specific problem, instead, is
a problem related to some specific PCI hosts. On some x86 platforms,
radeon/amdgpu devices can cause the same problem [1][2], and commit
faefba95c9e8ca3a ("drm/amdgpu: just suspend the hw on pci shutdown")
can resolve it.
As Tiezhu said, this occasionally shutdown or reboot failure is due to
clear PCI_COMMAND_MASTER on the device in do_pci_disable_device() [3].
static void do_pci_disable_device(struct pci_dev *dev)
{
u16 pci_command;
pci_read_config_word(dev, PCI_COMMAND, &pci_command);
if (pci_command & PCI_COMMAND_MASTER) {
pci_command &= ~PCI_COMMAND_MASTER;
pci_write_config_word(dev, PCI_COMMAND, pci_command);
}
pcibios_disable_device(dev);
}
When remove "pci_command &= ~PCI_COMMAND_MASTER;", it can work well when
shutdown or reboot. The root cause on Loongson platform is that CPU is
still accessing PCIe devices while poweroff/reboot, and if we disable
the Bus Master Bit at this time, the PCIe controller doesn't forward
requests to downstream devices, and also doesn't send TIMEOUT to CPU,
which causes CPU wait forever (hardware deadlock). This behavior is a
PCIe protocol violation, and will be fixed in new revisions of hardware
(add timeout mechanism for CPU read request, whether or not Bus Master
bit is cleared).
Radeon driver is more difficult than amdgpu due to its confusing symbol
names, and I have maintained an out-of-tree patch for a long time [4].
Recently, we found more and more devices can cause the same problem, and
it is very difficult to modify all problematic drivers as radeon/amdgpu
does (the .shutdown callback should make sure there is no DMA activity).
So, I think modify the PCIe port driver is a simple and effective way.
Because there is no poweroff/reboot problems before cc27b735ad3a75574a6a
("PCI/portdrv: Turn off PCIe services during shutdown"). And as early
discussed, kexec can still work fine after this patch [5].
[1] https://bugs.freedesktop.org/show_bug.cgi?id=97980
[2] https://bugs.freedesktop.org/show_bug.cgi?id=98638
[3] https://lore.kernel.org/patchwork/patch/1305067/
[4] https://github.com/chenhuacai/linux/commit/8da06f9b669831829416a3e9f4d1c57f217a42f0
[5] http://patchwork.ozlabs.org/project/linux-pci/patch/1600680138-10949-1-git-send-email-chenhc@lemote.com/
Cc: Sinan Kaya <okaya@kernel.org>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
drivers/pci/pcie/portdrv.h | 2 +-
drivers/pci/pcie/portdrv_core.c | 6 ++++--
drivers/pci/pcie/portdrv_pci.c | 15 +++++++++++++--
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h
index 2ff5724b8f13..358d7281f6e8 100644
--- a/drivers/pci/pcie/portdrv.h
+++ b/drivers/pci/pcie/portdrv.h
@@ -117,7 +117,7 @@ int pcie_port_device_resume(struct device *dev);
int pcie_port_device_runtime_suspend(struct device *dev);
int pcie_port_device_runtime_resume(struct device *dev);
#endif
-void pcie_port_device_remove(struct pci_dev *dev);
+void pcie_port_device_remove(struct pci_dev *dev, bool disable);
int __must_check pcie_port_bus_register(void);
void pcie_port_bus_unregister(void);
diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index e1fed6649c41..98c0a99a41d6 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -484,11 +484,13 @@ EXPORT_SYMBOL_GPL(pcie_port_find_device);
* Remove PCI Express port service devices associated with given port and
* disable MSI-X or MSI for the port.
*/
-void pcie_port_device_remove(struct pci_dev *dev)
+void pcie_port_device_remove(struct pci_dev *dev, bool disable)
{
device_for_each_child(&dev->dev, NULL, remove_iter);
pci_free_irq_vectors(dev);
- pci_disable_device(dev);
+
+ if (disable)
+ pci_disable_device(dev);
}
/**
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index c7ff1eea225a..562fbf3c1ea9 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -147,7 +147,18 @@ static void pcie_portdrv_remove(struct pci_dev *dev)
pm_runtime_dont_use_autosuspend(&dev->dev);
}
- pcie_port_device_remove(dev);
+ pcie_port_device_remove(dev, true);
+}
+
+static void pcie_portdrv_shutdown(struct pci_dev *dev)
+{
+ if (pci_bridge_d3_possible(dev)) {
+ pm_runtime_forbid(&dev->dev);
+ pm_runtime_get_noresume(&dev->dev);
+ pm_runtime_dont_use_autosuspend(&dev->dev);
+ }
+
+ pcie_port_device_remove(dev, false);
}
static pci_ers_result_t pcie_portdrv_error_detected(struct pci_dev *dev,
@@ -219,7 +230,7 @@ static struct pci_driver pcie_portdriver = {
.probe = pcie_portdrv_probe,
.remove = pcie_portdrv_remove,
- .shutdown = pcie_portdrv_remove,
+ .shutdown = pcie_portdrv_shutdown,
.err_handler = &pcie_portdrv_err_handler,
--
2.27.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c
2021-06-28 10:10 [PATCH V4 0/4] PCI: Loongson-related pci quirks Huacai Chen
2021-06-28 10:10 ` [PATCH V4 1/4] PCI/portdrv: Don't disable device during shutdown Huacai Chen
@ 2021-06-28 10:10 ` Huacai Chen
2021-06-28 10:13 ` Jiaxun Yang
2021-06-28 10:58 ` Jiaxun Yang
2021-06-28 10:10 ` [PATCH V4 3/4] PCI: Improve the MRRS quirk for LS7A Huacai Chen
2021-06-28 10:10 ` [PATCH V4 4/4] PCI: Add quirk for multifunction devices of LS7A Huacai Chen
3 siblings, 2 replies; 16+ messages in thread
From: Huacai Chen @ 2021-06-28 10:10 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Xuefeng Li, Huacai Chen, Jiaxun Yang, Huacai Chen
Loongson PCH (LS7A chipset) will be used by both MIPS-based and
LoongArch-based Loongson processors. MIPS-based Loongson uses FDT
but LoongArch-base Loongson uses ACPI, but the driver in drivers/
pci/controller/pci-loongson.c is FDT-only. So move the quirks to
quirks.c where can be shared by all architectures.
LoongArch is a new RISC ISA, mainline support will come soon, and
documentations are here (in translation):
https://github.com/loongson/LoongArch-Documentation
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
drivers/pci/controller/pci-loongson.c | 69 ---------------------------
drivers/pci/quirks.c | 69 +++++++++++++++++++++++++++
2 files changed, 69 insertions(+), 69 deletions(-)
diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
index 48169b1e3817..88066e9db69e 100644
--- a/drivers/pci/controller/pci-loongson.c
+++ b/drivers/pci/controller/pci-loongson.c
@@ -12,15 +12,6 @@
#include "../pci.h"
-/* Device IDs */
-#define DEV_PCIE_PORT_0 0x7a09
-#define DEV_PCIE_PORT_1 0x7a19
-#define DEV_PCIE_PORT_2 0x7a29
-
-#define DEV_LS2K_APB 0x7a02
-#define DEV_LS7A_CONF 0x7a10
-#define DEV_LS7A_LPC 0x7a0c
-
#define FLAG_CFG0 BIT(0)
#define FLAG_CFG1 BIT(1)
#define FLAG_DEV_FIX BIT(2)
@@ -32,66 +23,6 @@ struct loongson_pci {
u32 flags;
};
-/* Fixup wrong class code in PCIe bridges */
-static void bridge_class_quirk(struct pci_dev *dev)
-{
- dev->class = PCI_CLASS_BRIDGE_PCI << 8;
-}
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
- DEV_PCIE_PORT_0, bridge_class_quirk);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
- DEV_PCIE_PORT_1, bridge_class_quirk);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
- DEV_PCIE_PORT_2, bridge_class_quirk);
-
-static void system_bus_quirk(struct pci_dev *pdev)
-{
- /*
- * The address space consumed by these devices is outside the
- * resources of the host bridge.
- */
- pdev->mmio_always_on = 1;
- pdev->non_compliant_bars = 1;
-}
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
- DEV_LS2K_APB, system_bus_quirk);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
- DEV_LS7A_CONF, system_bus_quirk);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
- DEV_LS7A_LPC, system_bus_quirk);
-
-static void loongson_mrrs_quirk(struct pci_dev *dev)
-{
- struct pci_bus *bus = dev->bus;
- struct pci_dev *bridge;
- static const struct pci_device_id bridge_devids[] = {
- { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
- { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
- { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
- { 0, },
- };
-
- /* look for the matching bridge */
- while (!pci_is_root_bus(bus)) {
- bridge = bus->self;
- bus = bus->parent;
- /*
- * Some Loongson PCIe ports have a h/w limitation of
- * 256 bytes maximum read request size. They can't handle
- * anything larger than this. So force this limit on
- * any devices attached under these ports.
- */
- if (pci_match_id(bridge_devids, bridge)) {
- if (pcie_get_readrq(dev) > 256) {
- pci_info(dev, "limiting MRRS to 256\n");
- pcie_set_readrq(dev, 256);
- }
- break;
- }
- }
-}
-DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
-
static void __iomem *cfg1_map(struct loongson_pci *priv, int bus,
unsigned int devfn, int where)
{
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 22b2bb1109c9..dee4798a49fc 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -205,6 +205,75 @@ static void quirk_mmio_always_on(struct pci_dev *dev)
DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID,
PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on);
+/* Loongson-related quirks */
+#define DEV_PCIE_PORT_0 0x7a09
+#define DEV_PCIE_PORT_1 0x7a19
+#define DEV_PCIE_PORT_2 0x7a29
+
+#define DEV_LS2K_APB 0x7a02
+#define DEV_LS7A_CONF 0x7a10
+#define DEV_LS7A_LPC 0x7a0c
+
+/* Fixup wrong class code in PCIe bridges */
+static void loongson_bridge_class_quirk(struct pci_dev *dev)
+{
+ dev->class = PCI_CLASS_BRIDGE_PCI << 8;
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
+ DEV_PCIE_PORT_0, loongson_bridge_class_quirk);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
+ DEV_PCIE_PORT_1, loongson_bridge_class_quirk);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
+ DEV_PCIE_PORT_2, loongson_bridge_class_quirk);
+
+static void loongson_system_bus_quirk(struct pci_dev *pdev)
+{
+ /*
+ * The address space consumed by these devices is outside the
+ * resources of the host bridge.
+ */
+ pdev->mmio_always_on = 1;
+ pdev->non_compliant_bars = 1;
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
+ DEV_LS2K_APB, loongson_system_bus_quirk);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
+ DEV_LS7A_CONF, loongson_system_bus_quirk);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
+ DEV_LS7A_LPC, loongson_system_bus_quirk);
+
+static void loongson_mrrs_quirk(struct pci_dev *dev)
+{
+ struct pci_bus *bus = dev->bus;
+ struct pci_dev *bridge;
+ static const struct pci_device_id bridge_devids[] = {
+ { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
+ { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
+ { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
+ { 0, },
+ };
+
+ /* look for the matching bridge */
+ while (!pci_is_root_bus(bus)) {
+ bridge = bus->self;
+ bus = bus->parent;
+ /*
+ * Some Loongson PCIe ports have a h/w limitation of
+ * 256 bytes maximum read request size. They can't handle
+ * anything larger than this. So force this limit on
+ * any devices attached under these ports.
+ */
+ if (pci_match_id(bridge_devids, bridge)) {
+ if (pcie_get_readrq(dev) > 256) {
+ pci_info(dev, "limiting MRRS to 256\n");
+ pcie_set_readrq(dev, 256);
+ }
+ break;
+ }
+ }
+}
+DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
+
/*
* The Mellanox Tavor device gives false positive parity errors. Disable
* parity error reporting.
--
2.27.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V4 3/4] PCI: Improve the MRRS quirk for LS7A
2021-06-28 10:10 [PATCH V4 0/4] PCI: Loongson-related pci quirks Huacai Chen
2021-06-28 10:10 ` [PATCH V4 1/4] PCI/portdrv: Don't disable device during shutdown Huacai Chen
2021-06-28 10:10 ` [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c Huacai Chen
@ 2021-06-28 10:10 ` Huacai Chen
2021-06-28 10:59 ` Jiaxun Yang
2021-06-28 22:35 ` Bjorn Helgaas
2021-06-28 10:10 ` [PATCH V4 4/4] PCI: Add quirk for multifunction devices of LS7A Huacai Chen
3 siblings, 2 replies; 16+ messages in thread
From: Huacai Chen @ 2021-06-28 10:10 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Xuefeng Li, Huacai Chen, Jiaxun Yang, Huacai Chen
In new revision of LS7A, some PCIe ports support larger value than 256,
but their maximum supported MRRS values are not detectable. Moreover,
the current loongson_mrrs_quirk() cannot avoid devices increasing its
MRRS after pci_enable_device(), and some devices (e.g. Realtek 8169)
will actually set a big value in its driver. So the only possible way
is configure MRRS of all devices in BIOS, and add a PCI bus flag (i.e.,
PCI_BUS_FLAGS_NO_INC_MRRS) to stop the increasing MRRS operations.
However, according to PCIe Spec, it is legal for an OS to program any
value for MRRS, and it is also legal for an endpoint to generate a Read
Request with any size up to its MRRS. As the hardware engineers say, the
root cause here is LS7A doesn't break up large read requests. In detail,
LS7A PCIe port reports CA (Completer Abort) if it receives a Memory Read
request with a size that's "too big" (Yes, that is a problem in the LS7A
hardware design).
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
drivers/pci/pci.c | 5 +++++
drivers/pci/quirks.c | 41 +++++++++++------------------------------
include/linux/pci.h | 1 +
3 files changed, 17 insertions(+), 30 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8d4ebe095d0c..0f1ff4a6fe44 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5812,6 +5812,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
v = (ffs(rq) - 8) << 12;
+ if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_INC_MRRS) {
+ if (rq > pcie_get_readrq(dev))
+ return -EINVAL;
+ }
+
ret = pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL,
PCI_EXP_DEVCTL_READRQ, v);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index dee4798a49fc..4bbdf5a5425f 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -242,37 +242,18 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
DEV_LS7A_LPC, loongson_system_bus_quirk);
-static void loongson_mrrs_quirk(struct pci_dev *dev)
-{
- struct pci_bus *bus = dev->bus;
- struct pci_dev *bridge;
- static const struct pci_device_id bridge_devids[] = {
- { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
- { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
- { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
- { 0, },
- };
-
- /* look for the matching bridge */
- while (!pci_is_root_bus(bus)) {
- bridge = bus->self;
- bus = bus->parent;
- /*
- * Some Loongson PCIe ports have a h/w limitation of
- * 256 bytes maximum read request size. They can't handle
- * anything larger than this. So force this limit on
- * any devices attached under these ports.
- */
- if (pci_match_id(bridge_devids, bridge)) {
- if (pcie_get_readrq(dev) > 256) {
- pci_info(dev, "limiting MRRS to 256\n");
- pcie_set_readrq(dev, 256);
- }
- break;
- }
- }
+static void loongson_mrrs_quirk(struct pci_dev *pdev)
+{
+ /*
+ * Some Loongson PCIe ports have h/w limitations of maximum read
+ * request size. They can't handle anything larger than this. So
+ * force this limit on any devices attached under these ports.
+ */
+ pdev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_INC_MRRS;
}
-DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
+DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_0, loongson_mrrs_quirk);
+DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_1, loongson_mrrs_quirk);
+DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_2, loongson_mrrs_quirk);
/*
* The Mellanox Tavor device gives false positive parity errors. Disable
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 24306504226a..b336239b5282 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -240,6 +240,7 @@ enum pci_bus_flags {
PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,
PCI_BUS_FLAGS_NO_AERSID = (__force pci_bus_flags_t) 4,
PCI_BUS_FLAGS_NO_EXTCFG = (__force pci_bus_flags_t) 8,
+ PCI_BUS_FLAGS_NO_INC_MRRS = (__force pci_bus_flags_t) 16,
};
/* Values from Link Status register, PCIe r3.1, sec 7.8.8 */
--
2.27.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V4 4/4] PCI: Add quirk for multifunction devices of LS7A
2021-06-28 10:10 [PATCH V4 0/4] PCI: Loongson-related pci quirks Huacai Chen
` (2 preceding siblings ...)
2021-06-28 10:10 ` [PATCH V4 3/4] PCI: Improve the MRRS quirk for LS7A Huacai Chen
@ 2021-06-28 10:10 ` Huacai Chen
2021-06-28 10:59 ` Jiaxun Yang
3 siblings, 1 reply; 16+ messages in thread
From: Huacai Chen @ 2021-06-28 10:10 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Xuefeng Li, Huacai Chen, Jiaxun Yang, Jianmin Lv, Huacai Chen
From: Jianmin Lv <lvjianmin@loongson.cn>
In LS7A, multifunction device use same PCI PIN (because the PIN register
report the same INTx value to each function) but we need different IRQ
for different functions, so add a quirk to fix it for standard PCI PIN
usage.
This patch only affect ACPI based systems (and only needed by ACPI based
systems, too). For DT based systems, the irq mappings is defined in .dts
files and be handled by of_irq_parse_pci().
Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
---
drivers/pci/quirks.c | 14 ++++++++++++++
include/linux/pci_ids.h | 10 ++++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 4bbdf5a5425f..bb5d257f9ccd 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -242,6 +242,20 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
DEV_LS7A_LPC, loongson_system_bus_quirk);
+static void loongson_pci_pin_quirk(struct pci_dev *dev)
+{
+ dev->pin = 1 + (PCI_FUNC(dev->devfn) & 3);
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_0, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_1, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_2, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_AHCI, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_EHCI, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_OHCI, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_DC, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_GPU, loongson_pci_pin_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_GMAC, loongson_pci_pin_quirk);
+
static void loongson_mrrs_quirk(struct pci_dev *pdev)
{
/*
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 4c3fa5293d76..dc024ab21d91 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -151,6 +151,16 @@
/* Vendors and devices. Sort key: vendor first, device next. */
#define PCI_VENDOR_ID_LOONGSON 0x0014
+#define PCI_DEVICE_ID_LOONGSON_APB 0x7a02
+#define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03
+#define PCI_DEVICE_ID_LOONGSON_DC 0x7a06
+#define PCI_DEVICE_ID_LOONGSON_HDA 0x7a07
+#define PCI_DEVICE_ID_LOONGSON_GPU 0x7a15
+#define PCI_DEVICE_ID_LOONGSON_AHCI 0x7a08
+#define PCI_DEVICE_ID_LOONGSON_EHCI 0x7a14
+#define PCI_DEVICE_ID_LOONGSON_OHCI 0x7a24
+#define PCI_DEVICE_ID_LOONGSON_LPC 0x7a0c
+#define PCI_DEVICE_ID_LOONGSON_DMA 0x7a0f
#define PCI_VENDOR_ID_TTTECH 0x0357
#define PCI_DEVICE_ID_TTTECH_MC322 0x000a
--
2.27.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c
2021-06-28 10:10 ` [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c Huacai Chen
@ 2021-06-28 10:13 ` Jiaxun Yang
2021-06-28 10:38 ` Huacai Chen
2021-06-28 10:58 ` Jiaxun Yang
1 sibling, 1 reply; 16+ messages in thread
From: Jiaxun Yang @ 2021-06-28 10:13 UTC (permalink / raw)
To: Huacai Chen, Bjorn Helgaas; +Cc: linux-pci, Xuefeng Li, Huacai Chen
在 2021/6/28 下午6:10, Huacai Chen 写道:
> Loongson PCH (LS7A chipset) will be used by both MIPS-based and
> LoongArch-based Loongson processors. MIPS-based Loongson uses FDT
> but LoongArch-base Loongson uses ACPI, but the driver in drivers/
> pci/controller/pci-loongson.c is FDT-only. So move the quirks to
> quirks.c where can be shared by all architectures.
>
> LoongArch is a new RISC ISA, mainline support will come soon, and
> documentations are here (in translation):
>
> https://github.com/loongson/LoongArch-Documentation
Probably you should guard it with CONFIG_MACH_LOONGSON64 now and add
CONFIG_LOONGARCH
once LOONGARCH code is mainlined.
Thanks.
- Jiaxun
>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> drivers/pci/controller/pci-loongson.c | 69 ---------------------------
> drivers/pci/quirks.c | 69 +++++++++++++++++++++++++++
> 2 files changed, 69 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
> index 48169b1e3817..88066e9db69e 100644
> --- a/drivers/pci/controller/pci-loongson.c
> +++ b/drivers/pci/controller/pci-loongson.c
> @@ -12,15 +12,6 @@
>
> #include "../pci.h"
>
> -/* Device IDs */
> -#define DEV_PCIE_PORT_0 0x7a09
> -#define DEV_PCIE_PORT_1 0x7a19
> -#define DEV_PCIE_PORT_2 0x7a29
> -
> -#define DEV_LS2K_APB 0x7a02
> -#define DEV_LS7A_CONF 0x7a10
> -#define DEV_LS7A_LPC 0x7a0c
> -
> #define FLAG_CFG0 BIT(0)
> #define FLAG_CFG1 BIT(1)
> #define FLAG_DEV_FIX BIT(2)
> @@ -32,66 +23,6 @@ struct loongson_pci {
> u32 flags;
> };
>
> -/* Fixup wrong class code in PCIe bridges */
> -static void bridge_class_quirk(struct pci_dev *dev)
> -{
> - dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> -}
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_PCIE_PORT_0, bridge_class_quirk);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_PCIE_PORT_1, bridge_class_quirk);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_PCIE_PORT_2, bridge_class_quirk);
> -
> -static void system_bus_quirk(struct pci_dev *pdev)
> -{
> - /*
> - * The address space consumed by these devices is outside the
> - * resources of the host bridge.
> - */
> - pdev->mmio_always_on = 1;
> - pdev->non_compliant_bars = 1;
> -}
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_LS2K_APB, system_bus_quirk);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_LS7A_CONF, system_bus_quirk);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_LS7A_LPC, system_bus_quirk);
> -
> -static void loongson_mrrs_quirk(struct pci_dev *dev)
> -{
> - struct pci_bus *bus = dev->bus;
> - struct pci_dev *bridge;
> - static const struct pci_device_id bridge_devids[] = {
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> - { 0, },
> - };
> -
> - /* look for the matching bridge */
> - while (!pci_is_root_bus(bus)) {
> - bridge = bus->self;
> - bus = bus->parent;
> - /*
> - * Some Loongson PCIe ports have a h/w limitation of
> - * 256 bytes maximum read request size. They can't handle
> - * anything larger than this. So force this limit on
> - * any devices attached under these ports.
> - */
> - if (pci_match_id(bridge_devids, bridge)) {
> - if (pcie_get_readrq(dev) > 256) {
> - pci_info(dev, "limiting MRRS to 256\n");
> - pcie_set_readrq(dev, 256);
> - }
> - break;
> - }
> - }
> -}
> -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> -
> static void __iomem *cfg1_map(struct loongson_pci *priv, int bus,
> unsigned int devfn, int where)
> {
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 22b2bb1109c9..dee4798a49fc 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -205,6 +205,75 @@ static void quirk_mmio_always_on(struct pci_dev *dev)
> DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID,
> PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on);
>
> +/* Loongson-related quirks */
> +#define DEV_PCIE_PORT_0 0x7a09
> +#define DEV_PCIE_PORT_1 0x7a19
> +#define DEV_PCIE_PORT_2 0x7a29
> +
> +#define DEV_LS2K_APB 0x7a02
> +#define DEV_LS7A_CONF 0x7a10
> +#define DEV_LS7A_LPC 0x7a0c
> +
> +/* Fixup wrong class code in PCIe bridges */
> +static void loongson_bridge_class_quirk(struct pci_dev *dev)
> +{
> + dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_0, loongson_bridge_class_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_1, loongson_bridge_class_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_2, loongson_bridge_class_quirk);
> +
> +static void loongson_system_bus_quirk(struct pci_dev *pdev)
> +{
> + /*
> + * The address space consumed by these devices is outside the
> + * resources of the host bridge.
> + */
> + pdev->mmio_always_on = 1;
> + pdev->non_compliant_bars = 1;
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS2K_APB, loongson_system_bus_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS7A_CONF, loongson_system_bus_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS7A_LPC, loongson_system_bus_quirk);
> +
> +static void loongson_mrrs_quirk(struct pci_dev *dev)
> +{
> + struct pci_bus *bus = dev->bus;
> + struct pci_dev *bridge;
> + static const struct pci_device_id bridge_devids[] = {
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> + { 0, },
> + };
> +
> + /* look for the matching bridge */
> + while (!pci_is_root_bus(bus)) {
> + bridge = bus->self;
> + bus = bus->parent;
> + /*
> + * Some Loongson PCIe ports have a h/w limitation of
> + * 256 bytes maximum read request size. They can't handle
> + * anything larger than this. So force this limit on
> + * any devices attached under these ports.
> + */
> + if (pci_match_id(bridge_devids, bridge)) {
> + if (pcie_get_readrq(dev) > 256) {
> + pci_info(dev, "limiting MRRS to 256\n");
> + pcie_set_readrq(dev, 256);
> + }
> + break;
> + }
> + }
> +}
> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> +
> /*
> * The Mellanox Tavor device gives false positive parity errors. Disable
> * parity error reporting.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c
2021-06-28 10:13 ` Jiaxun Yang
@ 2021-06-28 10:38 ` Huacai Chen
2021-06-28 10:41 ` Jiaxun Yang
0 siblings, 1 reply; 16+ messages in thread
From: Huacai Chen @ 2021-06-28 10:38 UTC (permalink / raw)
To: Jiaxun Yang; +Cc: Huacai Chen, Bjorn Helgaas, linux-pci, Xuefeng Li
Hi, Jiaxun,
On Mon, Jun 28, 2021 at 6:13 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
>
>
> 在 2021/6/28 下午6:10, Huacai Chen 写道:
> > Loongson PCH (LS7A chipset) will be used by both MIPS-based and
> > LoongArch-based Loongson processors. MIPS-based Loongson uses FDT
> > but LoongArch-base Loongson uses ACPI, but the driver in drivers/
> > pci/controller/pci-loongson.c is FDT-only. So move the quirks to
> > quirks.c where can be shared by all architectures.
> >
> > LoongArch is a new RISC ISA, mainline support will come soon, and
> > documentations are here (in translation):
> >
> > https://github.com/loongson/LoongArch-Documentation
>
> Probably you should guard it with CONFIG_MACH_LOONGSON64 now and add
> CONFIG_LOONGARCH
> once LOONGARCH code is mainlined.
These quirks won't match non-Loongson platforms (because they are
matched by pci ids), so I think that is unnecessary.
Huacai
>
> Thanks.
>
> - Jiaxun
>
> >
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > drivers/pci/controller/pci-loongson.c | 69 ---------------------------
> > drivers/pci/quirks.c | 69 +++++++++++++++++++++++++++
> > 2 files changed, 69 insertions(+), 69 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
> > index 48169b1e3817..88066e9db69e 100644
> > --- a/drivers/pci/controller/pci-loongson.c
> > +++ b/drivers/pci/controller/pci-loongson.c
> > @@ -12,15 +12,6 @@
> >
> > #include "../pci.h"
> >
> > -/* Device IDs */
> > -#define DEV_PCIE_PORT_0 0x7a09
> > -#define DEV_PCIE_PORT_1 0x7a19
> > -#define DEV_PCIE_PORT_2 0x7a29
> > -
> > -#define DEV_LS2K_APB 0x7a02
> > -#define DEV_LS7A_CONF 0x7a10
> > -#define DEV_LS7A_LPC 0x7a0c
> > -
> > #define FLAG_CFG0 BIT(0)
> > #define FLAG_CFG1 BIT(1)
> > #define FLAG_DEV_FIX BIT(2)
> > @@ -32,66 +23,6 @@ struct loongson_pci {
> > u32 flags;
> > };
> >
> > -/* Fixup wrong class code in PCIe bridges */
> > -static void bridge_class_quirk(struct pci_dev *dev)
> > -{
> > - dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> > -}
> > -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > - DEV_PCIE_PORT_0, bridge_class_quirk);
> > -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > - DEV_PCIE_PORT_1, bridge_class_quirk);
> > -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > - DEV_PCIE_PORT_2, bridge_class_quirk);
> > -
> > -static void system_bus_quirk(struct pci_dev *pdev)
> > -{
> > - /*
> > - * The address space consumed by these devices is outside the
> > - * resources of the host bridge.
> > - */
> > - pdev->mmio_always_on = 1;
> > - pdev->non_compliant_bars = 1;
> > -}
> > -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > - DEV_LS2K_APB, system_bus_quirk);
> > -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > - DEV_LS7A_CONF, system_bus_quirk);
> > -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > - DEV_LS7A_LPC, system_bus_quirk);
> > -
> > -static void loongson_mrrs_quirk(struct pci_dev *dev)
> > -{
> > - struct pci_bus *bus = dev->bus;
> > - struct pci_dev *bridge;
> > - static const struct pci_device_id bridge_devids[] = {
> > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> > - { 0, },
> > - };
> > -
> > - /* look for the matching bridge */
> > - while (!pci_is_root_bus(bus)) {
> > - bridge = bus->self;
> > - bus = bus->parent;
> > - /*
> > - * Some Loongson PCIe ports have a h/w limitation of
> > - * 256 bytes maximum read request size. They can't handle
> > - * anything larger than this. So force this limit on
> > - * any devices attached under these ports.
> > - */
> > - if (pci_match_id(bridge_devids, bridge)) {
> > - if (pcie_get_readrq(dev) > 256) {
> > - pci_info(dev, "limiting MRRS to 256\n");
> > - pcie_set_readrq(dev, 256);
> > - }
> > - break;
> > - }
> > - }
> > -}
> > -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> > -
> > static void __iomem *cfg1_map(struct loongson_pci *priv, int bus,
> > unsigned int devfn, int where)
> > {
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 22b2bb1109c9..dee4798a49fc 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -205,6 +205,75 @@ static void quirk_mmio_always_on(struct pci_dev *dev)
> > DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID,
> > PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on);
> >
> > +/* Loongson-related quirks */
> > +#define DEV_PCIE_PORT_0 0x7a09
> > +#define DEV_PCIE_PORT_1 0x7a19
> > +#define DEV_PCIE_PORT_2 0x7a29
> > +
> > +#define DEV_LS2K_APB 0x7a02
> > +#define DEV_LS7A_CONF 0x7a10
> > +#define DEV_LS7A_LPC 0x7a0c
> > +
> > +/* Fixup wrong class code in PCIe bridges */
> > +static void loongson_bridge_class_quirk(struct pci_dev *dev)
> > +{
> > + dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> > +}
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > + DEV_PCIE_PORT_0, loongson_bridge_class_quirk);
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > + DEV_PCIE_PORT_1, loongson_bridge_class_quirk);
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > + DEV_PCIE_PORT_2, loongson_bridge_class_quirk);
> > +
> > +static void loongson_system_bus_quirk(struct pci_dev *pdev)
> > +{
> > + /*
> > + * The address space consumed by these devices is outside the
> > + * resources of the host bridge.
> > + */
> > + pdev->mmio_always_on = 1;
> > + pdev->non_compliant_bars = 1;
> > +}
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > + DEV_LS2K_APB, loongson_system_bus_quirk);
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > + DEV_LS7A_CONF, loongson_system_bus_quirk);
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > + DEV_LS7A_LPC, loongson_system_bus_quirk);
> > +
> > +static void loongson_mrrs_quirk(struct pci_dev *dev)
> > +{
> > + struct pci_bus *bus = dev->bus;
> > + struct pci_dev *bridge;
> > + static const struct pci_device_id bridge_devids[] = {
> > + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> > + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> > + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> > + { 0, },
> > + };
> > +
> > + /* look for the matching bridge */
> > + while (!pci_is_root_bus(bus)) {
> > + bridge = bus->self;
> > + bus = bus->parent;
> > + /*
> > + * Some Loongson PCIe ports have a h/w limitation of
> > + * 256 bytes maximum read request size. They can't handle
> > + * anything larger than this. So force this limit on
> > + * any devices attached under these ports.
> > + */
> > + if (pci_match_id(bridge_devids, bridge)) {
> > + if (pcie_get_readrq(dev) > 256) {
> > + pci_info(dev, "limiting MRRS to 256\n");
> > + pcie_set_readrq(dev, 256);
> > + }
> > + break;
> > + }
> > + }
> > +}
> > +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> > +
> > /*
> > * The Mellanox Tavor device gives false positive parity errors. Disable
> > * parity error reporting.
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c
2021-06-28 10:38 ` Huacai Chen
@ 2021-06-28 10:41 ` Jiaxun Yang
2021-06-28 10:55 ` Huacai Chen
0 siblings, 1 reply; 16+ messages in thread
From: Jiaxun Yang @ 2021-06-28 10:41 UTC (permalink / raw)
To: Huacai Chen; +Cc: Huacai Chen, Bjorn Helgaas, linux-pci, Xuefeng Li
在 2021/6/28 下午6:38, Huacai Chen 写道:
> Hi, Jiaxun,
>
> On Mon, Jun 28, 2021 at 6:13 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>>
>>
>> 在 2021/6/28 下午6:10, Huacai Chen 写道:
>>> Loongson PCH (LS7A chipset) will be used by both MIPS-based and
>>> LoongArch-based Loongson processors. MIPS-based Loongson uses FDT
>>> but LoongArch-base Loongson uses ACPI, but the driver in drivers/
>>> pci/controller/pci-loongson.c is FDT-only. So move the quirks to
>>> quirks.c where can be shared by all architectures.
>>>
>>> LoongArch is a new RISC ISA, mainline support will come soon, and
>>> documentations are here (in translation):
>>>
>>> https://github.com/loongson/LoongArch-Documentation
>> Probably you should guard it with CONFIG_MACH_LOONGSON64 now and add
>> CONFIG_LOONGARCH
>> once LOONGARCH code is mainlined.
> These quirks won't match non-Loongson platforms (because they are
> matched by pci ids), so I think that is unnecessary.
Are you sure?
As I saw
+DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID
It will slow down boot progress on all systems.
Thanks.
- Jiaxun
>
> Huacai
>> Thanks.
>>
>> - Jiaxun
>>
>>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
>>> ---
>>> drivers/pci/controller/pci-loongson.c | 69 ---------------------------
>>> drivers/pci/quirks.c | 69 +++++++++++++++++++++++++++
>>> 2 files changed, 69 insertions(+), 69 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
>>> index 48169b1e3817..88066e9db69e 100644
>>> --- a/drivers/pci/controller/pci-loongson.c
>>> +++ b/drivers/pci/controller/pci-loongson.c
>>> @@ -12,15 +12,6 @@
>>>
>>> #include "../pci.h"
>>>
>>> -/* Device IDs */
>>> -#define DEV_PCIE_PORT_0 0x7a09
>>> -#define DEV_PCIE_PORT_1 0x7a19
>>> -#define DEV_PCIE_PORT_2 0x7a29
>>> -
>>> -#define DEV_LS2K_APB 0x7a02
>>> -#define DEV_LS7A_CONF 0x7a10
>>> -#define DEV_LS7A_LPC 0x7a0c
>>> -
>>> #define FLAG_CFG0 BIT(0)
>>> #define FLAG_CFG1 BIT(1)
>>> #define FLAG_DEV_FIX BIT(2)
>>> @@ -32,66 +23,6 @@ struct loongson_pci {
>>> u32 flags;
>>> };
>>>
>>> -/* Fixup wrong class code in PCIe bridges */
>>> -static void bridge_class_quirk(struct pci_dev *dev)
>>> -{
>>> - dev->class = PCI_CLASS_BRIDGE_PCI << 8;
>>> -}
>>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> - DEV_PCIE_PORT_0, bridge_class_quirk);
>>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> - DEV_PCIE_PORT_1, bridge_class_quirk);
>>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> - DEV_PCIE_PORT_2, bridge_class_quirk);
>>> -
>>> -static void system_bus_quirk(struct pci_dev *pdev)
>>> -{
>>> - /*
>>> - * The address space consumed by these devices is outside the
>>> - * resources of the host bridge.
>>> - */
>>> - pdev->mmio_always_on = 1;
>>> - pdev->non_compliant_bars = 1;
>>> -}
>>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> - DEV_LS2K_APB, system_bus_quirk);
>>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> - DEV_LS7A_CONF, system_bus_quirk);
>>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> - DEV_LS7A_LPC, system_bus_quirk);
>>> -
>>> -static void loongson_mrrs_quirk(struct pci_dev *dev)
>>> -{
>>> - struct pci_bus *bus = dev->bus;
>>> - struct pci_dev *bridge;
>>> - static const struct pci_device_id bridge_devids[] = {
>>> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
>>> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
>>> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
>>> - { 0, },
>>> - };
>>> -
>>> - /* look for the matching bridge */
>>> - while (!pci_is_root_bus(bus)) {
>>> - bridge = bus->self;
>>> - bus = bus->parent;
>>> - /*
>>> - * Some Loongson PCIe ports have a h/w limitation of
>>> - * 256 bytes maximum read request size. They can't handle
>>> - * anything larger than this. So force this limit on
>>> - * any devices attached under these ports.
>>> - */
>>> - if (pci_match_id(bridge_devids, bridge)) {
>>> - if (pcie_get_readrq(dev) > 256) {
>>> - pci_info(dev, "limiting MRRS to 256\n");
>>> - pcie_set_readrq(dev, 256);
>>> - }
>>> - break;
>>> - }
>>> - }
>>> -}
>>> -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
>>> -
>>> static void __iomem *cfg1_map(struct loongson_pci *priv, int bus,
>>> unsigned int devfn, int where)
>>> {
>>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>>> index 22b2bb1109c9..dee4798a49fc 100644
>>> --- a/drivers/pci/quirks.c
>>> +++ b/drivers/pci/quirks.c
>>> @@ -205,6 +205,75 @@ static void quirk_mmio_always_on(struct pci_dev *dev)
>>> DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID,
>>> PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on);
>>>
>>> +/* Loongson-related quirks */
>>> +#define DEV_PCIE_PORT_0 0x7a09
>>> +#define DEV_PCIE_PORT_1 0x7a19
>>> +#define DEV_PCIE_PORT_2 0x7a29
>>> +
>>> +#define DEV_LS2K_APB 0x7a02
>>> +#define DEV_LS7A_CONF 0x7a10
>>> +#define DEV_LS7A_LPC 0x7a0c
>>> +
>>> +/* Fixup wrong class code in PCIe bridges */
>>> +static void loongson_bridge_class_quirk(struct pci_dev *dev)
>>> +{
>>> + dev->class = PCI_CLASS_BRIDGE_PCI << 8;
>>> +}
>>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> + DEV_PCIE_PORT_0, loongson_bridge_class_quirk);
>>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> + DEV_PCIE_PORT_1, loongson_bridge_class_quirk);
>>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> + DEV_PCIE_PORT_2, loongson_bridge_class_quirk);
>>> +
>>> +static void loongson_system_bus_quirk(struct pci_dev *pdev)
>>> +{
>>> + /*
>>> + * The address space consumed by these devices is outside the
>>> + * resources of the host bridge.
>>> + */
>>> + pdev->mmio_always_on = 1;
>>> + pdev->non_compliant_bars = 1;
>>> +}
>>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> + DEV_LS2K_APB, loongson_system_bus_quirk);
>>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> + DEV_LS7A_CONF, loongson_system_bus_quirk);
>>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
>>> + DEV_LS7A_LPC, loongson_system_bus_quirk);
>>> +
>>> +static void loongson_mrrs_quirk(struct pci_dev *dev)
>>> +{
>>> + struct pci_bus *bus = dev->bus;
>>> + struct pci_dev *bridge;
>>> + static const struct pci_device_id bridge_devids[] = {
>>> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
>>> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
>>> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
>>> + { 0, },
>>> + };
>>> +
>>> + /* look for the matching bridge */
>>> + while (!pci_is_root_bus(bus)) {
>>> + bridge = bus->self;
>>> + bus = bus->parent;
>>> + /*
>>> + * Some Loongson PCIe ports have a h/w limitation of
>>> + * 256 bytes maximum read request size. They can't handle
>>> + * anything larger than this. So force this limit on
>>> + * any devices attached under these ports.
>>> + */
>>> + if (pci_match_id(bridge_devids, bridge)) {
>>> + if (pcie_get_readrq(dev) > 256) {
>>> + pci_info(dev, "limiting MRRS to 256\n");
>>> + pcie_set_readrq(dev, 256);
>>> + }
>>> + break;
>>> + }
>>> + }
>>> +}
>>> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
>>> +
>>> /*
>>> * The Mellanox Tavor device gives false positive parity errors. Disable
>>> * parity error reporting.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c
2021-06-28 10:41 ` Jiaxun Yang
@ 2021-06-28 10:55 ` Huacai Chen
2021-06-28 10:57 ` Jiaxun Yang
0 siblings, 1 reply; 16+ messages in thread
From: Huacai Chen @ 2021-06-28 10:55 UTC (permalink / raw)
To: Jiaxun Yang; +Cc: Huacai Chen, Bjorn Helgaas, linux-pci, Xuefeng Li
Hi, Jiaxun,
On Mon, Jun 28, 2021 at 6:41 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>
>
>
> 在 2021/6/28 下午6:38, Huacai Chen 写道:
> > Hi, Jiaxun,
> >
> > On Mon, Jun 28, 2021 at 6:13 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
> >>
> >>
> >> 在 2021/6/28 下午6:10, Huacai Chen 写道:
> >>> Loongson PCH (LS7A chipset) will be used by both MIPS-based and
> >>> LoongArch-based Loongson processors. MIPS-based Loongson uses FDT
> >>> but LoongArch-base Loongson uses ACPI, but the driver in drivers/
> >>> pci/controller/pci-loongson.c is FDT-only. So move the quirks to
> >>> quirks.c where can be shared by all architectures.
> >>>
> >>> LoongArch is a new RISC ISA, mainline support will come soon, and
> >>> documentations are here (in translation):
> >>>
> >>> https://github.com/loongson/LoongArch-Documentation
> >> Probably you should guard it with CONFIG_MACH_LOONGSON64 now and add
> >> CONFIG_LOONGARCH
> >> once LOONGARCH code is mainlined.
> > These quirks won't match non-Loongson platforms (because they are
> > matched by pci ids), so I think that is unnecessary.
> Are you sure?
> As I saw
> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID
> It will slow down boot progress on all systems.
This ANY ID matching is only used by loongson_mrrs_quirk(), but the
next patch will rework loongson_mrrs_quirk().
Huacai
>
> Thanks.
>
> - Jiaxun
> >
> > Huacai
> >> Thanks.
> >>
> >> - Jiaxun
> >>
> >>> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> >>> ---
> >>> drivers/pci/controller/pci-loongson.c | 69 ---------------------------
> >>> drivers/pci/quirks.c | 69 +++++++++++++++++++++++++++
> >>> 2 files changed, 69 insertions(+), 69 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
> >>> index 48169b1e3817..88066e9db69e 100644
> >>> --- a/drivers/pci/controller/pci-loongson.c
> >>> +++ b/drivers/pci/controller/pci-loongson.c
> >>> @@ -12,15 +12,6 @@
> >>>
> >>> #include "../pci.h"
> >>>
> >>> -/* Device IDs */
> >>> -#define DEV_PCIE_PORT_0 0x7a09
> >>> -#define DEV_PCIE_PORT_1 0x7a19
> >>> -#define DEV_PCIE_PORT_2 0x7a29
> >>> -
> >>> -#define DEV_LS2K_APB 0x7a02
> >>> -#define DEV_LS7A_CONF 0x7a10
> >>> -#define DEV_LS7A_LPC 0x7a0c
> >>> -
> >>> #define FLAG_CFG0 BIT(0)
> >>> #define FLAG_CFG1 BIT(1)
> >>> #define FLAG_DEV_FIX BIT(2)
> >>> @@ -32,66 +23,6 @@ struct loongson_pci {
> >>> u32 flags;
> >>> };
> >>>
> >>> -/* Fixup wrong class code in PCIe bridges */
> >>> -static void bridge_class_quirk(struct pci_dev *dev)
> >>> -{
> >>> - dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> >>> -}
> >>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> - DEV_PCIE_PORT_0, bridge_class_quirk);
> >>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> - DEV_PCIE_PORT_1, bridge_class_quirk);
> >>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> - DEV_PCIE_PORT_2, bridge_class_quirk);
> >>> -
> >>> -static void system_bus_quirk(struct pci_dev *pdev)
> >>> -{
> >>> - /*
> >>> - * The address space consumed by these devices is outside the
> >>> - * resources of the host bridge.
> >>> - */
> >>> - pdev->mmio_always_on = 1;
> >>> - pdev->non_compliant_bars = 1;
> >>> -}
> >>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> - DEV_LS2K_APB, system_bus_quirk);
> >>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> - DEV_LS7A_CONF, system_bus_quirk);
> >>> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> - DEV_LS7A_LPC, system_bus_quirk);
> >>> -
> >>> -static void loongson_mrrs_quirk(struct pci_dev *dev)
> >>> -{
> >>> - struct pci_bus *bus = dev->bus;
> >>> - struct pci_dev *bridge;
> >>> - static const struct pci_device_id bridge_devids[] = {
> >>> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> >>> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> >>> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> >>> - { 0, },
> >>> - };
> >>> -
> >>> - /* look for the matching bridge */
> >>> - while (!pci_is_root_bus(bus)) {
> >>> - bridge = bus->self;
> >>> - bus = bus->parent;
> >>> - /*
> >>> - * Some Loongson PCIe ports have a h/w limitation of
> >>> - * 256 bytes maximum read request size. They can't handle
> >>> - * anything larger than this. So force this limit on
> >>> - * any devices attached under these ports.
> >>> - */
> >>> - if (pci_match_id(bridge_devids, bridge)) {
> >>> - if (pcie_get_readrq(dev) > 256) {
> >>> - pci_info(dev, "limiting MRRS to 256\n");
> >>> - pcie_set_readrq(dev, 256);
> >>> - }
> >>> - break;
> >>> - }
> >>> - }
> >>> -}
> >>> -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> >>> -
> >>> static void __iomem *cfg1_map(struct loongson_pci *priv, int bus,
> >>> unsigned int devfn, int where)
> >>> {
> >>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> >>> index 22b2bb1109c9..dee4798a49fc 100644
> >>> --- a/drivers/pci/quirks.c
> >>> +++ b/drivers/pci/quirks.c
> >>> @@ -205,6 +205,75 @@ static void quirk_mmio_always_on(struct pci_dev *dev)
> >>> DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID,
> >>> PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on);
> >>>
> >>> +/* Loongson-related quirks */
> >>> +#define DEV_PCIE_PORT_0 0x7a09
> >>> +#define DEV_PCIE_PORT_1 0x7a19
> >>> +#define DEV_PCIE_PORT_2 0x7a29
> >>> +
> >>> +#define DEV_LS2K_APB 0x7a02
> >>> +#define DEV_LS7A_CONF 0x7a10
> >>> +#define DEV_LS7A_LPC 0x7a0c
> >>> +
> >>> +/* Fixup wrong class code in PCIe bridges */
> >>> +static void loongson_bridge_class_quirk(struct pci_dev *dev)
> >>> +{
> >>> + dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> >>> +}
> >>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> + DEV_PCIE_PORT_0, loongson_bridge_class_quirk);
> >>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> + DEV_PCIE_PORT_1, loongson_bridge_class_quirk);
> >>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> + DEV_PCIE_PORT_2, loongson_bridge_class_quirk);
> >>> +
> >>> +static void loongson_system_bus_quirk(struct pci_dev *pdev)
> >>> +{
> >>> + /*
> >>> + * The address space consumed by these devices is outside the
> >>> + * resources of the host bridge.
> >>> + */
> >>> + pdev->mmio_always_on = 1;
> >>> + pdev->non_compliant_bars = 1;
> >>> +}
> >>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> + DEV_LS2K_APB, loongson_system_bus_quirk);
> >>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> + DEV_LS7A_CONF, loongson_system_bus_quirk);
> >>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> >>> + DEV_LS7A_LPC, loongson_system_bus_quirk);
> >>> +
> >>> +static void loongson_mrrs_quirk(struct pci_dev *dev)
> >>> +{
> >>> + struct pci_bus *bus = dev->bus;
> >>> + struct pci_dev *bridge;
> >>> + static const struct pci_device_id bridge_devids[] = {
> >>> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> >>> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> >>> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> >>> + { 0, },
> >>> + };
> >>> +
> >>> + /* look for the matching bridge */
> >>> + while (!pci_is_root_bus(bus)) {
> >>> + bridge = bus->self;
> >>> + bus = bus->parent;
> >>> + /*
> >>> + * Some Loongson PCIe ports have a h/w limitation of
> >>> + * 256 bytes maximum read request size. They can't handle
> >>> + * anything larger than this. So force this limit on
> >>> + * any devices attached under these ports.
> >>> + */
> >>> + if (pci_match_id(bridge_devids, bridge)) {
> >>> + if (pcie_get_readrq(dev) > 256) {
> >>> + pci_info(dev, "limiting MRRS to 256\n");
> >>> + pcie_set_readrq(dev, 256);
> >>> + }
> >>> + break;
> >>> + }
> >>> + }
> >>> +}
> >>> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> >>> +
> >>> /*
> >>> * The Mellanox Tavor device gives false positive parity errors. Disable
> >>> * parity error reporting.
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c
2021-06-28 10:55 ` Huacai Chen
@ 2021-06-28 10:57 ` Jiaxun Yang
0 siblings, 0 replies; 16+ messages in thread
From: Jiaxun Yang @ 2021-06-28 10:57 UTC (permalink / raw)
To: Huacai Chen; +Cc: Huacai Chen, Bjorn Helgaas, linux-pci, Xuefeng Li
在 2021/6/28 下午6:55, Huacai Chen 写道:
> Hi, Jiaxun,
>
> On Mon, Jun 28, 2021 at 6:41 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>>
>>
>> 在 2021/6/28 下午6:38, Huacai Chen 写道:
>>> Hi, Jiaxun,
>>>
>>> On Mon, Jun 28, 2021 at 6:13 PM Jiaxun Yang <jiaxun.yang@flygoat.com> wrote:
>>>>
>>>> 在 2021/6/28 下午6:10, Huacai Chen 写道:
>>>>> Loongson PCH (LS7A chipset) will be used by both MIPS-based and
>>>>> LoongArch-based Loongson processors. MIPS-based Loongson uses FDT
>>>>> but LoongArch-base Loongson uses ACPI, but the driver in drivers/
>>>>> pci/controller/pci-loongson.c is FDT-only. So move the quirks to
>>>>> quirks.c where can be shared by all architectures.
>>>>>
>>>>> LoongArch is a new RISC ISA, mainline support will come soon, and
>>>>> documentations are here (in translation):
>>>>>
>>>>> https://github.com/loongson/LoongArch-Documentation
>>>> Probably you should guard it with CONFIG_MACH_LOONGSON64 now and add
>>>> CONFIG_LOONGARCH
>>>> once LOONGARCH code is mainlined.
>>> These quirks won't match non-Loongson platforms (because they are
>>> matched by pci ids), so I think that is unnecessary.
>> Are you sure?
>> As I saw
>> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID
>> It will slow down boot progress on all systems.
> This ANY ID matching is only used by loongson_mrrs_quirk(), but the
> next patch will rework loongson_mrrs_quirk().
Oh I see. That seems better.
Thanks.
- Jiaxun
>
> Huacai
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c
2021-06-28 10:10 ` [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c Huacai Chen
2021-06-28 10:13 ` Jiaxun Yang
@ 2021-06-28 10:58 ` Jiaxun Yang
1 sibling, 0 replies; 16+ messages in thread
From: Jiaxun Yang @ 2021-06-28 10:58 UTC (permalink / raw)
To: Huacai Chen, Bjorn Helgaas; +Cc: linux-pci, Xuefeng Li, Huacai Chen
在 2021/6/28 下午6:10, Huacai Chen 写道:
> Loongson PCH (LS7A chipset) will be used by both MIPS-based and
> LoongArch-based Loongson processors. MIPS-based Loongson uses FDT
> but LoongArch-base Loongson uses ACPI, but the driver in drivers/
> pci/controller/pci-loongson.c is FDT-only. So move the quirks to
> quirks.c where can be shared by all architectures.
>
> LoongArch is a new RISC ISA, mainline support will come soon, and
> documentations are here (in translation):
>
> https://github.com/loongson/LoongArch-Documentation
>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Thanks.
> ---
> drivers/pci/controller/pci-loongson.c | 69 ---------------------------
> drivers/pci/quirks.c | 69 +++++++++++++++++++++++++++
> 2 files changed, 69 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
> index 48169b1e3817..88066e9db69e 100644
> --- a/drivers/pci/controller/pci-loongson.c
> +++ b/drivers/pci/controller/pci-loongson.c
> @@ -12,15 +12,6 @@
>
> #include "../pci.h"
>
> -/* Device IDs */
> -#define DEV_PCIE_PORT_0 0x7a09
> -#define DEV_PCIE_PORT_1 0x7a19
> -#define DEV_PCIE_PORT_2 0x7a29
> -
> -#define DEV_LS2K_APB 0x7a02
> -#define DEV_LS7A_CONF 0x7a10
> -#define DEV_LS7A_LPC 0x7a0c
> -
> #define FLAG_CFG0 BIT(0)
> #define FLAG_CFG1 BIT(1)
> #define FLAG_DEV_FIX BIT(2)
> @@ -32,66 +23,6 @@ struct loongson_pci {
> u32 flags;
> };
>
> -/* Fixup wrong class code in PCIe bridges */
> -static void bridge_class_quirk(struct pci_dev *dev)
> -{
> - dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> -}
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_PCIE_PORT_0, bridge_class_quirk);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_PCIE_PORT_1, bridge_class_quirk);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_PCIE_PORT_2, bridge_class_quirk);
> -
> -static void system_bus_quirk(struct pci_dev *pdev)
> -{
> - /*
> - * The address space consumed by these devices is outside the
> - * resources of the host bridge.
> - */
> - pdev->mmio_always_on = 1;
> - pdev->non_compliant_bars = 1;
> -}
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_LS2K_APB, system_bus_quirk);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_LS7A_CONF, system_bus_quirk);
> -DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> - DEV_LS7A_LPC, system_bus_quirk);
> -
> -static void loongson_mrrs_quirk(struct pci_dev *dev)
> -{
> - struct pci_bus *bus = dev->bus;
> - struct pci_dev *bridge;
> - static const struct pci_device_id bridge_devids[] = {
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> - { 0, },
> - };
> -
> - /* look for the matching bridge */
> - while (!pci_is_root_bus(bus)) {
> - bridge = bus->self;
> - bus = bus->parent;
> - /*
> - * Some Loongson PCIe ports have a h/w limitation of
> - * 256 bytes maximum read request size. They can't handle
> - * anything larger than this. So force this limit on
> - * any devices attached under these ports.
> - */
> - if (pci_match_id(bridge_devids, bridge)) {
> - if (pcie_get_readrq(dev) > 256) {
> - pci_info(dev, "limiting MRRS to 256\n");
> - pcie_set_readrq(dev, 256);
> - }
> - break;
> - }
> - }
> -}
> -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> -
> static void __iomem *cfg1_map(struct loongson_pci *priv, int bus,
> unsigned int devfn, int where)
> {
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 22b2bb1109c9..dee4798a49fc 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -205,6 +205,75 @@ static void quirk_mmio_always_on(struct pci_dev *dev)
> DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_ANY_ID, PCI_ANY_ID,
> PCI_CLASS_BRIDGE_HOST, 8, quirk_mmio_always_on);
>
> +/* Loongson-related quirks */
> +#define DEV_PCIE_PORT_0 0x7a09
> +#define DEV_PCIE_PORT_1 0x7a19
> +#define DEV_PCIE_PORT_2 0x7a29
> +
> +#define DEV_LS2K_APB 0x7a02
> +#define DEV_LS7A_CONF 0x7a10
> +#define DEV_LS7A_LPC 0x7a0c
> +
> +/* Fixup wrong class code in PCIe bridges */
> +static void loongson_bridge_class_quirk(struct pci_dev *dev)
> +{
> + dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_0, loongson_bridge_class_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_1, loongson_bridge_class_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_PCIE_PORT_2, loongson_bridge_class_quirk);
> +
> +static void loongson_system_bus_quirk(struct pci_dev *pdev)
> +{
> + /*
> + * The address space consumed by these devices is outside the
> + * resources of the host bridge.
> + */
> + pdev->mmio_always_on = 1;
> + pdev->non_compliant_bars = 1;
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS2K_APB, loongson_system_bus_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS7A_CONF, loongson_system_bus_quirk);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> + DEV_LS7A_LPC, loongson_system_bus_quirk);
> +
> +static void loongson_mrrs_quirk(struct pci_dev *dev)
> +{
> + struct pci_bus *bus = dev->bus;
> + struct pci_dev *bridge;
> + static const struct pci_device_id bridge_devids[] = {
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> + { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> + { 0, },
> + };
> +
> + /* look for the matching bridge */
> + while (!pci_is_root_bus(bus)) {
> + bridge = bus->self;
> + bus = bus->parent;
> + /*
> + * Some Loongson PCIe ports have a h/w limitation of
> + * 256 bytes maximum read request size. They can't handle
> + * anything larger than this. So force this limit on
> + * any devices attached under these ports.
> + */
> + if (pci_match_id(bridge_devids, bridge)) {
> + if (pcie_get_readrq(dev) > 256) {
> + pci_info(dev, "limiting MRRS to 256\n");
> + pcie_set_readrq(dev, 256);
> + }
> + break;
> + }
> + }
> +}
> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> +
> /*
> * The Mellanox Tavor device gives false positive parity errors. Disable
> * parity error reporting.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V4 3/4] PCI: Improve the MRRS quirk for LS7A
2021-06-28 10:10 ` [PATCH V4 3/4] PCI: Improve the MRRS quirk for LS7A Huacai Chen
@ 2021-06-28 10:59 ` Jiaxun Yang
2021-06-28 22:35 ` Bjorn Helgaas
1 sibling, 0 replies; 16+ messages in thread
From: Jiaxun Yang @ 2021-06-28 10:59 UTC (permalink / raw)
To: Huacai Chen, Bjorn Helgaas; +Cc: linux-pci, Xuefeng Li, Huacai Chen
在 2021/6/28 下午6:10, Huacai Chen 写道:
> In new revision of LS7A, some PCIe ports support larger value than 256,
> but their maximum supported MRRS values are not detectable. Moreover,
> the current loongson_mrrs_quirk() cannot avoid devices increasing its
> MRRS after pci_enable_device(), and some devices (e.g. Realtek 8169)
> will actually set a big value in its driver. So the only possible way
> is configure MRRS of all devices in BIOS, and add a PCI bus flag (i.e.,
> PCI_BUS_FLAGS_NO_INC_MRRS) to stop the increasing MRRS operations.
>
> However, according to PCIe Spec, it is legal for an OS to program any
> value for MRRS, and it is also legal for an endpoint to generate a Read
> Request with any size up to its MRRS. As the hardware engineers say, the
> root cause here is LS7A doesn't break up large read requests. In detail,
> LS7A PCIe port reports CA (Completer Abort) if it receives a Memory Read
> request with a size that's "too big" (Yes, that is a problem in the LS7A
> hardware design).
>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> drivers/pci/pci.c | 5 +++++
> drivers/pci/quirks.c | 41 +++++++++++------------------------------
> include/linux/pci.h | 1 +
> 3 files changed, 17 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8d4ebe095d0c..0f1ff4a6fe44 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5812,6 +5812,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
>
> v = (ffs(rq) - 8) << 12;
>
> + if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_INC_MRRS) {
> + if (rq > pcie_get_readrq(dev))
> + return -EINVAL;
> + }
> +
> ret = pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL,
> PCI_EXP_DEVCTL_READRQ, v);
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index dee4798a49fc..4bbdf5a5425f 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -242,37 +242,18 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DEV_LS7A_LPC, loongson_system_bus_quirk);
>
> -static void loongson_mrrs_quirk(struct pci_dev *dev)
> -{
> - struct pci_bus *bus = dev->bus;
> - struct pci_dev *bridge;
> - static const struct pci_device_id bridge_devids[] = {
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> - { 0, },
> - };
> -
> - /* look for the matching bridge */
> - while (!pci_is_root_bus(bus)) {
> - bridge = bus->self;
> - bus = bus->parent;
> - /*
> - * Some Loongson PCIe ports have a h/w limitation of
> - * 256 bytes maximum read request size. They can't handle
> - * anything larger than this. So force this limit on
> - * any devices attached under these ports.
> - */
> - if (pci_match_id(bridge_devids, bridge)) {
> - if (pcie_get_readrq(dev) > 256) {
> - pci_info(dev, "limiting MRRS to 256\n");
> - pcie_set_readrq(dev, 256);
> - }
> - break;
> - }
> - }
> +static void loongson_mrrs_quirk(struct pci_dev *pdev)
> +{
> + /*
> + * Some Loongson PCIe ports have h/w limitations of maximum read
> + * request size. They can't handle anything larger than this. So
> + * force this limit on any devices attached under these ports.
> + */
> + pdev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_INC_MRRS;
> }
> -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_0, loongson_mrrs_quirk);
> +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_1, loongson_mrrs_quirk);
> +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_2, loongson_mrrs_quirk);
>
> /*
> * The Mellanox Tavor device gives false positive parity errors. Disable
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 24306504226a..b336239b5282 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -240,6 +240,7 @@ enum pci_bus_flags {
> PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,
> PCI_BUS_FLAGS_NO_AERSID = (__force pci_bus_flags_t) 4,
> PCI_BUS_FLAGS_NO_EXTCFG = (__force pci_bus_flags_t) 8,
> + PCI_BUS_FLAGS_NO_INC_MRRS = (__force pci_bus_flags_t) 16,
> };
>
> /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V4 4/4] PCI: Add quirk for multifunction devices of LS7A
2021-06-28 10:10 ` [PATCH V4 4/4] PCI: Add quirk for multifunction devices of LS7A Huacai Chen
@ 2021-06-28 10:59 ` Jiaxun Yang
0 siblings, 0 replies; 16+ messages in thread
From: Jiaxun Yang @ 2021-06-28 10:59 UTC (permalink / raw)
To: Huacai Chen, Bjorn Helgaas; +Cc: linux-pci, Xuefeng Li, Huacai Chen, Jianmin Lv
在 2021/6/28 下午6:10, Huacai Chen 写道:
> From: Jianmin Lv <lvjianmin@loongson.cn>
>
> In LS7A, multifunction device use same PCI PIN (because the PIN register
> report the same INTx value to each function) but we need different IRQ
> for different functions, so add a quirk to fix it for standard PCI PIN
> usage.
>
> This patch only affect ACPI based systems (and only needed by ACPI based
> systems, too). For DT based systems, the irq mappings is defined in .dts
> files and be handled by of_irq_parse_pci().
>
> Signed-off-by: Jianmin Lv <lvjianmin@loongson.cn>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> drivers/pci/quirks.c | 14 ++++++++++++++
> include/linux/pci_ids.h | 10 ++++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 4bbdf5a5425f..bb5d257f9ccd 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -242,6 +242,20 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DEV_LS7A_LPC, loongson_system_bus_quirk);
>
> +static void loongson_pci_pin_quirk(struct pci_dev *dev)
> +{
> + dev->pin = 1 + (PCI_FUNC(dev->devfn) & 3);
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_0, loongson_pci_pin_quirk);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_1, loongson_pci_pin_quirk);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_2, loongson_pci_pin_quirk);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_AHCI, loongson_pci_pin_quirk);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_EHCI, loongson_pci_pin_quirk);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_OHCI, loongson_pci_pin_quirk);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_DC, loongson_pci_pin_quirk);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_GPU, loongson_pci_pin_quirk);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, PCI_DEVICE_ID_LOONGSON_GMAC, loongson_pci_pin_quirk);
> +
> static void loongson_mrrs_quirk(struct pci_dev *pdev)
> {
> /*
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 4c3fa5293d76..dc024ab21d91 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -151,6 +151,16 @@
> /* Vendors and devices. Sort key: vendor first, device next. */
>
> #define PCI_VENDOR_ID_LOONGSON 0x0014
> +#define PCI_DEVICE_ID_LOONGSON_APB 0x7a02
> +#define PCI_DEVICE_ID_LOONGSON_GMAC 0x7a03
> +#define PCI_DEVICE_ID_LOONGSON_DC 0x7a06
> +#define PCI_DEVICE_ID_LOONGSON_HDA 0x7a07
> +#define PCI_DEVICE_ID_LOONGSON_GPU 0x7a15
> +#define PCI_DEVICE_ID_LOONGSON_AHCI 0x7a08
> +#define PCI_DEVICE_ID_LOONGSON_EHCI 0x7a14
> +#define PCI_DEVICE_ID_LOONGSON_OHCI 0x7a24
> +#define PCI_DEVICE_ID_LOONGSON_LPC 0x7a0c
> +#define PCI_DEVICE_ID_LOONGSON_DMA 0x7a0f
>
> #define PCI_VENDOR_ID_TTTECH 0x0357
> #define PCI_DEVICE_ID_TTTECH_MC322 0x000a
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V4 3/4] PCI: Improve the MRRS quirk for LS7A
2021-06-28 10:10 ` [PATCH V4 3/4] PCI: Improve the MRRS quirk for LS7A Huacai Chen
2021-06-28 10:59 ` Jiaxun Yang
@ 2021-06-28 22:35 ` Bjorn Helgaas
2021-06-29 3:20 ` Huacai Chen
1 sibling, 1 reply; 16+ messages in thread
From: Bjorn Helgaas @ 2021-06-28 22:35 UTC (permalink / raw)
To: Huacai Chen
Cc: Bjorn Helgaas, linux-pci, Xuefeng Li, Huacai Chen, Jiaxun Yang
On Mon, Jun 28, 2021 at 06:10:26PM +0800, Huacai Chen wrote:
> In new revision of LS7A, some PCIe ports support larger value than 256,
> but their maximum supported MRRS values are not detectable. Moreover,
> the current loongson_mrrs_quirk() cannot avoid devices increasing its
> MRRS after pci_enable_device(), and some devices (e.g. Realtek 8169)
> will actually set a big value in its driver. So the only possible way
> is configure MRRS of all devices in BIOS, and add a PCI bus flag (i.e.,
> PCI_BUS_FLAGS_NO_INC_MRRS) to stop the increasing MRRS operations.
>
> However, according to PCIe Spec, it is legal for an OS to program any
> value for MRRS, and it is also legal for an endpoint to generate a Read
> Request with any size up to its MRRS. As the hardware engineers say, the
> root cause here is LS7A doesn't break up large read requests. In detail,
> LS7A PCIe port reports CA (Completer Abort) if it receives a Memory Read
> request with a size that's "too big" (Yes, that is a problem in the LS7A
> hardware design).
>
> Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> ---
> drivers/pci/pci.c | 5 +++++
> drivers/pci/quirks.c | 41 +++++++++++------------------------------
> include/linux/pci.h | 1 +
> 3 files changed, 17 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8d4ebe095d0c..0f1ff4a6fe44 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5812,6 +5812,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
>
> v = (ffs(rq) - 8) << 12;
>
> + if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_INC_MRRS) {
> + if (rq > pcie_get_readrq(dev))
> + return -EINVAL;
I'd prefer to make this simpler, so we just never touch MRRS at all,
like this:
@@ -5785,6 +5785,9 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
u16 v;
int ret;
+ if (<loongson-quirk>)
+ return -EINVAL;
+
if (rq < 128 || rq > 4096 || !is_power_of_2(rq))
return -EINVAL;
What would that break? It's just harder to analyze the behavior if it
depends on what the driver is trying to do. AFAIK, devices should
*work* correctly with any value of MRRS.
> + }
> +
> ret = pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL,
> PCI_EXP_DEVCTL_READRQ, v);
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index dee4798a49fc..4bbdf5a5425f 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -242,37 +242,18 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> DEV_LS7A_LPC, loongson_system_bus_quirk);
>
> -static void loongson_mrrs_quirk(struct pci_dev *dev)
> -{
> - struct pci_bus *bus = dev->bus;
> - struct pci_dev *bridge;
> - static const struct pci_device_id bridge_devids[] = {
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> - { 0, },
> - };
> -
> - /* look for the matching bridge */
> - while (!pci_is_root_bus(bus)) {
> - bridge = bus->self;
> - bus = bus->parent;
> - /*
> - * Some Loongson PCIe ports have a h/w limitation of
> - * 256 bytes maximum read request size. They can't handle
> - * anything larger than this. So force this limit on
> - * any devices attached under these ports.
> - */
> - if (pci_match_id(bridge_devids, bridge)) {
> - if (pcie_get_readrq(dev) > 256) {
> - pci_info(dev, "limiting MRRS to 256\n");
> - pcie_set_readrq(dev, 256);
> - }
> - break;
> - }
> - }
> +static void loongson_mrrs_quirk(struct pci_dev *pdev)
> +{
> + /*
> + * Some Loongson PCIe ports have h/w limitations of maximum read
> + * request size. They can't handle anything larger than this. So
> + * force this limit on any devices attached under these ports.
> + */
> + pdev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_INC_MRRS;
> }
> -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_0, loongson_mrrs_quirk);
> +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_1, loongson_mrrs_quirk);
> +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_2, loongson_mrrs_quirk);
Thanks for making this quirk Loongson-specific. Can you reverse the
order of patches 2 and 3, so this fix happens before moving the quirk
to drivers/pci/quirks.c?
> /*
> * The Mellanox Tavor device gives false positive parity errors. Disable
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 24306504226a..b336239b5282 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -240,6 +240,7 @@ enum pci_bus_flags {
> PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,
> PCI_BUS_FLAGS_NO_AERSID = (__force pci_bus_flags_t) 4,
> PCI_BUS_FLAGS_NO_EXTCFG = (__force pci_bus_flags_t) 8,
> + PCI_BUS_FLAGS_NO_INC_MRRS = (__force pci_bus_flags_t) 16,
This is not a property of the *bus*.
Apparently it's a property of the Root Port or maybe of the Root
Complex itself. What about RCiePs, which don't have a Root Port above
them? They still have an MRRS field in their Device Control
registers. Are there restrictions on how MRRS can be set for an
RCiEP?
If you need to restrict MRRS for RCiEPs as well as for devices below
LS7A Root Ports, I think setting a bit in struct pci_host_bridge and
using pci_find_host_bridge() would work.
If you don't need to restrict MRRS for RCiEPs (or if there are no
RCiEPs at all) you could put a bit in the struct pci_dev and use
pcie_find_root_port(). But this would consume a bit in *every*
pci_dev on every system, so it's a little more wasteful in that sense.
> };
>
> /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */
> --
> 2.27.0
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V4 3/4] PCI: Improve the MRRS quirk for LS7A
2021-06-28 22:35 ` Bjorn Helgaas
@ 2021-06-29 3:20 ` Huacai Chen
2021-06-29 19:42 ` Bjorn Helgaas
0 siblings, 1 reply; 16+ messages in thread
From: Huacai Chen @ 2021-06-29 3:20 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Huacai Chen, Bjorn Helgaas, linux-pci, Xuefeng Li, Jiaxun Yang
Hi, Bjorn,
On Tue, Jun 29, 2021 at 6:35 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Jun 28, 2021 at 06:10:26PM +0800, Huacai Chen wrote:
> > In new revision of LS7A, some PCIe ports support larger value than 256,
> > but their maximum supported MRRS values are not detectable. Moreover,
> > the current loongson_mrrs_quirk() cannot avoid devices increasing its
> > MRRS after pci_enable_device(), and some devices (e.g. Realtek 8169)
> > will actually set a big value in its driver. So the only possible way
> > is configure MRRS of all devices in BIOS, and add a PCI bus flag (i.e.,
> > PCI_BUS_FLAGS_NO_INC_MRRS) to stop the increasing MRRS operations.
> >
> > However, according to PCIe Spec, it is legal for an OS to program any
> > value for MRRS, and it is also legal for an endpoint to generate a Read
> > Request with any size up to its MRRS. As the hardware engineers say, the
> > root cause here is LS7A doesn't break up large read requests. In detail,
> > LS7A PCIe port reports CA (Completer Abort) if it receives a Memory Read
> > request with a size that's "too big" (Yes, that is a problem in the LS7A
> > hardware design).
> >
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> > drivers/pci/pci.c | 5 +++++
> > drivers/pci/quirks.c | 41 +++++++++++------------------------------
> > include/linux/pci.h | 1 +
> > 3 files changed, 17 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 8d4ebe095d0c..0f1ff4a6fe44 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -5812,6 +5812,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
> >
> > v = (ffs(rq) - 8) << 12;
> >
> > + if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_INC_MRRS) {
> > + if (rq > pcie_get_readrq(dev))
> > + return -EINVAL;
>
> I'd prefer to make this simpler, so we just never touch MRRS at all,
> like this:
>
> @@ -5785,6 +5785,9 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
> u16 v;
> int ret;
>
> + if (<loongson-quirk>)
> + return -EINVAL;
> +
> if (rq < 128 || rq > 4096 || !is_power_of_2(rq))
> return -EINVAL;
>
> What would that break? It's just harder to analyze the behavior if it
> depends on what the driver is trying to do. AFAIK, devices should
> *work* correctly with any value of MRRS.
This disables both increase and decrease MRRS, and so make
pcie_bus_config completely useless. I think allowing decrease MRRS is
useful in the PEER2PEER case.
>
> > + }
> > +
> > ret = pcie_capability_clear_and_set_word(dev, PCI_EXP_DEVCTL,
> > PCI_EXP_DEVCTL_READRQ, v);
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index dee4798a49fc..4bbdf5a5425f 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -242,37 +242,18 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_LOONGSON,
> > DEV_LS7A_LPC, loongson_system_bus_quirk);
> >
> > -static void loongson_mrrs_quirk(struct pci_dev *dev)
> > -{
> > - struct pci_bus *bus = dev->bus;
> > - struct pci_dev *bridge;
> > - static const struct pci_device_id bridge_devids[] = {
> > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_0) },
> > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_1) },
> > - { PCI_VDEVICE(LOONGSON, DEV_PCIE_PORT_2) },
> > - { 0, },
> > - };
> > -
> > - /* look for the matching bridge */
> > - while (!pci_is_root_bus(bus)) {
> > - bridge = bus->self;
> > - bus = bus->parent;
> > - /*
> > - * Some Loongson PCIe ports have a h/w limitation of
> > - * 256 bytes maximum read request size. They can't handle
> > - * anything larger than this. So force this limit on
> > - * any devices attached under these ports.
> > - */
> > - if (pci_match_id(bridge_devids, bridge)) {
> > - if (pcie_get_readrq(dev) > 256) {
> > - pci_info(dev, "limiting MRRS to 256\n");
> > - pcie_set_readrq(dev, 256);
> > - }
> > - break;
> > - }
> > - }
> > +static void loongson_mrrs_quirk(struct pci_dev *pdev)
> > +{
> > + /*
> > + * Some Loongson PCIe ports have h/w limitations of maximum read
> > + * request size. They can't handle anything larger than this. So
> > + * force this limit on any devices attached under these ports.
> > + */
> > + pdev->subordinate->bus_flags |= PCI_BUS_FLAGS_NO_INC_MRRS;
> > }
> > -DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, loongson_mrrs_quirk);
> > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_0, loongson_mrrs_quirk);
> > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_1, loongson_mrrs_quirk);
> > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_LOONGSON, DEV_PCIE_PORT_2, loongson_mrrs_quirk);
>
> Thanks for making this quirk Loongson-specific. Can you reverse the
> order of patches 2 and 3, so this fix happens before moving the quirk
> to drivers/pci/quirks.c?
OK, I will reverse them.
>
> > /*
> > * The Mellanox Tavor device gives false positive parity errors. Disable
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 24306504226a..b336239b5282 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -240,6 +240,7 @@ enum pci_bus_flags {
> > PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,
> > PCI_BUS_FLAGS_NO_AERSID = (__force pci_bus_flags_t) 4,
> > PCI_BUS_FLAGS_NO_EXTCFG = (__force pci_bus_flags_t) 8,
> > + PCI_BUS_FLAGS_NO_INC_MRRS = (__force pci_bus_flags_t) 16,
>
> This is not a property of the *bus*.
>
> Apparently it's a property of the Root Port or maybe of the Root
> Complex itself. What about RCiePs, which don't have a Root Port above
> them? They still have an MRRS field in their Device Control
> registers. Are there restrictions on how MRRS can be set for an
> RCiEP?
>
> If you need to restrict MRRS for RCiEPs as well as for devices below
> LS7A Root Ports, I think setting a bit in struct pci_host_bridge and
> using pci_find_host_bridge() would work.
OK, I will set a bit in pci_host_bridge.
Huacai
>
> If you don't need to restrict MRRS for RCiEPs (or if there are no
> RCiEPs at all) you could put a bit in the struct pci_dev and use
> pcie_find_root_port(). But this would consume a bit in *every*
> pci_dev on every system, so it's a little more wasteful in that sense.
>
> > };
> >
> > /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */
> > --
> > 2.27.0
> >
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V4 3/4] PCI: Improve the MRRS quirk for LS7A
2021-06-29 3:20 ` Huacai Chen
@ 2021-06-29 19:42 ` Bjorn Helgaas
0 siblings, 0 replies; 16+ messages in thread
From: Bjorn Helgaas @ 2021-06-29 19:42 UTC (permalink / raw)
To: Huacai Chen
Cc: Huacai Chen, Bjorn Helgaas, linux-pci, Xuefeng Li, Jiaxun Yang
On Tue, Jun 29, 2021 at 11:20:45AM +0800, Huacai Chen wrote:
> On Tue, Jun 29, 2021 at 6:35 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Jun 28, 2021 at 06:10:26PM +0800, Huacai Chen wrote:
> > > In new revision of LS7A, some PCIe ports support larger value than 256,
> > > but their maximum supported MRRS values are not detectable. Moreover,
> > > the current loongson_mrrs_quirk() cannot avoid devices increasing its
> > > MRRS after pci_enable_device(), and some devices (e.g. Realtek 8169)
> > > will actually set a big value in its driver. So the only possible way
> > > is configure MRRS of all devices in BIOS, and add a PCI bus flag (i.e.,
> > > PCI_BUS_FLAGS_NO_INC_MRRS) to stop the increasing MRRS operations.
> > >
> > > However, according to PCIe Spec, it is legal for an OS to program any
> > > value for MRRS, and it is also legal for an endpoint to generate a Read
> > > Request with any size up to its MRRS. As the hardware engineers say, the
> > > root cause here is LS7A doesn't break up large read requests. In detail,
> > > LS7A PCIe port reports CA (Completer Abort) if it receives a Memory Read
> > > request with a size that's "too big" (Yes, that is a problem in the LS7A
> > > hardware design).
> > >
> > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > > ---
> > > drivers/pci/pci.c | 5 +++++
> > > drivers/pci/quirks.c | 41 +++++++++++------------------------------
> > > include/linux/pci.h | 1 +
> > > 3 files changed, 17 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index 8d4ebe095d0c..0f1ff4a6fe44 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -5812,6 +5812,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
> > >
> > > v = (ffs(rq) - 8) << 12;
> > >
> > > + if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_INC_MRRS) {
> > > + if (rq > pcie_get_readrq(dev))
> > > + return -EINVAL;
> >
> > I'd prefer to make this simpler, so we just never touch MRRS at all,
> > like this:
> >
> > @@ -5785,6 +5785,9 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
> > u16 v;
> > int ret;
> >
> > + if (<loongson-quirk>)
> > + return -EINVAL;
> > +
> > if (rq < 128 || rq > 4096 || !is_power_of_2(rq))
> > return -EINVAL;
> >
> > What would that break? It's just harder to analyze the behavior if it
> > depends on what the driver is trying to do. AFAIK, devices should
> > *work* correctly with any value of MRRS.
>
> This disables both increase and decrease MRRS, and so make
> pcie_bus_config completely useless. I think allowing decrease MRRS is
> useful in the PEER2PEER case.
True. The MPS/MRRS algorithm, including pcie_bus_config, is very
complicated and poorly understood, so I'm a little hesitant to
complicate it even more with one-off quirks. I'd really prefer to
push LS7A off into a corner and have the generic MPS/MRRS algorithm
ignore it completely.
But I guess just keeping pcie_set_readrq() from *increasing* MRRS
doesn't add too much complication and should always be safe.
Bjorn
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-06-29 19:42 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-28 10:10 [PATCH V4 0/4] PCI: Loongson-related pci quirks Huacai Chen
2021-06-28 10:10 ` [PATCH V4 1/4] PCI/portdrv: Don't disable device during shutdown Huacai Chen
2021-06-28 10:10 ` [PATCH V4 2/4] PCI: Move loongson pci quirks to quirks.c Huacai Chen
2021-06-28 10:13 ` Jiaxun Yang
2021-06-28 10:38 ` Huacai Chen
2021-06-28 10:41 ` Jiaxun Yang
2021-06-28 10:55 ` Huacai Chen
2021-06-28 10:57 ` Jiaxun Yang
2021-06-28 10:58 ` Jiaxun Yang
2021-06-28 10:10 ` [PATCH V4 3/4] PCI: Improve the MRRS quirk for LS7A Huacai Chen
2021-06-28 10:59 ` Jiaxun Yang
2021-06-28 22:35 ` Bjorn Helgaas
2021-06-29 3:20 ` Huacai Chen
2021-06-29 19:42 ` Bjorn Helgaas
2021-06-28 10:10 ` [PATCH V4 4/4] PCI: Add quirk for multifunction devices of LS7A Huacai Chen
2021-06-28 10:59 ` Jiaxun Yang
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).