* [PATCH 0/3] CM4 ACPI PCIe quirk
@ 2021-08-05 21:11 ` Jeremy Linton
0 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-05 21:11 UTC (permalink / raw)
To: linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel, Jeremy Linton
The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
standard because its PCIe config space isn't ECAM compliant and is
split into two parts. One part for the root port registers and a
moveable window which points at a given device's 4K config space.
Thus it doesn't have a MCFG (and really any MCFG provided would be
nonsense anyway). As linux doesn't support the PCIe SMCCC standard
we key off a linux specific host bridge _DSD to add custom ECAM
ops and cfgres. The cfg op selects between those two regions, as
well as disallowing problematic accesses, particularly if the link
is down because there isn't an attached device.
Jeremy Linton (3):
PCI: brcmstb: Break register definitions into separate header
PCI: brcmstb: Add ACPI config space quirk
PCI/ACPI: Add new quirk detection, enable bcm2711
drivers/acpi/pci_mcfg.c | 14 ++
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-brcmstb-acpi.c | 77 +++++++++
drivers/pci/controller/pcie-brcmstb.c | 179 +-------------------
drivers/pci/controller/pcie-brcmstb.h | 182 +++++++++++++++++++++
include/linux/pci-ecam.h | 1 +
6 files changed, 276 insertions(+), 178 deletions(-)
create mode 100644 drivers/pci/controller/pcie-brcmstb-acpi.c
create mode 100644 drivers/pci/controller/pcie-brcmstb.h
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 1/3] PCI: brcmstb: Break register definitions into separate header
2021-08-05 21:11 ` Jeremy Linton
@ 2021-08-05 21:11 ` Jeremy Linton
-1 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-05 21:11 UTC (permalink / raw)
To: linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel, Jeremy Linton
We are about to create a standalone ACPI quirk module for the
bcmstb controller. Lets move the register definitions into a separate
file so they can be shared between the APCI quirk and the normal
host bridge driver.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
drivers/pci/controller/pcie-brcmstb.c | 179 +------------------------
drivers/pci/controller/pcie-brcmstb.h | 182 ++++++++++++++++++++++++++
2 files changed, 183 insertions(+), 178 deletions(-)
create mode 100644 drivers/pci/controller/pcie-brcmstb.h
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 08bc788d9422..916de435ced9 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -31,192 +31,15 @@
#include <linux/types.h>
#include "../pci.h"
-
-/* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
-#define BRCM_PCIE_CAP_REGS 0x00ac
-
-/* Broadcom STB PCIe Register Offsets */
-#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1 0x0188
-#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK 0xc
-#define PCIE_RC_CFG_VENDOR_SPCIFIC_REG1_LITTLE_ENDIAN 0x0
-
-#define PCIE_RC_CFG_PRIV1_ID_VAL3 0x043c
-#define PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK 0xffffff
-
-#define PCIE_RC_CFG_PRIV1_LINK_CAPABILITY 0x04dc
-#define PCIE_RC_CFG_PRIV1_LINK_CAPABILITY_ASPM_SUPPORT_MASK 0xc00
-
-#define PCIE_RC_DL_MDIO_ADDR 0x1100
-#define PCIE_RC_DL_MDIO_WR_DATA 0x1104
-#define PCIE_RC_DL_MDIO_RD_DATA 0x1108
-
-#define PCIE_MISC_MISC_CTRL 0x4008
-#define PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK 0x1000
-#define PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK 0x2000
-#define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK 0x300000
-
-#define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK 0xf8000000
-#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK 0x07c00000
-#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK 0x0000001f
-#define SCB_SIZE_MASK(x) PCIE_MISC_MISC_CTRL_SCB ## x ## _SIZE_MASK
-
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO 0x400c
-#define PCIE_MEM_WIN0_LO(win) \
- PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO + ((win) * 8)
-
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI 0x4010
-#define PCIE_MEM_WIN0_HI(win) \
- PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI + ((win) * 8)
-
-#define PCIE_MISC_RC_BAR1_CONFIG_LO 0x402c
-#define PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK 0x1f
-
-#define PCIE_MISC_RC_BAR2_CONFIG_LO 0x4034
-#define PCIE_MISC_RC_BAR2_CONFIG_LO_SIZE_MASK 0x1f
-#define PCIE_MISC_RC_BAR2_CONFIG_HI 0x4038
-
-#define PCIE_MISC_RC_BAR3_CONFIG_LO 0x403c
-#define PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK 0x1f
-
-#define PCIE_MISC_MSI_BAR_CONFIG_LO 0x4044
-#define PCIE_MISC_MSI_BAR_CONFIG_HI 0x4048
-
-#define PCIE_MISC_MSI_DATA_CONFIG 0x404c
-#define PCIE_MISC_MSI_DATA_CONFIG_VAL_32 0xffe06540
-#define PCIE_MISC_MSI_DATA_CONFIG_VAL_8 0xfff86540
-
-#define PCIE_MISC_PCIE_CTRL 0x4064
-#define PCIE_MISC_PCIE_CTRL_PCIE_L23_REQUEST_MASK 0x1
-#define PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_MASK 0x4
-
-#define PCIE_MISC_PCIE_STATUS 0x4068
-#define PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK 0x80
-#define PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK 0x20
-#define PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK 0x10
-#define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK 0x40
-
-#define PCIE_MISC_REVISION 0x406c
-#define BRCM_PCIE_HW_REV_33 0x0303
-#define BRCM_PCIE_HW_REV_3_20 0x0320
-
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT 0x4070
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK 0xfff00000
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK 0xfff0
-#define PCIE_MEM_WIN0_BASE_LIMIT(win) \
- PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT + ((win) * 4)
-
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI 0x4080
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI_BASE_MASK 0xff
-#define PCIE_MEM_WIN0_BASE_HI(win) \
- PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI + ((win) * 8)
-
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI 0x4084
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI_LIMIT_MASK 0xff
-#define PCIE_MEM_WIN0_LIMIT_HI(win) \
- PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI + ((win) * 8)
-
-#define PCIE_MISC_HARD_PCIE_HARD_DEBUG 0x4204
-#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK 0x2
-#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK 0x08000000
-
-
-#define PCIE_INTR2_CPU_BASE 0x4300
-#define PCIE_MSI_INTR2_BASE 0x4500
-/* Offsets from PCIE_INTR2_CPU_BASE and PCIE_MSI_INTR2_BASE */
-#define MSI_INT_STATUS 0x0
-#define MSI_INT_CLR 0x8
-#define MSI_INT_MASK_SET 0x10
-#define MSI_INT_MASK_CLR 0x14
-
-#define PCIE_EXT_CFG_DATA 0x8000
-#define PCIE_EXT_CFG_INDEX 0x9000
-
-#define PCIE_RGR1_SW_INIT_1_PERST_MASK 0x1
-#define PCIE_RGR1_SW_INIT_1_PERST_SHIFT 0x0
-
-#define RGR1_SW_INIT_1_INIT_GENERIC_MASK 0x2
-#define RGR1_SW_INIT_1_INIT_GENERIC_SHIFT 0x1
-#define RGR1_SW_INIT_1_INIT_7278_MASK 0x1
-#define RGR1_SW_INIT_1_INIT_7278_SHIFT 0x0
-
-/* PCIe parameters */
-#define BRCM_NUM_PCIE_OUT_WINS 0x4
-#define BRCM_INT_PCI_MSI_NR 32
-#define BRCM_INT_PCI_MSI_LEGACY_NR 8
-#define BRCM_INT_PCI_MSI_SHIFT 0
-
-/* MSI target adresses */
-#define BRCM_MSI_TARGET_ADDR_LT_4GB 0x0fffffffcULL
-#define BRCM_MSI_TARGET_ADDR_GT_4GB 0xffffffffcULL
-
-/* MDIO registers */
-#define MDIO_PORT0 0x0
-#define MDIO_DATA_MASK 0x7fffffff
-#define MDIO_PORT_MASK 0xf0000
-#define MDIO_REGAD_MASK 0xffff
-#define MDIO_CMD_MASK 0xfff00000
-#define MDIO_CMD_READ 0x1
-#define MDIO_CMD_WRITE 0x0
-#define MDIO_DATA_DONE_MASK 0x80000000
-#define MDIO_RD_DONE(x) (((x) & MDIO_DATA_DONE_MASK) ? 1 : 0)
-#define MDIO_WT_DONE(x) (((x) & MDIO_DATA_DONE_MASK) ? 0 : 1)
-#define SSC_REGS_ADDR 0x1100
-#define SET_ADDR_OFFSET 0x1f
-#define SSC_CNTL_OFFSET 0x2
-#define SSC_CNTL_OVRD_EN_MASK 0x8000
-#define SSC_CNTL_OVRD_VAL_MASK 0x4000
-#define SSC_STATUS_OFFSET 0x1
-#define SSC_STATUS_SSC_MASK 0x400
-#define SSC_STATUS_PLL_LOCK_MASK 0x800
-#define PCIE_BRCM_MAX_MEMC 3
-
-#define IDX_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_INDEX])
-#define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
-#define PCIE_RGR1_SW_INIT_1(pcie) (pcie->reg_offsets[RGR1_SW_INIT_1])
-
-/* Rescal registers */
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL 0xc700
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS 0x3
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_MASK 0x4
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_SHIFT 0x2
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_MASK 0x2
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_SHIFT 0x1
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_MASK 0x1
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_SHIFT 0x0
+#include "pcie-brcmstb.h"
/* Forward declarations */
-struct brcm_pcie;
static inline void brcm_pcie_bridge_sw_init_set_7278(struct brcm_pcie *pcie, u32 val);
static inline void brcm_pcie_bridge_sw_init_set_generic(struct brcm_pcie *pcie, u32 val);
static inline void brcm_pcie_perst_set_4908(struct brcm_pcie *pcie, u32 val);
static inline void brcm_pcie_perst_set_7278(struct brcm_pcie *pcie, u32 val);
static inline void brcm_pcie_perst_set_generic(struct brcm_pcie *pcie, u32 val);
-enum {
- RGR1_SW_INIT_1,
- EXT_CFG_INDEX,
- EXT_CFG_DATA,
-};
-
-enum {
- RGR1_SW_INIT_1_INIT_MASK,
- RGR1_SW_INIT_1_INIT_SHIFT,
-};
-
-enum pcie_type {
- GENERIC,
- BCM4908,
- BCM7278,
- BCM2711,
-};
-
-struct pcie_cfg_data {
- const int *offsets;
- const enum pcie_type type;
- void (*perst_set)(struct brcm_pcie *pcie, u32 val);
- void (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
-};
-
static const int pcie_offsets[] = {
[RGR1_SW_INIT_1] = 0x9210,
[EXT_CFG_INDEX] = 0x9000,
diff --git a/drivers/pci/controller/pcie-brcmstb.h b/drivers/pci/controller/pcie-brcmstb.h
new file mode 100644
index 000000000000..b732a0ec56a0
--- /dev/null
+++ b/drivers/pci/controller/pcie-brcmstb.h
@@ -0,0 +1,182 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (C) 2009 - 2021 Broadcom */
+
+/* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
+#define BRCM_PCIE_CAP_REGS 0x00ac
+
+/* Broadcom STB PCIe Register Offsets */
+#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1 0x0188
+#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK 0xc
+#define PCIE_RC_CFG_VENDOR_SPCIFIC_REG1_LITTLE_ENDIAN 0x0
+
+#define PCIE_RC_CFG_PRIV1_ID_VAL3 0x043c
+#define PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK 0xffffff
+
+#define PCIE_RC_CFG_PRIV1_LINK_CAPABILITY 0x04dc
+#define PCIE_RC_CFG_PRIV1_LINK_CAPABILITY_ASPM_SUPPORT_MASK 0xc00
+
+#define PCIE_RC_DL_MDIO_ADDR 0x1100
+#define PCIE_RC_DL_MDIO_WR_DATA 0x1104
+#define PCIE_RC_DL_MDIO_RD_DATA 0x1108
+
+#define PCIE_MISC_MISC_CTRL 0x4008
+#define PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK 0x1000
+#define PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK 0x2000
+#define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK 0x300000
+
+#define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK 0xf8000000
+#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK 0x07c00000
+#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK 0x0000001f
+#define SCB_SIZE_MASK(x) PCIE_MISC_MISC_CTRL_SCB ## x ## _SIZE_MASK
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO 0x400c
+#define PCIE_MEM_WIN0_LO(win) \
+ PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO + ((win) * 8)
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI 0x4010
+#define PCIE_MEM_WIN0_HI(win) \
+ PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI + ((win) * 8)
+
+#define PCIE_MISC_RC_BAR1_CONFIG_LO 0x402c
+#define PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK 0x1f
+
+#define PCIE_MISC_RC_BAR2_CONFIG_LO 0x4034
+#define PCIE_MISC_RC_BAR2_CONFIG_LO_SIZE_MASK 0x1f
+#define PCIE_MISC_RC_BAR2_CONFIG_HI 0x4038
+
+#define PCIE_MISC_RC_BAR3_CONFIG_LO 0x403c
+#define PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK 0x1f
+
+#define PCIE_MISC_MSI_BAR_CONFIG_LO 0x4044
+#define PCIE_MISC_MSI_BAR_CONFIG_HI 0x4048
+
+#define PCIE_MISC_MSI_DATA_CONFIG 0x404c
+#define PCIE_MISC_MSI_DATA_CONFIG_VAL_32 0xffe06540
+#define PCIE_MISC_MSI_DATA_CONFIG_VAL_8 0xfff86540
+
+#define PCIE_MISC_PCIE_CTRL 0x4064
+#define PCIE_MISC_PCIE_CTRL_PCIE_L23_REQUEST_MASK 0x1
+#define PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_MASK 0x4
+
+#define PCIE_MISC_PCIE_STATUS 0x4068
+#define PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK 0x80
+#define PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK 0x20
+#define PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK 0x10
+#define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK 0x40
+
+#define PCIE_MISC_REVISION 0x406c
+#define BRCM_PCIE_HW_REV_33 0x0303
+#define BRCM_PCIE_HW_REV_3_20 0x0320
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT 0x4070
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK 0xfff00000
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK 0xfff0
+#define PCIE_MEM_WIN0_BASE_LIMIT(win) \
+ PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT + ((win) * 4)
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI 0x4080
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI_BASE_MASK 0xff
+#define PCIE_MEM_WIN0_BASE_HI(win) \
+ PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI + ((win) * 8)
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI 0x4084
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI_LIMIT_MASK 0xff
+#define PCIE_MEM_WIN0_LIMIT_HI(win) \
+ PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI + ((win) * 8)
+
+#define PCIE_MISC_HARD_PCIE_HARD_DEBUG 0x4204
+#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK 0x2
+#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK 0x08000000
+
+
+#define PCIE_INTR2_CPU_BASE 0x4300
+#define PCIE_MSI_INTR2_BASE 0x4500
+/* Offsets from PCIE_INTR2_CPU_BASE and PCIE_MSI_INTR2_BASE */
+#define MSI_INT_STATUS 0x0
+#define MSI_INT_CLR 0x8
+#define MSI_INT_MASK_SET 0x10
+#define MSI_INT_MASK_CLR 0x14
+
+#define PCIE_EXT_CFG_DATA 0x8000
+#define PCIE_EXT_CFG_INDEX 0x9000
+
+#define PCIE_RGR1_SW_INIT_1_PERST_MASK 0x1
+#define PCIE_RGR1_SW_INIT_1_PERST_SHIFT 0x0
+
+#define RGR1_SW_INIT_1_INIT_GENERIC_MASK 0x2
+#define RGR1_SW_INIT_1_INIT_GENERIC_SHIFT 0x1
+#define RGR1_SW_INIT_1_INIT_7278_MASK 0x1
+#define RGR1_SW_INIT_1_INIT_7278_SHIFT 0x0
+
+/* PCIe parameters */
+#define BRCM_NUM_PCIE_OUT_WINS 0x4
+#define BRCM_INT_PCI_MSI_NR 32
+#define BRCM_INT_PCI_MSI_LEGACY_NR 8
+#define BRCM_INT_PCI_MSI_SHIFT 0
+
+/* MSI target addresses */
+#define BRCM_MSI_TARGET_ADDR_LT_4GB 0x0fffffffcULL
+#define BRCM_MSI_TARGET_ADDR_GT_4GB 0xffffffffcULL
+
+/* MDIO registers */
+#define MDIO_PORT0 0x0
+#define MDIO_DATA_MASK 0x7fffffff
+#define MDIO_PORT_MASK 0xf0000
+#define MDIO_REGAD_MASK 0xffff
+#define MDIO_CMD_MASK 0xfff00000
+#define MDIO_CMD_READ 0x1
+#define MDIO_CMD_WRITE 0x0
+#define MDIO_DATA_DONE_MASK 0x80000000
+#define MDIO_RD_DONE(x) (((x) & MDIO_DATA_DONE_MASK) ? 1 : 0)
+#define MDIO_WT_DONE(x) (((x) & MDIO_DATA_DONE_MASK) ? 0 : 1)
+#define SSC_REGS_ADDR 0x1100
+#define SET_ADDR_OFFSET 0x1f
+#define SSC_CNTL_OFFSET 0x2
+#define SSC_CNTL_OVRD_EN_MASK 0x8000
+#define SSC_CNTL_OVRD_VAL_MASK 0x4000
+#define SSC_STATUS_OFFSET 0x1
+#define SSC_STATUS_SSC_MASK 0x400
+#define SSC_STATUS_PLL_LOCK_MASK 0x800
+#define PCIE_BRCM_MAX_MEMC 3
+
+#define IDX_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_INDEX])
+#define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
+#define PCIE_RGR1_SW_INIT_1(pcie) (pcie->reg_offsets[RGR1_SW_INIT_1])
+
+/* Rescal registers */
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL 0xc700
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS 0x3
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_MASK 0x4
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_SHIFT 0x2
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_MASK 0x2
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_SHIFT 0x1
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_MASK 0x1
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_SHIFT 0x0
+
+/* Forward declarations */
+struct brcm_pcie;
+
+enum {
+ RGR1_SW_INIT_1,
+ EXT_CFG_INDEX,
+ EXT_CFG_DATA,
+};
+
+enum {
+ RGR1_SW_INIT_1_INIT_MASK,
+ RGR1_SW_INIT_1_INIT_SHIFT,
+};
+
+enum pcie_type {
+ GENERIC,
+ BCM4908,
+ BCM7278,
+ BCM2711,
+};
+
+struct pcie_cfg_data {
+ const int *offsets;
+ const enum pcie_type type;
+ void (*perst_set)(struct brcm_pcie *pcie, u32 val);
+ void (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
+};
--
2.31.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 1/3] PCI: brcmstb: Break register definitions into separate header
@ 2021-08-05 21:11 ` Jeremy Linton
0 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-05 21:11 UTC (permalink / raw)
To: linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel, Jeremy Linton
We are about to create a standalone ACPI quirk module for the
bcmstb controller. Lets move the register definitions into a separate
file so they can be shared between the APCI quirk and the normal
host bridge driver.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
drivers/pci/controller/pcie-brcmstb.c | 179 +------------------------
drivers/pci/controller/pcie-brcmstb.h | 182 ++++++++++++++++++++++++++
2 files changed, 183 insertions(+), 178 deletions(-)
create mode 100644 drivers/pci/controller/pcie-brcmstb.h
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 08bc788d9422..916de435ced9 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -31,192 +31,15 @@
#include <linux/types.h>
#include "../pci.h"
-
-/* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
-#define BRCM_PCIE_CAP_REGS 0x00ac
-
-/* Broadcom STB PCIe Register Offsets */
-#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1 0x0188
-#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK 0xc
-#define PCIE_RC_CFG_VENDOR_SPCIFIC_REG1_LITTLE_ENDIAN 0x0
-
-#define PCIE_RC_CFG_PRIV1_ID_VAL3 0x043c
-#define PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK 0xffffff
-
-#define PCIE_RC_CFG_PRIV1_LINK_CAPABILITY 0x04dc
-#define PCIE_RC_CFG_PRIV1_LINK_CAPABILITY_ASPM_SUPPORT_MASK 0xc00
-
-#define PCIE_RC_DL_MDIO_ADDR 0x1100
-#define PCIE_RC_DL_MDIO_WR_DATA 0x1104
-#define PCIE_RC_DL_MDIO_RD_DATA 0x1108
-
-#define PCIE_MISC_MISC_CTRL 0x4008
-#define PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK 0x1000
-#define PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK 0x2000
-#define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK 0x300000
-
-#define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK 0xf8000000
-#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK 0x07c00000
-#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK 0x0000001f
-#define SCB_SIZE_MASK(x) PCIE_MISC_MISC_CTRL_SCB ## x ## _SIZE_MASK
-
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO 0x400c
-#define PCIE_MEM_WIN0_LO(win) \
- PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO + ((win) * 8)
-
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI 0x4010
-#define PCIE_MEM_WIN0_HI(win) \
- PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI + ((win) * 8)
-
-#define PCIE_MISC_RC_BAR1_CONFIG_LO 0x402c
-#define PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK 0x1f
-
-#define PCIE_MISC_RC_BAR2_CONFIG_LO 0x4034
-#define PCIE_MISC_RC_BAR2_CONFIG_LO_SIZE_MASK 0x1f
-#define PCIE_MISC_RC_BAR2_CONFIG_HI 0x4038
-
-#define PCIE_MISC_RC_BAR3_CONFIG_LO 0x403c
-#define PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK 0x1f
-
-#define PCIE_MISC_MSI_BAR_CONFIG_LO 0x4044
-#define PCIE_MISC_MSI_BAR_CONFIG_HI 0x4048
-
-#define PCIE_MISC_MSI_DATA_CONFIG 0x404c
-#define PCIE_MISC_MSI_DATA_CONFIG_VAL_32 0xffe06540
-#define PCIE_MISC_MSI_DATA_CONFIG_VAL_8 0xfff86540
-
-#define PCIE_MISC_PCIE_CTRL 0x4064
-#define PCIE_MISC_PCIE_CTRL_PCIE_L23_REQUEST_MASK 0x1
-#define PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_MASK 0x4
-
-#define PCIE_MISC_PCIE_STATUS 0x4068
-#define PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK 0x80
-#define PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK 0x20
-#define PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK 0x10
-#define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK 0x40
-
-#define PCIE_MISC_REVISION 0x406c
-#define BRCM_PCIE_HW_REV_33 0x0303
-#define BRCM_PCIE_HW_REV_3_20 0x0320
-
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT 0x4070
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK 0xfff00000
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK 0xfff0
-#define PCIE_MEM_WIN0_BASE_LIMIT(win) \
- PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT + ((win) * 4)
-
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI 0x4080
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI_BASE_MASK 0xff
-#define PCIE_MEM_WIN0_BASE_HI(win) \
- PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI + ((win) * 8)
-
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI 0x4084
-#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI_LIMIT_MASK 0xff
-#define PCIE_MEM_WIN0_LIMIT_HI(win) \
- PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI + ((win) * 8)
-
-#define PCIE_MISC_HARD_PCIE_HARD_DEBUG 0x4204
-#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK 0x2
-#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK 0x08000000
-
-
-#define PCIE_INTR2_CPU_BASE 0x4300
-#define PCIE_MSI_INTR2_BASE 0x4500
-/* Offsets from PCIE_INTR2_CPU_BASE and PCIE_MSI_INTR2_BASE */
-#define MSI_INT_STATUS 0x0
-#define MSI_INT_CLR 0x8
-#define MSI_INT_MASK_SET 0x10
-#define MSI_INT_MASK_CLR 0x14
-
-#define PCIE_EXT_CFG_DATA 0x8000
-#define PCIE_EXT_CFG_INDEX 0x9000
-
-#define PCIE_RGR1_SW_INIT_1_PERST_MASK 0x1
-#define PCIE_RGR1_SW_INIT_1_PERST_SHIFT 0x0
-
-#define RGR1_SW_INIT_1_INIT_GENERIC_MASK 0x2
-#define RGR1_SW_INIT_1_INIT_GENERIC_SHIFT 0x1
-#define RGR1_SW_INIT_1_INIT_7278_MASK 0x1
-#define RGR1_SW_INIT_1_INIT_7278_SHIFT 0x0
-
-/* PCIe parameters */
-#define BRCM_NUM_PCIE_OUT_WINS 0x4
-#define BRCM_INT_PCI_MSI_NR 32
-#define BRCM_INT_PCI_MSI_LEGACY_NR 8
-#define BRCM_INT_PCI_MSI_SHIFT 0
-
-/* MSI target adresses */
-#define BRCM_MSI_TARGET_ADDR_LT_4GB 0x0fffffffcULL
-#define BRCM_MSI_TARGET_ADDR_GT_4GB 0xffffffffcULL
-
-/* MDIO registers */
-#define MDIO_PORT0 0x0
-#define MDIO_DATA_MASK 0x7fffffff
-#define MDIO_PORT_MASK 0xf0000
-#define MDIO_REGAD_MASK 0xffff
-#define MDIO_CMD_MASK 0xfff00000
-#define MDIO_CMD_READ 0x1
-#define MDIO_CMD_WRITE 0x0
-#define MDIO_DATA_DONE_MASK 0x80000000
-#define MDIO_RD_DONE(x) (((x) & MDIO_DATA_DONE_MASK) ? 1 : 0)
-#define MDIO_WT_DONE(x) (((x) & MDIO_DATA_DONE_MASK) ? 0 : 1)
-#define SSC_REGS_ADDR 0x1100
-#define SET_ADDR_OFFSET 0x1f
-#define SSC_CNTL_OFFSET 0x2
-#define SSC_CNTL_OVRD_EN_MASK 0x8000
-#define SSC_CNTL_OVRD_VAL_MASK 0x4000
-#define SSC_STATUS_OFFSET 0x1
-#define SSC_STATUS_SSC_MASK 0x400
-#define SSC_STATUS_PLL_LOCK_MASK 0x800
-#define PCIE_BRCM_MAX_MEMC 3
-
-#define IDX_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_INDEX])
-#define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
-#define PCIE_RGR1_SW_INIT_1(pcie) (pcie->reg_offsets[RGR1_SW_INIT_1])
-
-/* Rescal registers */
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL 0xc700
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS 0x3
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_MASK 0x4
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_SHIFT 0x2
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_MASK 0x2
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_SHIFT 0x1
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_MASK 0x1
-#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_SHIFT 0x0
+#include "pcie-brcmstb.h"
/* Forward declarations */
-struct brcm_pcie;
static inline void brcm_pcie_bridge_sw_init_set_7278(struct brcm_pcie *pcie, u32 val);
static inline void brcm_pcie_bridge_sw_init_set_generic(struct brcm_pcie *pcie, u32 val);
static inline void brcm_pcie_perst_set_4908(struct brcm_pcie *pcie, u32 val);
static inline void brcm_pcie_perst_set_7278(struct brcm_pcie *pcie, u32 val);
static inline void brcm_pcie_perst_set_generic(struct brcm_pcie *pcie, u32 val);
-enum {
- RGR1_SW_INIT_1,
- EXT_CFG_INDEX,
- EXT_CFG_DATA,
-};
-
-enum {
- RGR1_SW_INIT_1_INIT_MASK,
- RGR1_SW_INIT_1_INIT_SHIFT,
-};
-
-enum pcie_type {
- GENERIC,
- BCM4908,
- BCM7278,
- BCM2711,
-};
-
-struct pcie_cfg_data {
- const int *offsets;
- const enum pcie_type type;
- void (*perst_set)(struct brcm_pcie *pcie, u32 val);
- void (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
-};
-
static const int pcie_offsets[] = {
[RGR1_SW_INIT_1] = 0x9210,
[EXT_CFG_INDEX] = 0x9000,
diff --git a/drivers/pci/controller/pcie-brcmstb.h b/drivers/pci/controller/pcie-brcmstb.h
new file mode 100644
index 000000000000..b732a0ec56a0
--- /dev/null
+++ b/drivers/pci/controller/pcie-brcmstb.h
@@ -0,0 +1,182 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/* Copyright (C) 2009 - 2021 Broadcom */
+
+/* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
+#define BRCM_PCIE_CAP_REGS 0x00ac
+
+/* Broadcom STB PCIe Register Offsets */
+#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1 0x0188
+#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK 0xc
+#define PCIE_RC_CFG_VENDOR_SPCIFIC_REG1_LITTLE_ENDIAN 0x0
+
+#define PCIE_RC_CFG_PRIV1_ID_VAL3 0x043c
+#define PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK 0xffffff
+
+#define PCIE_RC_CFG_PRIV1_LINK_CAPABILITY 0x04dc
+#define PCIE_RC_CFG_PRIV1_LINK_CAPABILITY_ASPM_SUPPORT_MASK 0xc00
+
+#define PCIE_RC_DL_MDIO_ADDR 0x1100
+#define PCIE_RC_DL_MDIO_WR_DATA 0x1104
+#define PCIE_RC_DL_MDIO_RD_DATA 0x1108
+
+#define PCIE_MISC_MISC_CTRL 0x4008
+#define PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK 0x1000
+#define PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK 0x2000
+#define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK 0x300000
+
+#define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK 0xf8000000
+#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK 0x07c00000
+#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK 0x0000001f
+#define SCB_SIZE_MASK(x) PCIE_MISC_MISC_CTRL_SCB ## x ## _SIZE_MASK
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO 0x400c
+#define PCIE_MEM_WIN0_LO(win) \
+ PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO + ((win) * 8)
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI 0x4010
+#define PCIE_MEM_WIN0_HI(win) \
+ PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI + ((win) * 8)
+
+#define PCIE_MISC_RC_BAR1_CONFIG_LO 0x402c
+#define PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK 0x1f
+
+#define PCIE_MISC_RC_BAR2_CONFIG_LO 0x4034
+#define PCIE_MISC_RC_BAR2_CONFIG_LO_SIZE_MASK 0x1f
+#define PCIE_MISC_RC_BAR2_CONFIG_HI 0x4038
+
+#define PCIE_MISC_RC_BAR3_CONFIG_LO 0x403c
+#define PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK 0x1f
+
+#define PCIE_MISC_MSI_BAR_CONFIG_LO 0x4044
+#define PCIE_MISC_MSI_BAR_CONFIG_HI 0x4048
+
+#define PCIE_MISC_MSI_DATA_CONFIG 0x404c
+#define PCIE_MISC_MSI_DATA_CONFIG_VAL_32 0xffe06540
+#define PCIE_MISC_MSI_DATA_CONFIG_VAL_8 0xfff86540
+
+#define PCIE_MISC_PCIE_CTRL 0x4064
+#define PCIE_MISC_PCIE_CTRL_PCIE_L23_REQUEST_MASK 0x1
+#define PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_MASK 0x4
+
+#define PCIE_MISC_PCIE_STATUS 0x4068
+#define PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK 0x80
+#define PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK 0x20
+#define PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK 0x10
+#define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK 0x40
+
+#define PCIE_MISC_REVISION 0x406c
+#define BRCM_PCIE_HW_REV_33 0x0303
+#define BRCM_PCIE_HW_REV_3_20 0x0320
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT 0x4070
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK 0xfff00000
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK 0xfff0
+#define PCIE_MEM_WIN0_BASE_LIMIT(win) \
+ PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT + ((win) * 4)
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI 0x4080
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI_BASE_MASK 0xff
+#define PCIE_MEM_WIN0_BASE_HI(win) \
+ PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI + ((win) * 8)
+
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI 0x4084
+#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI_LIMIT_MASK 0xff
+#define PCIE_MEM_WIN0_LIMIT_HI(win) \
+ PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI + ((win) * 8)
+
+#define PCIE_MISC_HARD_PCIE_HARD_DEBUG 0x4204
+#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK 0x2
+#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK 0x08000000
+
+
+#define PCIE_INTR2_CPU_BASE 0x4300
+#define PCIE_MSI_INTR2_BASE 0x4500
+/* Offsets from PCIE_INTR2_CPU_BASE and PCIE_MSI_INTR2_BASE */
+#define MSI_INT_STATUS 0x0
+#define MSI_INT_CLR 0x8
+#define MSI_INT_MASK_SET 0x10
+#define MSI_INT_MASK_CLR 0x14
+
+#define PCIE_EXT_CFG_DATA 0x8000
+#define PCIE_EXT_CFG_INDEX 0x9000
+
+#define PCIE_RGR1_SW_INIT_1_PERST_MASK 0x1
+#define PCIE_RGR1_SW_INIT_1_PERST_SHIFT 0x0
+
+#define RGR1_SW_INIT_1_INIT_GENERIC_MASK 0x2
+#define RGR1_SW_INIT_1_INIT_GENERIC_SHIFT 0x1
+#define RGR1_SW_INIT_1_INIT_7278_MASK 0x1
+#define RGR1_SW_INIT_1_INIT_7278_SHIFT 0x0
+
+/* PCIe parameters */
+#define BRCM_NUM_PCIE_OUT_WINS 0x4
+#define BRCM_INT_PCI_MSI_NR 32
+#define BRCM_INT_PCI_MSI_LEGACY_NR 8
+#define BRCM_INT_PCI_MSI_SHIFT 0
+
+/* MSI target addresses */
+#define BRCM_MSI_TARGET_ADDR_LT_4GB 0x0fffffffcULL
+#define BRCM_MSI_TARGET_ADDR_GT_4GB 0xffffffffcULL
+
+/* MDIO registers */
+#define MDIO_PORT0 0x0
+#define MDIO_DATA_MASK 0x7fffffff
+#define MDIO_PORT_MASK 0xf0000
+#define MDIO_REGAD_MASK 0xffff
+#define MDIO_CMD_MASK 0xfff00000
+#define MDIO_CMD_READ 0x1
+#define MDIO_CMD_WRITE 0x0
+#define MDIO_DATA_DONE_MASK 0x80000000
+#define MDIO_RD_DONE(x) (((x) & MDIO_DATA_DONE_MASK) ? 1 : 0)
+#define MDIO_WT_DONE(x) (((x) & MDIO_DATA_DONE_MASK) ? 0 : 1)
+#define SSC_REGS_ADDR 0x1100
+#define SET_ADDR_OFFSET 0x1f
+#define SSC_CNTL_OFFSET 0x2
+#define SSC_CNTL_OVRD_EN_MASK 0x8000
+#define SSC_CNTL_OVRD_VAL_MASK 0x4000
+#define SSC_STATUS_OFFSET 0x1
+#define SSC_STATUS_SSC_MASK 0x400
+#define SSC_STATUS_PLL_LOCK_MASK 0x800
+#define PCIE_BRCM_MAX_MEMC 3
+
+#define IDX_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_INDEX])
+#define DATA_ADDR(pcie) (pcie->reg_offsets[EXT_CFG_DATA])
+#define PCIE_RGR1_SW_INIT_1(pcie) (pcie->reg_offsets[RGR1_SW_INIT_1])
+
+/* Rescal registers */
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL 0xc700
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_NFLDS 0x3
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_MASK 0x4
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_DIG_RESET_SHIFT 0x2
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_MASK 0x2
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_RESET_SHIFT 0x1
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_MASK 0x1
+#define PCIE_DVT_PMU_PCIE_PHY_CTRL_DAST_PWRDN_SHIFT 0x0
+
+/* Forward declarations */
+struct brcm_pcie;
+
+enum {
+ RGR1_SW_INIT_1,
+ EXT_CFG_INDEX,
+ EXT_CFG_DATA,
+};
+
+enum {
+ RGR1_SW_INIT_1_INIT_MASK,
+ RGR1_SW_INIT_1_INIT_SHIFT,
+};
+
+enum pcie_type {
+ GENERIC,
+ BCM4908,
+ BCM7278,
+ BCM2711,
+};
+
+struct pcie_cfg_data {
+ const int *offsets;
+ const enum pcie_type type;
+ void (*perst_set)(struct brcm_pcie *pcie, u32 val);
+ void (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
+};
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH 1/3] PCI: brcmstb: Break register definitions into separate header
2021-08-05 21:11 ` Jeremy Linton
@ 2021-08-10 10:07 ` Florian Fainelli
-1 siblings, 0 replies; 42+ messages in thread
From: Florian Fainelli @ 2021-08-10 10:07 UTC (permalink / raw)
To: Jeremy Linton, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
On 8/5/2021 2:11 PM, Jeremy Linton wrote:
> We are about to create a standalone ACPI quirk module for the
> bcmstb controller. Lets move the register definitions into a separate
> file so they can be shared between the APCI quirk and the normal
> host bridge driver.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> drivers/pci/controller/pcie-brcmstb.c | 179 +------------------------
> drivers/pci/controller/pcie-brcmstb.h | 182 ++++++++++++++++++++++++++
> 2 files changed, 183 insertions(+), 178 deletions(-)
> create mode 100644 drivers/pci/controller/pcie-brcmstb.h
You moved more than just register definitions into pcie-brcmstb.h you
also moved internal structure definitions, enumerations, etc. which are
not required since pcie-brcmstb-acpi.c does not access the brcm_pcie
structure but open codes accesses to the MISC_STATUS register instead.
There are no include guards added to this file (it is debatable whether
we should add them), and it is also not covered by the existing BROADCOM
BCM2711/BCM2835 ARM ARCHITECTURE MAINTAINERS file entry.
Given that there can be new platforms supported by this PCIe controller
in the future possibly with the same limitations as the 2711, but with a
seemingly different MISC_STATUS layout, you will have to think about a
solution that scales, maybe we cross that bridge when we get there.
--
Florian
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/3] PCI: brcmstb: Break register definitions into separate header
@ 2021-08-10 10:07 ` Florian Fainelli
0 siblings, 0 replies; 42+ messages in thread
From: Florian Fainelli @ 2021-08-10 10:07 UTC (permalink / raw)
To: Jeremy Linton, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
On 8/5/2021 2:11 PM, Jeremy Linton wrote:
> We are about to create a standalone ACPI quirk module for the
> bcmstb controller. Lets move the register definitions into a separate
> file so they can be shared between the APCI quirk and the normal
> host bridge driver.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> drivers/pci/controller/pcie-brcmstb.c | 179 +------------------------
> drivers/pci/controller/pcie-brcmstb.h | 182 ++++++++++++++++++++++++++
> 2 files changed, 183 insertions(+), 178 deletions(-)
> create mode 100644 drivers/pci/controller/pcie-brcmstb.h
You moved more than just register definitions into pcie-brcmstb.h you
also moved internal structure definitions, enumerations, etc. which are
not required since pcie-brcmstb-acpi.c does not access the brcm_pcie
structure but open codes accesses to the MISC_STATUS register instead.
There are no include guards added to this file (it is debatable whether
we should add them), and it is also not covered by the existing BROADCOM
BCM2711/BCM2835 ARM ARCHITECTURE MAINTAINERS file entry.
Given that there can be new platforms supported by this PCIe controller
in the future possibly with the same limitations as the 2711, but with a
seemingly different MISC_STATUS layout, you will have to think about a
solution that scales, maybe we cross that bridge when we get there.
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/3] PCI: brcmstb: Break register definitions into separate header
2021-08-10 10:07 ` Florian Fainelli
@ 2021-08-10 15:10 ` Jeremy Linton
-1 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-10 15:10 UTC (permalink / raw)
To: Florian Fainelli, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
bcm-kernel-feedback-list, linux-acpi, linux-arm-kernel,
linux-rpi-kernel, linux-kernel
Hi,
Thanks for taking a look at this!
On 8/10/21 5:07 AM, Florian Fainelli wrote:
>
>
> On 8/5/2021 2:11 PM, Jeremy Linton wrote:
>> We are about to create a standalone ACPI quirk module for the
>> bcmstb controller. Lets move the register definitions into a separate
>> file so they can be shared between the APCI quirk and the normal
>> host bridge driver.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>> drivers/pci/controller/pcie-brcmstb.c | 179 +------------------------
>> drivers/pci/controller/pcie-brcmstb.h | 182 ++++++++++++++++++++++++++
>> 2 files changed, 183 insertions(+), 178 deletions(-)
>> create mode 100644 drivers/pci/controller/pcie-brcmstb.h
>
> You moved more than just register definitions into pcie-brcmstb.h you
> also moved internal structure definitions, enumerations, etc. which are
> not required since pcie-brcmstb-acpi.c does not access the brcm_pcie
> structure but open codes accesses to the MISC_STATUS register instead.
>
> There are no include guards added to this file (it is debatable whether
> we should add them), and it is also not covered by the existing BROADCOM
> BCM2711/BCM2835 ARM ARCHITECTURE MAINTAINERS file entry.
Sure, I will reduce the .h to just the register definitions, guard it,
and tweak maintainers to cover pcie-brcmstb*.
>
> Given that there can be new platforms supported by this PCIe controller
> in the future possibly with the same limitations as the 2711, but with a
> seemingly different MISC_STATUS layout, you will have to think about a
> solution that scales, maybe we cross that bridge when we get there.
Yes, given I don't know what those changes are I can't predict how they
would have to be handled, or even if the platform would be a target of
the community maintaining the UEFI/ACPI port on the RPi. So punting on
that topic seems a reasonable solution at the moment. Better yet, more
of the linux community will see the advantage of the firmware interface
and this platform can utilize that method.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/3] PCI: brcmstb: Break register definitions into separate header
@ 2021-08-10 15:10 ` Jeremy Linton
0 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-10 15:10 UTC (permalink / raw)
To: Florian Fainelli, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
bcm-kernel-feedback-list, linux-acpi, linux-arm-kernel,
linux-rpi-kernel, linux-kernel
Hi,
Thanks for taking a look at this!
On 8/10/21 5:07 AM, Florian Fainelli wrote:
>
>
> On 8/5/2021 2:11 PM, Jeremy Linton wrote:
>> We are about to create a standalone ACPI quirk module for the
>> bcmstb controller. Lets move the register definitions into a separate
>> file so they can be shared between the APCI quirk and the normal
>> host bridge driver.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>> drivers/pci/controller/pcie-brcmstb.c | 179 +------------------------
>> drivers/pci/controller/pcie-brcmstb.h | 182 ++++++++++++++++++++++++++
>> 2 files changed, 183 insertions(+), 178 deletions(-)
>> create mode 100644 drivers/pci/controller/pcie-brcmstb.h
>
> You moved more than just register definitions into pcie-brcmstb.h you
> also moved internal structure definitions, enumerations, etc. which are
> not required since pcie-brcmstb-acpi.c does not access the brcm_pcie
> structure but open codes accesses to the MISC_STATUS register instead.
>
> There are no include guards added to this file (it is debatable whether
> we should add them), and it is also not covered by the existing BROADCOM
> BCM2711/BCM2835 ARM ARCHITECTURE MAINTAINERS file entry.
Sure, I will reduce the .h to just the register definitions, guard it,
and tweak maintainers to cover pcie-brcmstb*.
>
> Given that there can be new platforms supported by this PCIe controller
> in the future possibly with the same limitations as the 2711, but with a
> seemingly different MISC_STATUS layout, you will have to think about a
> solution that scales, maybe we cross that bridge when we get there.
Yes, given I don't know what those changes are I can't predict how they
would have to be handled, or even if the platform would be a target of
the community maintaining the UEFI/ACPI port on the RPi. So punting on
that topic seems a reasonable solution at the moment. Better yet, more
of the linux community will see the advantage of the firmware interface
and this platform can utilize that method.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/3] PCI: brcmstb: Break register definitions into separate header
2021-08-10 15:10 ` Jeremy Linton
@ 2021-08-11 8:39 ` Florian Fainelli
-1 siblings, 0 replies; 42+ messages in thread
From: Florian Fainelli @ 2021-08-11 8:39 UTC (permalink / raw)
To: Jeremy Linton, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
bcm-kernel-feedback-list, linux-acpi, linux-arm-kernel,
linux-rpi-kernel, linux-kernel
On 8/10/2021 8:10 AM, Jeremy Linton wrote:
> Hi,
>
> Thanks for taking a look at this!
>
>
> On 8/10/21 5:07 AM, Florian Fainelli wrote:
>>
>>
>> On 8/5/2021 2:11 PM, Jeremy Linton wrote:
>>> We are about to create a standalone ACPI quirk module for the
>>> bcmstb controller. Lets move the register definitions into a separate
>>> file so they can be shared between the APCI quirk and the normal
>>> host bridge driver.
>>>
>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>> ---
>>> drivers/pci/controller/pcie-brcmstb.c | 179 +------------------------
>>> drivers/pci/controller/pcie-brcmstb.h | 182 ++++++++++++++++++++++++++
>>> 2 files changed, 183 insertions(+), 178 deletions(-)
>>> create mode 100644 drivers/pci/controller/pcie-brcmstb.h
>>
>> You moved more than just register definitions into pcie-brcmstb.h you
>> also moved internal structure definitions, enumerations, etc. which
>> are not required since pcie-brcmstb-acpi.c does not access the
>> brcm_pcie structure but open codes accesses to the MISC_STATUS
>> register instead.
>>
>> There are no include guards added to this file (it is debatable
>> whether we should add them), and it is also not covered by the
>> existing BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE MAINTAINERS file
>> entry.
>
> Sure, I will reduce the .h to just the register definitions, guard it,
> and tweak maintainers to cover pcie-brcmstb*.
>
>
>>
>> Given that there can be new platforms supported by this PCIe
>> controller in the future possibly with the same limitations as the
>> 2711, but with a seemingly different MISC_STATUS layout, you will have
>> to think about a solution that scales, maybe we cross that bridge when
>> we get there.
>
> Yes, given I don't know what those changes are I can't predict how they
> would have to be handled, or even if the platform would be a target of
> the community maintaining the UEFI/ACPI port on the RPi. So punting on
> that topic seems a reasonable solution at the moment. Better yet, more
> of the linux community will see the advantage of the firmware interface
> and this platform can utilize that method.
Ideally newer platforms would support ECAM and would not require a
custom quirk if nothing else, we do have discussions about that right
now, although it is not clear to me how it will materialize into a
product that people can buy.
--
Florian
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/3] PCI: brcmstb: Break register definitions into separate header
@ 2021-08-11 8:39 ` Florian Fainelli
0 siblings, 0 replies; 42+ messages in thread
From: Florian Fainelli @ 2021-08-11 8:39 UTC (permalink / raw)
To: Jeremy Linton, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
bcm-kernel-feedback-list, linux-acpi, linux-arm-kernel,
linux-rpi-kernel, linux-kernel
On 8/10/2021 8:10 AM, Jeremy Linton wrote:
> Hi,
>
> Thanks for taking a look at this!
>
>
> On 8/10/21 5:07 AM, Florian Fainelli wrote:
>>
>>
>> On 8/5/2021 2:11 PM, Jeremy Linton wrote:
>>> We are about to create a standalone ACPI quirk module for the
>>> bcmstb controller. Lets move the register definitions into a separate
>>> file so they can be shared between the APCI quirk and the normal
>>> host bridge driver.
>>>
>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>> ---
>>> drivers/pci/controller/pcie-brcmstb.c | 179 +------------------------
>>> drivers/pci/controller/pcie-brcmstb.h | 182 ++++++++++++++++++++++++++
>>> 2 files changed, 183 insertions(+), 178 deletions(-)
>>> create mode 100644 drivers/pci/controller/pcie-brcmstb.h
>>
>> You moved more than just register definitions into pcie-brcmstb.h you
>> also moved internal structure definitions, enumerations, etc. which
>> are not required since pcie-brcmstb-acpi.c does not access the
>> brcm_pcie structure but open codes accesses to the MISC_STATUS
>> register instead.
>>
>> There are no include guards added to this file (it is debatable
>> whether we should add them), and it is also not covered by the
>> existing BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE MAINTAINERS file
>> entry.
>
> Sure, I will reduce the .h to just the register definitions, guard it,
> and tweak maintainers to cover pcie-brcmstb*.
>
>
>>
>> Given that there can be new platforms supported by this PCIe
>> controller in the future possibly with the same limitations as the
>> 2711, but with a seemingly different MISC_STATUS layout, you will have
>> to think about a solution that scales, maybe we cross that bridge when
>> we get there.
>
> Yes, given I don't know what those changes are I can't predict how they
> would have to be handled, or even if the platform would be a target of
> the community maintaining the UEFI/ACPI port on the RPi. So punting on
> that topic seems a reasonable solution at the moment. Better yet, more
> of the linux community will see the advantage of the firmware interface
> and this platform can utilize that method.
Ideally newer platforms would support ECAM and would not require a
custom quirk if nothing else, we do have discussions about that right
now, although it is not clear to me how it will materialize into a
product that people can buy.
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
2021-08-05 21:11 ` Jeremy Linton
@ 2021-08-05 21:11 ` Jeremy Linton
-1 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-05 21:11 UTC (permalink / raw)
To: linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel, Jeremy Linton
The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
standard because its PCIe config space isn't ECAM compliant and is
split into two parts. One part for the root port registers and a
moveable window which points at a given device's 4K config space.
Thus it doesn't have a MCFG (and really any MCFG provided would be
nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
we key off a Linux specific host bridge _DSD to add custom ECAM
ops and cfgres. The cfg op selects between those two regions, as
well as disallowing problematic accesses, particularly if the link
is down because there isn't an attached device.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-brcmstb-acpi.c | 77 ++++++++++++++++++++++
include/linux/pci-ecam.h | 1 +
3 files changed, 79 insertions(+)
create mode 100644 drivers/pci/controller/pcie-brcmstb-acpi.c
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index aaf30b3dcc14..65aa6fd3ed89 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -57,5 +57,6 @@ ifdef CONFIG_PCI_QUIRKS
obj-$(CONFIG_ARM64) += pci-thunder-ecam.o
obj-$(CONFIG_ARM64) += pci-thunder-pem.o
obj-$(CONFIG_ARM64) += pci-xgene.o
+obj-$(CONFIG_ARM64) += pcie-brcmstb-acpi.o
endif
endif
diff --git a/drivers/pci/controller/pcie-brcmstb-acpi.c b/drivers/pci/controller/pcie-brcmstb-acpi.c
new file mode 100644
index 000000000000..76944876155f
--- /dev/null
+++ b/drivers/pci/controller/pcie-brcmstb-acpi.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * ACPI quirks for Brcm2711 PCIe host controller
+ * As used on the Raspberry Pi Compute Module 4
+ *
+ * Copyright (C) 2021 Arm Ltd.
+ */
+
+#include <linux/io.h>
+#include <linux/pci.h>
+#include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
+#include "../pci.h"
+#include "pcie-brcmstb.h"
+
+static int brcm_acpi_init(struct pci_config_window *cfg)
+{
+ /*
+ * This platform doesn't technically have anything that could be called
+ * ECAM. Its config region has root port specific registers between
+ * standard PCIe defined config registers. Thus the region setup by the
+ * generic ECAM code needs to be adjusted. The HW can access bus 0-ff
+ * but the footprint isn't a nice power of 2 (40k). For purposes of
+ * mapping the config region we are just going to squash the standard
+ * and nonstandard registers together rather than mapping them
+ * separately. This code simply honors the quirk provided base+size
+ * instead.
+ */
+ iounmap(cfg->win);
+ cfg->win = pci_remap_cfgspace(cfg->res.start, resource_size(&cfg->res));
+ if (!cfg->win)
+ goto err_exit;
+
+ /* MSI is nonstandard as well */
+ pci_no_msi();
+
+ return 0;
+err_exit:
+ dev_err(cfg->parent, "PCI: Failed to remap config\n");
+ return -ENOMEM;
+}
+
+static void __iomem *brcm_pcie_map_conf2(struct pci_bus *bus, unsigned int devfn,
+ int where)
+{
+ struct pci_config_window *cfg = bus->sysdata;
+ void __iomem *base = cfg->win;
+ int idx;
+ u32 up;
+
+ /* Accesses to the RC go right to the RC registers if slot==0 */
+ if (pci_is_root_bus(bus))
+ return PCI_SLOT(devfn) ? NULL : base + where;
+
+ /* Assure link up before sending request */
+ up = readl(base + PCIE_MISC_PCIE_STATUS);
+ if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK))
+ return NULL;
+
+ if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK))
+ return NULL;
+
+ /* For devices, write to the config space index register */
+ idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
+ writel(idx, base + PCIE_EXT_CFG_INDEX);
+ return base + PCIE_EXT_CFG_DATA + where;
+}
+
+const struct pci_ecam_ops bcm2711_pcie_ops = {
+ .init = brcm_acpi_init,
+ .bus_shift = 1,
+ .pci_ops = {
+ .map_bus = brcm_pcie_map_conf2,
+ .read = pci_generic_config_read,
+ .write = pci_generic_config_write,
+ }
+};
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
index adea5a4771cf..a5de0285bb7f 100644
--- a/include/linux/pci-ecam.h
+++ b/include/linux/pci-ecam.h
@@ -87,6 +87,7 @@ extern const struct pci_ecam_ops xgene_v1_pcie_ecam_ops; /* APM X-Gene PCIe v1 *
extern const struct pci_ecam_ops xgene_v2_pcie_ecam_ops; /* APM X-Gene PCIe v2.x */
extern const struct pci_ecam_ops al_pcie_ops; /* Amazon Annapurna Labs PCIe */
extern const struct pci_ecam_ops tegra194_pcie_ops; /* Tegra194 PCIe */
+extern const struct pci_ecam_ops bcm2711_pcie_ops; /* Bcm2711 PCIe */
#endif
#if IS_ENABLED(CONFIG_PCI_HOST_COMMON)
--
2.31.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
@ 2021-08-05 21:11 ` Jeremy Linton
0 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-05 21:11 UTC (permalink / raw)
To: linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel, Jeremy Linton
The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
standard because its PCIe config space isn't ECAM compliant and is
split into two parts. One part for the root port registers and a
moveable window which points at a given device's 4K config space.
Thus it doesn't have a MCFG (and really any MCFG provided would be
nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
we key off a Linux specific host bridge _DSD to add custom ECAM
ops and cfgres. The cfg op selects between those two regions, as
well as disallowing problematic accesses, particularly if the link
is down because there isn't an attached device.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-brcmstb-acpi.c | 77 ++++++++++++++++++++++
include/linux/pci-ecam.h | 1 +
3 files changed, 79 insertions(+)
create mode 100644 drivers/pci/controller/pcie-brcmstb-acpi.c
diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
index aaf30b3dcc14..65aa6fd3ed89 100644
--- a/drivers/pci/controller/Makefile
+++ b/drivers/pci/controller/Makefile
@@ -57,5 +57,6 @@ ifdef CONFIG_PCI_QUIRKS
obj-$(CONFIG_ARM64) += pci-thunder-ecam.o
obj-$(CONFIG_ARM64) += pci-thunder-pem.o
obj-$(CONFIG_ARM64) += pci-xgene.o
+obj-$(CONFIG_ARM64) += pcie-brcmstb-acpi.o
endif
endif
diff --git a/drivers/pci/controller/pcie-brcmstb-acpi.c b/drivers/pci/controller/pcie-brcmstb-acpi.c
new file mode 100644
index 000000000000..76944876155f
--- /dev/null
+++ b/drivers/pci/controller/pcie-brcmstb-acpi.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * ACPI quirks for Brcm2711 PCIe host controller
+ * As used on the Raspberry Pi Compute Module 4
+ *
+ * Copyright (C) 2021 Arm Ltd.
+ */
+
+#include <linux/io.h>
+#include <linux/pci.h>
+#include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
+#include "../pci.h"
+#include "pcie-brcmstb.h"
+
+static int brcm_acpi_init(struct pci_config_window *cfg)
+{
+ /*
+ * This platform doesn't technically have anything that could be called
+ * ECAM. Its config region has root port specific registers between
+ * standard PCIe defined config registers. Thus the region setup by the
+ * generic ECAM code needs to be adjusted. The HW can access bus 0-ff
+ * but the footprint isn't a nice power of 2 (40k). For purposes of
+ * mapping the config region we are just going to squash the standard
+ * and nonstandard registers together rather than mapping them
+ * separately. This code simply honors the quirk provided base+size
+ * instead.
+ */
+ iounmap(cfg->win);
+ cfg->win = pci_remap_cfgspace(cfg->res.start, resource_size(&cfg->res));
+ if (!cfg->win)
+ goto err_exit;
+
+ /* MSI is nonstandard as well */
+ pci_no_msi();
+
+ return 0;
+err_exit:
+ dev_err(cfg->parent, "PCI: Failed to remap config\n");
+ return -ENOMEM;
+}
+
+static void __iomem *brcm_pcie_map_conf2(struct pci_bus *bus, unsigned int devfn,
+ int where)
+{
+ struct pci_config_window *cfg = bus->sysdata;
+ void __iomem *base = cfg->win;
+ int idx;
+ u32 up;
+
+ /* Accesses to the RC go right to the RC registers if slot==0 */
+ if (pci_is_root_bus(bus))
+ return PCI_SLOT(devfn) ? NULL : base + where;
+
+ /* Assure link up before sending request */
+ up = readl(base + PCIE_MISC_PCIE_STATUS);
+ if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK))
+ return NULL;
+
+ if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK))
+ return NULL;
+
+ /* For devices, write to the config space index register */
+ idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
+ writel(idx, base + PCIE_EXT_CFG_INDEX);
+ return base + PCIE_EXT_CFG_DATA + where;
+}
+
+const struct pci_ecam_ops bcm2711_pcie_ops = {
+ .init = brcm_acpi_init,
+ .bus_shift = 1,
+ .pci_ops = {
+ .map_bus = brcm_pcie_map_conf2,
+ .read = pci_generic_config_read,
+ .write = pci_generic_config_write,
+ }
+};
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
index adea5a4771cf..a5de0285bb7f 100644
--- a/include/linux/pci-ecam.h
+++ b/include/linux/pci-ecam.h
@@ -87,6 +87,7 @@ extern const struct pci_ecam_ops xgene_v1_pcie_ecam_ops; /* APM X-Gene PCIe v1 *
extern const struct pci_ecam_ops xgene_v2_pcie_ecam_ops; /* APM X-Gene PCIe v2.x */
extern const struct pci_ecam_ops al_pcie_ops; /* Amazon Annapurna Labs PCIe */
extern const struct pci_ecam_ops tegra194_pcie_ops; /* Tegra194 PCIe */
+extern const struct pci_ecam_ops bcm2711_pcie_ops; /* Bcm2711 PCIe */
#endif
#if IS_ENABLED(CONFIG_PCI_HOST_COMMON)
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
2021-08-05 21:11 ` Jeremy Linton
@ 2021-08-06 22:21 ` Bjorn Helgaas
-1 siblings, 0 replies; 42+ messages in thread
From: Bjorn Helgaas @ 2021-08-06 22:21 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
> The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
> standard because its PCIe config space isn't ECAM compliant and is
> split into two parts. One part for the root port registers and a
> moveable window which points at a given device's 4K config space.
> Thus it doesn't have a MCFG (and really any MCFG provided would be
> nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
> we key off a Linux specific host bridge _DSD to add custom ECAM
> ops and cfgres. The cfg op selects between those two regions, as
> well as disallowing problematic accesses, particularly if the link
> is down because there isn't an attached device.
I'm not sure SMCCC is *really* relevant here. If it is, an expansion
of the acronym and a link to a spec would be helpful.
But AFAICT the only important thing here is that it doesn't have
standard ECAM, and we're going to work around that.
I don't see anything about _DSD in this series.
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> drivers/pci/controller/Makefile | 1 +
> drivers/pci/controller/pcie-brcmstb-acpi.c | 77 ++++++++++++++++++++++
> include/linux/pci-ecam.h | 1 +
> 3 files changed, 79 insertions(+)
> create mode 100644 drivers/pci/controller/pcie-brcmstb-acpi.c
>
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index aaf30b3dcc14..65aa6fd3ed89 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -57,5 +57,6 @@ ifdef CONFIG_PCI_QUIRKS
> obj-$(CONFIG_ARM64) += pci-thunder-ecam.o
> obj-$(CONFIG_ARM64) += pci-thunder-pem.o
> obj-$(CONFIG_ARM64) += pci-xgene.o
> +obj-$(CONFIG_ARM64) += pcie-brcmstb-acpi.o
> endif
> endif
> diff --git a/drivers/pci/controller/pcie-brcmstb-acpi.c b/drivers/pci/controller/pcie-brcmstb-acpi.c
> new file mode 100644
> index 000000000000..76944876155f
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-brcmstb-acpi.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * ACPI quirks for Brcm2711 PCIe host controller
> + * As used on the Raspberry Pi Compute Module 4
> + *
> + * Copyright (C) 2021 Arm Ltd.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
Do we use something from pci-acpi.h?
> +#include <linux/pci-ecam.h>
> +#include "../pci.h"
> +#include "pcie-brcmstb.h"
> +
> +static int brcm_acpi_init(struct pci_config_window *cfg)
> +{
> + /*
> + * This platform doesn't technically have anything that could be called
> + * ECAM. Its config region has root port specific registers between
> + * standard PCIe defined config registers. Thus the region setup by the
> + * generic ECAM code needs to be adjusted. The HW can access bus 0-ff
> + * but the footprint isn't a nice power of 2 (40k). For purposes of
> + * mapping the config region we are just going to squash the standard
> + * and nonstandard registers together rather than mapping them
> + * separately. This code simply honors the quirk provided base+size
> + * instead.
> + */
> + iounmap(cfg->win);
> + cfg->win = pci_remap_cfgspace(cfg->res.start, resource_size(&cfg->res));
> + if (!cfg->win)
> + goto err_exit;
> +
> + /* MSI is nonstandard as well */
> + pci_no_msi();
> +
> + return 0;
> +err_exit:
> + dev_err(cfg->parent, "PCI: Failed to remap config\n");
> + return -ENOMEM;
> +}
> +
> +static void __iomem *brcm_pcie_map_conf2(struct pci_bus *bus, unsigned int devfn,
> + int where)
Rewrap to fit in 80 columns. 81 is just ... weird :)
> +{
> + struct pci_config_window *cfg = bus->sysdata;
> + void __iomem *base = cfg->win;
> + int idx;
> + u32 up;
> +
> + /* Accesses to the RC go right to the RC registers if slot==0 */
> + if (pci_is_root_bus(bus))
> + return PCI_SLOT(devfn) ? NULL : base + where;
> +
> + /* Assure link up before sending request */
> + up = readl(base + PCIE_MISC_PCIE_STATUS);
> + if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK))
> + return NULL;
> +
> + if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK))
> + return NULL;
What happens if the link goes down here? Hopefully that's a
recoverable error?
> + /* For devices, write to the config space index register */
> + idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
> + writel(idx, base + PCIE_EXT_CFG_INDEX);
> + return base + PCIE_EXT_CFG_DATA + where;
> +}
> +
> +const struct pci_ecam_ops bcm2711_pcie_ops = {
> + .init = brcm_acpi_init,
> + .bus_shift = 1,
> + .pci_ops = {
> + .map_bus = brcm_pcie_map_conf2,
> + .read = pci_generic_config_read,
> + .write = pci_generic_config_write,
> + }
> +};
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index adea5a4771cf..a5de0285bb7f 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -87,6 +87,7 @@ extern const struct pci_ecam_ops xgene_v1_pcie_ecam_ops; /* APM X-Gene PCIe v1 *
> extern const struct pci_ecam_ops xgene_v2_pcie_ecam_ops; /* APM X-Gene PCIe v2.x */
> extern const struct pci_ecam_ops al_pcie_ops; /* Amazon Annapurna Labs PCIe */
> extern const struct pci_ecam_ops tegra194_pcie_ops; /* Tegra194 PCIe */
> +extern const struct pci_ecam_ops bcm2711_pcie_ops; /* Bcm2711 PCIe */
> #endif
>
> #if IS_ENABLED(CONFIG_PCI_HOST_COMMON)
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
@ 2021-08-06 22:21 ` Bjorn Helgaas
0 siblings, 0 replies; 42+ messages in thread
From: Bjorn Helgaas @ 2021-08-06 22:21 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
> The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
> standard because its PCIe config space isn't ECAM compliant and is
> split into two parts. One part for the root port registers and a
> moveable window which points at a given device's 4K config space.
> Thus it doesn't have a MCFG (and really any MCFG provided would be
> nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
> we key off a Linux specific host bridge _DSD to add custom ECAM
> ops and cfgres. The cfg op selects between those two regions, as
> well as disallowing problematic accesses, particularly if the link
> is down because there isn't an attached device.
I'm not sure SMCCC is *really* relevant here. If it is, an expansion
of the acronym and a link to a spec would be helpful.
But AFAICT the only important thing here is that it doesn't have
standard ECAM, and we're going to work around that.
I don't see anything about _DSD in this series.
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> drivers/pci/controller/Makefile | 1 +
> drivers/pci/controller/pcie-brcmstb-acpi.c | 77 ++++++++++++++++++++++
> include/linux/pci-ecam.h | 1 +
> 3 files changed, 79 insertions(+)
> create mode 100644 drivers/pci/controller/pcie-brcmstb-acpi.c
>
> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
> index aaf30b3dcc14..65aa6fd3ed89 100644
> --- a/drivers/pci/controller/Makefile
> +++ b/drivers/pci/controller/Makefile
> @@ -57,5 +57,6 @@ ifdef CONFIG_PCI_QUIRKS
> obj-$(CONFIG_ARM64) += pci-thunder-ecam.o
> obj-$(CONFIG_ARM64) += pci-thunder-pem.o
> obj-$(CONFIG_ARM64) += pci-xgene.o
> +obj-$(CONFIG_ARM64) += pcie-brcmstb-acpi.o
> endif
> endif
> diff --git a/drivers/pci/controller/pcie-brcmstb-acpi.c b/drivers/pci/controller/pcie-brcmstb-acpi.c
> new file mode 100644
> index 000000000000..76944876155f
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-brcmstb-acpi.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * ACPI quirks for Brcm2711 PCIe host controller
> + * As used on the Raspberry Pi Compute Module 4
> + *
> + * Copyright (C) 2021 Arm Ltd.
> + */
> +
> +#include <linux/io.h>
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
Do we use something from pci-acpi.h?
> +#include <linux/pci-ecam.h>
> +#include "../pci.h"
> +#include "pcie-brcmstb.h"
> +
> +static int brcm_acpi_init(struct pci_config_window *cfg)
> +{
> + /*
> + * This platform doesn't technically have anything that could be called
> + * ECAM. Its config region has root port specific registers between
> + * standard PCIe defined config registers. Thus the region setup by the
> + * generic ECAM code needs to be adjusted. The HW can access bus 0-ff
> + * but the footprint isn't a nice power of 2 (40k). For purposes of
> + * mapping the config region we are just going to squash the standard
> + * and nonstandard registers together rather than mapping them
> + * separately. This code simply honors the quirk provided base+size
> + * instead.
> + */
> + iounmap(cfg->win);
> + cfg->win = pci_remap_cfgspace(cfg->res.start, resource_size(&cfg->res));
> + if (!cfg->win)
> + goto err_exit;
> +
> + /* MSI is nonstandard as well */
> + pci_no_msi();
> +
> + return 0;
> +err_exit:
> + dev_err(cfg->parent, "PCI: Failed to remap config\n");
> + return -ENOMEM;
> +}
> +
> +static void __iomem *brcm_pcie_map_conf2(struct pci_bus *bus, unsigned int devfn,
> + int where)
Rewrap to fit in 80 columns. 81 is just ... weird :)
> +{
> + struct pci_config_window *cfg = bus->sysdata;
> + void __iomem *base = cfg->win;
> + int idx;
> + u32 up;
> +
> + /* Accesses to the RC go right to the RC registers if slot==0 */
> + if (pci_is_root_bus(bus))
> + return PCI_SLOT(devfn) ? NULL : base + where;
> +
> + /* Assure link up before sending request */
> + up = readl(base + PCIE_MISC_PCIE_STATUS);
> + if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK))
> + return NULL;
> +
> + if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK))
> + return NULL;
What happens if the link goes down here? Hopefully that's a
recoverable error?
> + /* For devices, write to the config space index register */
> + idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
> + writel(idx, base + PCIE_EXT_CFG_INDEX);
> + return base + PCIE_EXT_CFG_DATA + where;
> +}
> +
> +const struct pci_ecam_ops bcm2711_pcie_ops = {
> + .init = brcm_acpi_init,
> + .bus_shift = 1,
> + .pci_ops = {
> + .map_bus = brcm_pcie_map_conf2,
> + .read = pci_generic_config_read,
> + .write = pci_generic_config_write,
> + }
> +};
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index adea5a4771cf..a5de0285bb7f 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -87,6 +87,7 @@ extern const struct pci_ecam_ops xgene_v1_pcie_ecam_ops; /* APM X-Gene PCIe v1 *
> extern const struct pci_ecam_ops xgene_v2_pcie_ecam_ops; /* APM X-Gene PCIe v2.x */
> extern const struct pci_ecam_ops al_pcie_ops; /* Amazon Annapurna Labs PCIe */
> extern const struct pci_ecam_ops tegra194_pcie_ops; /* Tegra194 PCIe */
> +extern const struct pci_ecam_ops bcm2711_pcie_ops; /* Bcm2711 PCIe */
> #endif
>
> #if IS_ENABLED(CONFIG_PCI_HOST_COMMON)
> --
> 2.31.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
2021-08-06 22:21 ` Bjorn Helgaas
@ 2021-08-07 2:55 ` Jeremy Linton
-1 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-07 2:55 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi,
On 8/6/21 5:21 PM, Bjorn Helgaas wrote:
> On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
>> The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
>> standard because its PCIe config space isn't ECAM compliant and is
>> split into two parts. One part for the root port registers and a
>> moveable window which points at a given device's 4K config space.
>> Thus it doesn't have a MCFG (and really any MCFG provided would be
>> nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
>> we key off a Linux specific host bridge _DSD to add custom ECAM
>> ops and cfgres. The cfg op selects between those two regions, as
>> well as disallowing problematic accesses, particularly if the link
>> is down because there isn't an attached device.
>
> I'm not sure SMCCC is *really* relevant here. If it is, an expansion
> of the acronym and a link to a spec would be helpful.
>
> But AFAICT the only important thing here is that it doesn't have
> standard ECAM, and we're going to work around that.
I will reword it a bit.
>
> I don't see anything about _DSD in this series.
That is the "linux,pci-quirk" in the next patch.
>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>> drivers/pci/controller/Makefile | 1 +
>> drivers/pci/controller/pcie-brcmstb-acpi.c | 77 ++++++++++++++++++++++
>> include/linux/pci-ecam.h | 1 +
>> 3 files changed, 79 insertions(+)
>> create mode 100644 drivers/pci/controller/pcie-brcmstb-acpi.c
>>
>> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
>> index aaf30b3dcc14..65aa6fd3ed89 100644
>> --- a/drivers/pci/controller/Makefile
>> +++ b/drivers/pci/controller/Makefile
>> @@ -57,5 +57,6 @@ ifdef CONFIG_PCI_QUIRKS
>> obj-$(CONFIG_ARM64) += pci-thunder-ecam.o
>> obj-$(CONFIG_ARM64) += pci-thunder-pem.o
>> obj-$(CONFIG_ARM64) += pci-xgene.o
>> +obj-$(CONFIG_ARM64) += pcie-brcmstb-acpi.o
>> endif
>> endif
>> diff --git a/drivers/pci/controller/pcie-brcmstb-acpi.c b/drivers/pci/controller/pcie-brcmstb-acpi.c
>> new file mode 100644
>> index 000000000000..76944876155f
>> --- /dev/null
>> +++ b/drivers/pci/controller/pcie-brcmstb-acpi.c
>> @@ -0,0 +1,77 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * ACPI quirks for Brcm2711 PCIe host controller
>> + * As used on the Raspberry Pi Compute Module 4
>> + *
>> + * Copyright (C) 2021 Arm Ltd.
>> + */
>> +
>> +#include <linux/io.h>
>> +#include <linux/pci.h>
>> +#include <linux/pci-acpi.h>
>
> Do we use something from pci-acpi.h?
Good catch.
>
>> +#include <linux/pci-ecam.h>
>> +#include "../pci.h"
>> +#include "pcie-brcmstb.h"
>> +
>> +static int brcm_acpi_init(struct pci_config_window *cfg)
>> +{
>> + /*
>> + * This platform doesn't technically have anything that could be called
>> + * ECAM. Its config region has root port specific registers between
>> + * standard PCIe defined config registers. Thus the region setup by the
>> + * generic ECAM code needs to be adjusted. The HW can access bus 0-ff
>> + * but the footprint isn't a nice power of 2 (40k). For purposes of
>> + * mapping the config region we are just going to squash the standard
>> + * and nonstandard registers together rather than mapping them
>> + * separately. This code simply honors the quirk provided base+size
>> + * instead.
>> + */
>> + iounmap(cfg->win);
>> + cfg->win = pci_remap_cfgspace(cfg->res.start, resource_size(&cfg->res));
>> + if (!cfg->win)
>> + goto err_exit;
>> +
>> + /* MSI is nonstandard as well */
>> + pci_no_msi();
>> +
>> + return 0;
>> +err_exit:
>> + dev_err(cfg->parent, "PCI: Failed to remap config\n");
>> + return -ENOMEM;
>> +}
>> +
>> +static void __iomem *brcm_pcie_map_conf2(struct pci_bus *bus, unsigned int devfn,
>> + int where)
>
> Rewrap to fit in 80 columns. 81 is just ... weird :)
Sure,
>
>> +{
>> + struct pci_config_window *cfg = bus->sysdata;
>> + void __iomem *base = cfg->win;
>> + int idx;
>> + u32 up;
>> +
>> + /* Accesses to the RC go right to the RC registers if slot==0 */
>> + if (pci_is_root_bus(bus))
>> + return PCI_SLOT(devfn) ? NULL : base + where;
>> +
>> + /* Assure link up before sending request */
>> + up = readl(base + PCIE_MISC_PCIE_STATUS);
>> + if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK))
>> + return NULL;
>> +
>> + if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK))
>> + return NULL;
>
> What happens if the link goes down here? Hopefully that's a
> recoverable error?
This check is intended to verify that something is plugged into the
first bus/slot before attempting to probe it. If nothing is plugged in
bad things (TM) happen without this check.
But, you sparked my curiosity. I've had some reasonable luck with error
recovery with a number of those cheap pcie->pcie switches that passively
route PCIe over USB3 cables when plugged into this thing. So, I tried
hotpluging the first one to see what happens.
Game over! :)
Linux was nice enough to tell me it couldn't change the power state back
to active, but it looks like the link state actually recovered. OTOH,
the downstream switch appeared to need reconfiguration even though it
was on a separate power domain. Given, I booted the machine off a NVMe
drive plugged into that switch, I got another nice screen full of blk_mq
errors before it officially passed on.
So, the link appears to recover, but that doesn't appear to be enough to
make the downstream devices recoverable.
Thanks,
>
>> + /* For devices, write to the config space index register */
>> + idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
>> + writel(idx, base + PCIE_EXT_CFG_INDEX);
>> + return base + PCIE_EXT_CFG_DATA + where;
>> +}
>> +
>> +const struct pci_ecam_ops bcm2711_pcie_ops = {
>> + .init = brcm_acpi_init,
>> + .bus_shift = 1,
>> + .pci_ops = {
>> + .map_bus = brcm_pcie_map_conf2,
>> + .read = pci_generic_config_read,
>> + .write = pci_generic_config_write,
>> + }
>> +};
>> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
>> index adea5a4771cf..a5de0285bb7f 100644
>> --- a/include/linux/pci-ecam.h
>> +++ b/include/linux/pci-ecam.h
>> @@ -87,6 +87,7 @@ extern const struct pci_ecam_ops xgene_v1_pcie_ecam_ops; /* APM X-Gene PCIe v1 *
>> extern const struct pci_ecam_ops xgene_v2_pcie_ecam_ops; /* APM X-Gene PCIe v2.x */
>> extern const struct pci_ecam_ops al_pcie_ops; /* Amazon Annapurna Labs PCIe */
>> extern const struct pci_ecam_ops tegra194_pcie_ops; /* Tegra194 PCIe */
>> +extern const struct pci_ecam_ops bcm2711_pcie_ops; /* Bcm2711 PCIe */
>> #endif
>>
>> #if IS_ENABLED(CONFIG_PCI_HOST_COMMON)
>> --
>> 2.31.1
>>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
@ 2021-08-07 2:55 ` Jeremy Linton
0 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-07 2:55 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi,
On 8/6/21 5:21 PM, Bjorn Helgaas wrote:
> On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
>> The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
>> standard because its PCIe config space isn't ECAM compliant and is
>> split into two parts. One part for the root port registers and a
>> moveable window which points at a given device's 4K config space.
>> Thus it doesn't have a MCFG (and really any MCFG provided would be
>> nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
>> we key off a Linux specific host bridge _DSD to add custom ECAM
>> ops and cfgres. The cfg op selects between those two regions, as
>> well as disallowing problematic accesses, particularly if the link
>> is down because there isn't an attached device.
>
> I'm not sure SMCCC is *really* relevant here. If it is, an expansion
> of the acronym and a link to a spec would be helpful.
>
> But AFAICT the only important thing here is that it doesn't have
> standard ECAM, and we're going to work around that.
I will reword it a bit.
>
> I don't see anything about _DSD in this series.
That is the "linux,pci-quirk" in the next patch.
>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>> drivers/pci/controller/Makefile | 1 +
>> drivers/pci/controller/pcie-brcmstb-acpi.c | 77 ++++++++++++++++++++++
>> include/linux/pci-ecam.h | 1 +
>> 3 files changed, 79 insertions(+)
>> create mode 100644 drivers/pci/controller/pcie-brcmstb-acpi.c
>>
>> diff --git a/drivers/pci/controller/Makefile b/drivers/pci/controller/Makefile
>> index aaf30b3dcc14..65aa6fd3ed89 100644
>> --- a/drivers/pci/controller/Makefile
>> +++ b/drivers/pci/controller/Makefile
>> @@ -57,5 +57,6 @@ ifdef CONFIG_PCI_QUIRKS
>> obj-$(CONFIG_ARM64) += pci-thunder-ecam.o
>> obj-$(CONFIG_ARM64) += pci-thunder-pem.o
>> obj-$(CONFIG_ARM64) += pci-xgene.o
>> +obj-$(CONFIG_ARM64) += pcie-brcmstb-acpi.o
>> endif
>> endif
>> diff --git a/drivers/pci/controller/pcie-brcmstb-acpi.c b/drivers/pci/controller/pcie-brcmstb-acpi.c
>> new file mode 100644
>> index 000000000000..76944876155f
>> --- /dev/null
>> +++ b/drivers/pci/controller/pcie-brcmstb-acpi.c
>> @@ -0,0 +1,77 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * ACPI quirks for Brcm2711 PCIe host controller
>> + * As used on the Raspberry Pi Compute Module 4
>> + *
>> + * Copyright (C) 2021 Arm Ltd.
>> + */
>> +
>> +#include <linux/io.h>
>> +#include <linux/pci.h>
>> +#include <linux/pci-acpi.h>
>
> Do we use something from pci-acpi.h?
Good catch.
>
>> +#include <linux/pci-ecam.h>
>> +#include "../pci.h"
>> +#include "pcie-brcmstb.h"
>> +
>> +static int brcm_acpi_init(struct pci_config_window *cfg)
>> +{
>> + /*
>> + * This platform doesn't technically have anything that could be called
>> + * ECAM. Its config region has root port specific registers between
>> + * standard PCIe defined config registers. Thus the region setup by the
>> + * generic ECAM code needs to be adjusted. The HW can access bus 0-ff
>> + * but the footprint isn't a nice power of 2 (40k). For purposes of
>> + * mapping the config region we are just going to squash the standard
>> + * and nonstandard registers together rather than mapping them
>> + * separately. This code simply honors the quirk provided base+size
>> + * instead.
>> + */
>> + iounmap(cfg->win);
>> + cfg->win = pci_remap_cfgspace(cfg->res.start, resource_size(&cfg->res));
>> + if (!cfg->win)
>> + goto err_exit;
>> +
>> + /* MSI is nonstandard as well */
>> + pci_no_msi();
>> +
>> + return 0;
>> +err_exit:
>> + dev_err(cfg->parent, "PCI: Failed to remap config\n");
>> + return -ENOMEM;
>> +}
>> +
>> +static void __iomem *brcm_pcie_map_conf2(struct pci_bus *bus, unsigned int devfn,
>> + int where)
>
> Rewrap to fit in 80 columns. 81 is just ... weird :)
Sure,
>
>> +{
>> + struct pci_config_window *cfg = bus->sysdata;
>> + void __iomem *base = cfg->win;
>> + int idx;
>> + u32 up;
>> +
>> + /* Accesses to the RC go right to the RC registers if slot==0 */
>> + if (pci_is_root_bus(bus))
>> + return PCI_SLOT(devfn) ? NULL : base + where;
>> +
>> + /* Assure link up before sending request */
>> + up = readl(base + PCIE_MISC_PCIE_STATUS);
>> + if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK))
>> + return NULL;
>> +
>> + if (!(up & PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK))
>> + return NULL;
>
> What happens if the link goes down here? Hopefully that's a
> recoverable error?
This check is intended to verify that something is plugged into the
first bus/slot before attempting to probe it. If nothing is plugged in
bad things (TM) happen without this check.
But, you sparked my curiosity. I've had some reasonable luck with error
recovery with a number of those cheap pcie->pcie switches that passively
route PCIe over USB3 cables when plugged into this thing. So, I tried
hotpluging the first one to see what happens.
Game over! :)
Linux was nice enough to tell me it couldn't change the power state back
to active, but it looks like the link state actually recovered. OTOH,
the downstream switch appeared to need reconfiguration even though it
was on a separate power domain. Given, I booted the machine off a NVMe
drive plugged into that switch, I got another nice screen full of blk_mq
errors before it officially passed on.
So, the link appears to recover, but that doesn't appear to be enough to
make the downstream devices recoverable.
Thanks,
>
>> + /* For devices, write to the config space index register */
>> + idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
>> + writel(idx, base + PCIE_EXT_CFG_INDEX);
>> + return base + PCIE_EXT_CFG_DATA + where;
>> +}
>> +
>> +const struct pci_ecam_ops bcm2711_pcie_ops = {
>> + .init = brcm_acpi_init,
>> + .bus_shift = 1,
>> + .pci_ops = {
>> + .map_bus = brcm_pcie_map_conf2,
>> + .read = pci_generic_config_read,
>> + .write = pci_generic_config_write,
>> + }
>> +};
>> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
>> index adea5a4771cf..a5de0285bb7f 100644
>> --- a/include/linux/pci-ecam.h
>> +++ b/include/linux/pci-ecam.h
>> @@ -87,6 +87,7 @@ extern const struct pci_ecam_ops xgene_v1_pcie_ecam_ops; /* APM X-Gene PCIe v1 *
>> extern const struct pci_ecam_ops xgene_v2_pcie_ecam_ops; /* APM X-Gene PCIe v2.x */
>> extern const struct pci_ecam_ops al_pcie_ops; /* Amazon Annapurna Labs PCIe */
>> extern const struct pci_ecam_ops tegra194_pcie_ops; /* Tegra194 PCIe */
>> +extern const struct pci_ecam_ops bcm2711_pcie_ops; /* Bcm2711 PCIe */
>> #endif
>>
>> #if IS_ENABLED(CONFIG_PCI_HOST_COMMON)
>> --
>> 2.31.1
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
2021-08-07 2:55 ` Jeremy Linton
@ 2021-08-09 17:42 ` Bjorn Helgaas
-1 siblings, 0 replies; 42+ messages in thread
From: Bjorn Helgaas @ 2021-08-09 17:42 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
On Fri, Aug 06, 2021 at 09:55:27PM -0500, Jeremy Linton wrote:
> Hi,
>
> On 8/6/21 5:21 PM, Bjorn Helgaas wrote:
> > On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
> > > The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
> > > standard because its PCIe config space isn't ECAM compliant and is
> > > split into two parts. One part for the root port registers and a
> > > moveable window which points at a given device's 4K config space.
> > > Thus it doesn't have a MCFG (and really any MCFG provided would be
> > > nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
> > > we key off a Linux specific host bridge _DSD to add custom ECAM
> > > ops and cfgres. The cfg op selects between those two regions, as
> > > well as disallowing problematic accesses, particularly if the link
> > > is down because there isn't an attached device.
> >
> > I'm not sure SMCCC is *really* relevant here. If it is, an expansion
> > of the acronym and a link to a spec would be helpful.
> >
> > But AFAICT the only important thing here is that it doesn't have
> > standard ECAM, and we're going to work around that.
>
> I will reword it a bit.
>
> > I don't see anything about _DSD in this series.
>
> That is the "linux,pci-quirk" in the next patch.
The next patch doesn't mention _DSD either. Is it obfuscated by
being inside fwnode_property_read_string()? If so, it's well and
truly hidden; I gave up trying to connect that with ACPI.
Bjorn
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
@ 2021-08-09 17:42 ` Bjorn Helgaas
0 siblings, 0 replies; 42+ messages in thread
From: Bjorn Helgaas @ 2021-08-09 17:42 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
On Fri, Aug 06, 2021 at 09:55:27PM -0500, Jeremy Linton wrote:
> Hi,
>
> On 8/6/21 5:21 PM, Bjorn Helgaas wrote:
> > On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
> > > The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
> > > standard because its PCIe config space isn't ECAM compliant and is
> > > split into two parts. One part for the root port registers and a
> > > moveable window which points at a given device's 4K config space.
> > > Thus it doesn't have a MCFG (and really any MCFG provided would be
> > > nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
> > > we key off a Linux specific host bridge _DSD to add custom ECAM
> > > ops and cfgres. The cfg op selects between those two regions, as
> > > well as disallowing problematic accesses, particularly if the link
> > > is down because there isn't an attached device.
> >
> > I'm not sure SMCCC is *really* relevant here. If it is, an expansion
> > of the acronym and a link to a spec would be helpful.
> >
> > But AFAICT the only important thing here is that it doesn't have
> > standard ECAM, and we're going to work around that.
>
> I will reword it a bit.
>
> > I don't see anything about _DSD in this series.
>
> That is the "linux,pci-quirk" in the next patch.
The next patch doesn't mention _DSD either. Is it obfuscated by
being inside fwnode_property_read_string()? If so, it's well and
truly hidden; I gave up trying to connect that with ACPI.
Bjorn
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
2021-08-09 17:42 ` Bjorn Helgaas
@ 2021-08-09 19:48 ` Jeremy Linton
-1 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-09 19:48 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi,
On 8/9/21 12:42 PM, Bjorn Helgaas wrote:
> On Fri, Aug 06, 2021 at 09:55:27PM -0500, Jeremy Linton wrote:
>> Hi,
>>
>> On 8/6/21 5:21 PM, Bjorn Helgaas wrote:
>>> On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
>>>> The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
>>>> standard because its PCIe config space isn't ECAM compliant and is
>>>> split into two parts. One part for the root port registers and a
>>>> moveable window which points at a given device's 4K config space.
>>>> Thus it doesn't have a MCFG (and really any MCFG provided would be
>>>> nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
>>>> we key off a Linux specific host bridge _DSD to add custom ECAM
>>>> ops and cfgres. The cfg op selects between those two regions, as
>>>> well as disallowing problematic accesses, particularly if the link
>>>> is down because there isn't an attached device.
>>>
>>> I'm not sure SMCCC is *really* relevant here. If it is, an expansion
>>> of the acronym and a link to a spec would be helpful.
>>>
>>> But AFAICT the only important thing here is that it doesn't have
>>> standard ECAM, and we're going to work around that.
>>
>> I will reword it a bit.
>>
>>> I don't see anything about _DSD in this series.
>>
>> That is the "linux,pci-quirk" in the next patch.
>
> The next patch doesn't mention _DSD either. Is it obfuscated by
> being inside fwnode_property_read_string()? If so, it's well and
> truly hidden; I gave up trying to connect that with ACPI.
Right, the fwnode stuff works as a DT/ACPI abstraction for reading
values from firmware tables. In this case the ACPI definition looks
something like:
Device(PCI0) {
...
Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
Package () { "linux-pcie-quirk", "bcm2711" },
}
})
...
}
Which explains a bit of why the underlying code is a bit uh... complicated.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
@ 2021-08-09 19:48 ` Jeremy Linton
0 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-09 19:48 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi,
On 8/9/21 12:42 PM, Bjorn Helgaas wrote:
> On Fri, Aug 06, 2021 at 09:55:27PM -0500, Jeremy Linton wrote:
>> Hi,
>>
>> On 8/6/21 5:21 PM, Bjorn Helgaas wrote:
>>> On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
>>>> The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
>>>> standard because its PCIe config space isn't ECAM compliant and is
>>>> split into two parts. One part for the root port registers and a
>>>> moveable window which points at a given device's 4K config space.
>>>> Thus it doesn't have a MCFG (and really any MCFG provided would be
>>>> nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
>>>> we key off a Linux specific host bridge _DSD to add custom ECAM
>>>> ops and cfgres. The cfg op selects between those two regions, as
>>>> well as disallowing problematic accesses, particularly if the link
>>>> is down because there isn't an attached device.
>>>
>>> I'm not sure SMCCC is *really* relevant here. If it is, an expansion
>>> of the acronym and a link to a spec would be helpful.
>>>
>>> But AFAICT the only important thing here is that it doesn't have
>>> standard ECAM, and we're going to work around that.
>>
>> I will reword it a bit.
>>
>>> I don't see anything about _DSD in this series.
>>
>> That is the "linux,pci-quirk" in the next patch.
>
> The next patch doesn't mention _DSD either. Is it obfuscated by
> being inside fwnode_property_read_string()? If so, it's well and
> truly hidden; I gave up trying to connect that with ACPI.
Right, the fwnode stuff works as a DT/ACPI abstraction for reading
values from firmware tables. In this case the ACPI definition looks
something like:
Device(PCI0) {
...
Name (_DSD, Package () {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
Package () { "linux-pcie-quirk", "bcm2711" },
}
})
...
}
Which explains a bit of why the underlying code is a bit uh... complicated.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
2021-08-09 19:48 ` Jeremy Linton
@ 2021-08-09 20:33 ` Bjorn Helgaas
-1 siblings, 0 replies; 42+ messages in thread
From: Bjorn Helgaas @ 2021-08-09 20:33 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
On Mon, Aug 09, 2021 at 02:48:17PM -0500, Jeremy Linton wrote:
> Hi,
>
> On 8/9/21 12:42 PM, Bjorn Helgaas wrote:
> > On Fri, Aug 06, 2021 at 09:55:27PM -0500, Jeremy Linton wrote:
> > > Hi,
> > >
> > > On 8/6/21 5:21 PM, Bjorn Helgaas wrote:
> > > > On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
> > > > > The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
> > > > > standard because its PCIe config space isn't ECAM compliant and is
> > > > > split into two parts. One part for the root port registers and a
> > > > > moveable window which points at a given device's 4K config space.
> > > > > Thus it doesn't have a MCFG (and really any MCFG provided would be
> > > > > nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
> > > > > we key off a Linux specific host bridge _DSD to add custom ECAM
> > > > > ops and cfgres. The cfg op selects between those two regions, as
> > > > > well as disallowing problematic accesses, particularly if the link
> > > > > is down because there isn't an attached device.
> > > >
> > > > I'm not sure SMCCC is *really* relevant here. If it is, an expansion
> > > > of the acronym and a link to a spec would be helpful.
> > > >
> > > > But AFAICT the only important thing here is that it doesn't have
> > > > standard ECAM, and we're going to work around that.
> > >
> > > I will reword it a bit.
> > >
> > > > I don't see anything about _DSD in this series.
> > >
> > > That is the "linux,pci-quirk" in the next patch.
> >
> > The next patch doesn't mention _DSD either. Is it obfuscated by
> > being inside fwnode_property_read_string()? If so, it's well and
> > truly hidden; I gave up trying to connect that with ACPI.
>
> Right, the fwnode stuff works as a DT/ACPI abstraction for reading values
> from firmware tables. In this case the ACPI definition looks something like:
>
> Device(PCI0) {
> ...
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package () { "linux-pcie-quirk", "bcm2711" },
> }
> })
>
> ...
> }
>
> Which explains a bit of why the underlying code is a bit uh... complicated.
Wow, that's ... special.
I think I would include "ecam" or something in the name. There might
be a variety of quirks, e.g., "P2PDMA allowed between root ports",
that could reasonably fit under "linux-pcie-quirk".
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
@ 2021-08-09 20:33 ` Bjorn Helgaas
0 siblings, 0 replies; 42+ messages in thread
From: Bjorn Helgaas @ 2021-08-09 20:33 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
On Mon, Aug 09, 2021 at 02:48:17PM -0500, Jeremy Linton wrote:
> Hi,
>
> On 8/9/21 12:42 PM, Bjorn Helgaas wrote:
> > On Fri, Aug 06, 2021 at 09:55:27PM -0500, Jeremy Linton wrote:
> > > Hi,
> > >
> > > On 8/6/21 5:21 PM, Bjorn Helgaas wrote:
> > > > On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
> > > > > The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
> > > > > standard because its PCIe config space isn't ECAM compliant and is
> > > > > split into two parts. One part for the root port registers and a
> > > > > moveable window which points at a given device's 4K config space.
> > > > > Thus it doesn't have a MCFG (and really any MCFG provided would be
> > > > > nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
> > > > > we key off a Linux specific host bridge _DSD to add custom ECAM
> > > > > ops and cfgres. The cfg op selects between those two regions, as
> > > > > well as disallowing problematic accesses, particularly if the link
> > > > > is down because there isn't an attached device.
> > > >
> > > > I'm not sure SMCCC is *really* relevant here. If it is, an expansion
> > > > of the acronym and a link to a spec would be helpful.
> > > >
> > > > But AFAICT the only important thing here is that it doesn't have
> > > > standard ECAM, and we're going to work around that.
> > >
> > > I will reword it a bit.
> > >
> > > > I don't see anything about _DSD in this series.
> > >
> > > That is the "linux,pci-quirk" in the next patch.
> >
> > The next patch doesn't mention _DSD either. Is it obfuscated by
> > being inside fwnode_property_read_string()? If so, it's well and
> > truly hidden; I gave up trying to connect that with ACPI.
>
> Right, the fwnode stuff works as a DT/ACPI abstraction for reading values
> from firmware tables. In this case the ACPI definition looks something like:
>
> Device(PCI0) {
> ...
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package () { "linux-pcie-quirk", "bcm2711" },
> }
> })
>
> ...
> }
>
> Which explains a bit of why the underlying code is a bit uh... complicated.
Wow, that's ... special.
I think I would include "ecam" or something in the name. There might
be a variety of quirks, e.g., "P2PDMA allowed between root ports",
that could reasonably fit under "linux-pcie-quirk".
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
2021-08-09 20:33 ` Bjorn Helgaas
@ 2021-08-09 21:21 ` Jeremy Linton
-1 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-09 21:21 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi,
On 8/9/21 3:33 PM, Bjorn Helgaas wrote:
> On Mon, Aug 09, 2021 at 02:48:17PM -0500, Jeremy Linton wrote:
>> Hi,
>>
>> On 8/9/21 12:42 PM, Bjorn Helgaas wrote:
>>> On Fri, Aug 06, 2021 at 09:55:27PM -0500, Jeremy Linton wrote:
>>>> Hi,
>>>>
>>>> On 8/6/21 5:21 PM, Bjorn Helgaas wrote:
>>>>> On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
>>>>>> The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
>>>>>> standard because its PCIe config space isn't ECAM compliant and is
>>>>>> split into two parts. One part for the root port registers and a
>>>>>> moveable window which points at a given device's 4K config space.
>>>>>> Thus it doesn't have a MCFG (and really any MCFG provided would be
>>>>>> nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
>>>>>> we key off a Linux specific host bridge _DSD to add custom ECAM
>>>>>> ops and cfgres. The cfg op selects between those two regions, as
>>>>>> well as disallowing problematic accesses, particularly if the link
>>>>>> is down because there isn't an attached device.
>>>>>
>>>>> I'm not sure SMCCC is *really* relevant here. If it is, an expansion
>>>>> of the acronym and a link to a spec would be helpful.
>>>>>
>>>>> But AFAICT the only important thing here is that it doesn't have
>>>>> standard ECAM, and we're going to work around that.
>>>>
>>>> I will reword it a bit.
>>>>
>>>>> I don't see anything about _DSD in this series.
>>>>
>>>> That is the "linux,pci-quirk" in the next patch.
>>>
>>> The next patch doesn't mention _DSD either. Is it obfuscated by
>>> being inside fwnode_property_read_string()? If so, it's well and
>>> truly hidden; I gave up trying to connect that with ACPI.
>>
>> Right, the fwnode stuff works as a DT/ACPI abstraction for reading values
>> from firmware tables. In this case the ACPI definition looks something like:
>>
>> Device(PCI0) {
>> ...
>> Name (_DSD, Package () {
>> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>> Package () {
>> Package () { "linux-pcie-quirk", "bcm2711" },
>> }
>> })
>>
>> ...
>> }
>>
>> Which explains a bit of why the underlying code is a bit uh... complicated.
>
> Wow, that's ... special.
>
> I think I would include "ecam" or something in the name. There might
> be a variety of quirks, e.g., "P2PDMA allowed between root ports",
> that could reasonably fit under "linux-pcie-quirk".
>
I think I mentioned "linux-ecam-quirk-id" in the bit with Rob. How is that?
I think the description would be something roughly: MCFG oem id override
string which selects a platform specific ECAM accessor quirk.
Thanks,
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 2/3] PCI: brcmstb: Add ACPI config space quirk
@ 2021-08-09 21:21 ` Jeremy Linton
0 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-09 21:21 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi,
On 8/9/21 3:33 PM, Bjorn Helgaas wrote:
> On Mon, Aug 09, 2021 at 02:48:17PM -0500, Jeremy Linton wrote:
>> Hi,
>>
>> On 8/9/21 12:42 PM, Bjorn Helgaas wrote:
>>> On Fri, Aug 06, 2021 at 09:55:27PM -0500, Jeremy Linton wrote:
>>>> Hi,
>>>>
>>>> On 8/6/21 5:21 PM, Bjorn Helgaas wrote:
>>>>> On Thu, Aug 05, 2021 at 04:11:59PM -0500, Jeremy Linton wrote:
>>>>>> The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
>>>>>> standard because its PCIe config space isn't ECAM compliant and is
>>>>>> split into two parts. One part for the root port registers and a
>>>>>> moveable window which points at a given device's 4K config space.
>>>>>> Thus it doesn't have a MCFG (and really any MCFG provided would be
>>>>>> nonsense anyway). As Linux doesn't support the PCIe SMCCC standard
>>>>>> we key off a Linux specific host bridge _DSD to add custom ECAM
>>>>>> ops and cfgres. The cfg op selects between those two regions, as
>>>>>> well as disallowing problematic accesses, particularly if the link
>>>>>> is down because there isn't an attached device.
>>>>>
>>>>> I'm not sure SMCCC is *really* relevant here. If it is, an expansion
>>>>> of the acronym and a link to a spec would be helpful.
>>>>>
>>>>> But AFAICT the only important thing here is that it doesn't have
>>>>> standard ECAM, and we're going to work around that.
>>>>
>>>> I will reword it a bit.
>>>>
>>>>> I don't see anything about _DSD in this series.
>>>>
>>>> That is the "linux,pci-quirk" in the next patch.
>>>
>>> The next patch doesn't mention _DSD either. Is it obfuscated by
>>> being inside fwnode_property_read_string()? If so, it's well and
>>> truly hidden; I gave up trying to connect that with ACPI.
>>
>> Right, the fwnode stuff works as a DT/ACPI abstraction for reading values
>> from firmware tables. In this case the ACPI definition looks something like:
>>
>> Device(PCI0) {
>> ...
>> Name (_DSD, Package () {
>> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>> Package () {
>> Package () { "linux-pcie-quirk", "bcm2711" },
>> }
>> })
>>
>> ...
>> }
>>
>> Which explains a bit of why the underlying code is a bit uh... complicated.
>
> Wow, that's ... special.
>
> I think I would include "ecam" or something in the name. There might
> be a variety of quirks, e.g., "P2PDMA allowed between root ports",
> that could reasonably fit under "linux-pcie-quirk".
>
I think I mentioned "linux-ecam-quirk-id" in the bit with Rob. How is that?
I think the description would be something roughly: MCFG oem id override
string which selects a platform specific ECAM accessor quirk.
Thanks,
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
2021-08-05 21:11 ` Jeremy Linton
@ 2021-08-05 21:12 ` Jeremy Linton
-1 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-05 21:12 UTC (permalink / raw)
To: linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel, Jeremy Linton
Now that we have a bcm2711 quirk, we need to be able to
detect it when the MCFG is missing. Use a namespace
property as an alternative to the MCFG OEM.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index 53cab975f612..7d77fc72c2a4 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
ALTRA_ECAM_QUIRK(1, 13),
ALTRA_ECAM_QUIRK(1, 14),
ALTRA_ECAM_QUIRK(1, 15),
+
+ { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
+ DEFINE_RES_MEM(0xFD500000, 0xA000) },
};
static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
@@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
u16 segment = root->segment;
struct resource *bus_range = &root->secondary;
struct mcfg_fixup *f;
+ const char *soc;
int i;
+ /*
+ * This could be a machine with a PCI/SMC conduit,
+ * which means it doens't have MCFG. Get the machineid from
+ * the namespace definition instead.
+ */
+ if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
+ "linux,pcie-quirk", &soc)) {
+ memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
+ }
+
for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
if (pci_mcfg_quirk_matches(f, segment, bus_range)) {
if (f->cfgres.start)
--
2.31.1
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
@ 2021-08-05 21:12 ` Jeremy Linton
0 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-05 21:12 UTC (permalink / raw)
To: linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel, Jeremy Linton
Now that we have a bcm2711 quirk, we need to be able to
detect it when the MCFG is missing. Use a namespace
property as an alternative to the MCFG OEM.
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index 53cab975f612..7d77fc72c2a4 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
ALTRA_ECAM_QUIRK(1, 13),
ALTRA_ECAM_QUIRK(1, 14),
ALTRA_ECAM_QUIRK(1, 15),
+
+ { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
+ DEFINE_RES_MEM(0xFD500000, 0xA000) },
};
static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
@@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
u16 segment = root->segment;
struct resource *bus_range = &root->secondary;
struct mcfg_fixup *f;
+ const char *soc;
int i;
+ /*
+ * This could be a machine with a PCI/SMC conduit,
+ * which means it doens't have MCFG. Get the machineid from
+ * the namespace definition instead.
+ */
+ if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
+ "linux,pcie-quirk", &soc)) {
+ memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
+ }
+
for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
if (pci_mcfg_quirk_matches(f, segment, bus_range)) {
if (f->cfgres.start)
--
2.31.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
2021-08-05 21:12 ` Jeremy Linton
@ 2021-08-06 22:12 ` Bjorn Helgaas
-1 siblings, 0 replies; 42+ messages in thread
From: Bjorn Helgaas @ 2021-08-06 22:12 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
In subject, this or similar would match history:
PCI/ACPI: Add Broadcom bcm2711 MCFG quirk
On Thu, Aug 05, 2021 at 04:12:00PM -0500, Jeremy Linton wrote:
> Now that we have a bcm2711 quirk, we need to be able to
> detect it when the MCFG is missing. Use a namespace
> property as an alternative to the MCFG OEM.
Rewrap to use ~75 columns.
Mention the DT namespace property here.
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index 53cab975f612..7d77fc72c2a4 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
> ALTRA_ECAM_QUIRK(1, 13),
> ALTRA_ECAM_QUIRK(1, 14),
> ALTRA_ECAM_QUIRK(1, 15),
> +
> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
> };
>
> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
> u16 segment = root->segment;
> struct resource *bus_range = &root->secondary;
> struct mcfg_fixup *f;
> + const char *soc;
> int i;
>
> + /*
> + * This could be a machine with a PCI/SMC conduit,
> + * which means it doens't have MCFG. Get the machineid from
> + * the namespace definition instead.
s/SMC/SMCCC/ ? Cover letter uses SMCCC (not sure it's relevant anyway)
s/doens't/doesn't/
Rewrap comment to use ~80 columns.
Seems pretty reasonable that a platform without standard ECAM might
not have MCFG, since MCFG basically implies ECAM.
Is "linux,pcie-quirk" the right property to look for? It doesn't
sound very generic, and it doesn't sound like anything related to
ECAM. Is it new? I don't see it in the tree yet. Should it be in
Documentation/devicetree/bindings/pci/pci.txt so we don't get a
different property name for every new platform?
> + */
> + if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
> + "linux,pcie-quirk", &soc)) {
> + memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
> + }
> +
> for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
> if (pci_mcfg_quirk_matches(f, segment, bus_range)) {
> if (f->cfgres.start)
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
@ 2021-08-06 22:12 ` Bjorn Helgaas
0 siblings, 0 replies; 42+ messages in thread
From: Bjorn Helgaas @ 2021-08-06 22:12 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
In subject, this or similar would match history:
PCI/ACPI: Add Broadcom bcm2711 MCFG quirk
On Thu, Aug 05, 2021 at 04:12:00PM -0500, Jeremy Linton wrote:
> Now that we have a bcm2711 quirk, we need to be able to
> detect it when the MCFG is missing. Use a namespace
> property as an alternative to the MCFG OEM.
Rewrap to use ~75 columns.
Mention the DT namespace property here.
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index 53cab975f612..7d77fc72c2a4 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
> ALTRA_ECAM_QUIRK(1, 13),
> ALTRA_ECAM_QUIRK(1, 14),
> ALTRA_ECAM_QUIRK(1, 15),
> +
> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
> };
>
> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
> u16 segment = root->segment;
> struct resource *bus_range = &root->secondary;
> struct mcfg_fixup *f;
> + const char *soc;
> int i;
>
> + /*
> + * This could be a machine with a PCI/SMC conduit,
> + * which means it doens't have MCFG. Get the machineid from
> + * the namespace definition instead.
s/SMC/SMCCC/ ? Cover letter uses SMCCC (not sure it's relevant anyway)
s/doens't/doesn't/
Rewrap comment to use ~80 columns.
Seems pretty reasonable that a platform without standard ECAM might
not have MCFG, since MCFG basically implies ECAM.
Is "linux,pcie-quirk" the right property to look for? It doesn't
sound very generic, and it doesn't sound like anything related to
ECAM. Is it new? I don't see it in the tree yet. Should it be in
Documentation/devicetree/bindings/pci/pci.txt so we don't get a
different property name for every new platform?
> + */
> + if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
> + "linux,pcie-quirk", &soc)) {
> + memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
> + }
> +
> for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
> if (pci_mcfg_quirk_matches(f, segment, bus_range)) {
> if (f->cfgres.start)
> --
> 2.31.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
2021-08-06 22:12 ` Bjorn Helgaas
@ 2021-08-07 0:34 ` Jeremy Linton
-1 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-07 0:34 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi,
Thanks for looking at this.
On 8/6/21 5:12 PM, Bjorn Helgaas wrote:
> In subject, this or similar would match history:
>
> PCI/ACPI: Add Broadcom bcm2711 MCFG quirk
>
> On Thu, Aug 05, 2021 at 04:12:00PM -0500, Jeremy Linton wrote:
>> Now that we have a bcm2711 quirk, we need to be able to
>> detect it when the MCFG is missing. Use a namespace
>> property as an alternative to the MCFG OEM.
>
> Rewrap to use ~75 columns.
>
> Mention the DT namespace property here.
>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index 53cab975f612..7d77fc72c2a4 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
>> ALTRA_ECAM_QUIRK(1, 13),
>> ALTRA_ECAM_QUIRK(1, 14),
>> ALTRA_ECAM_QUIRK(1, 15),
>> +
>> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
>> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
>> };
>>
>> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
>> u16 segment = root->segment;
>> struct resource *bus_range = &root->secondary;
>> struct mcfg_fixup *f;
>> + const char *soc;
>> int i;
>>
>> + /*
>> + * This could be a machine with a PCI/SMC conduit,
>> + * which means it doens't have MCFG. Get the machineid from
>> + * the namespace definition instead.
>
> s/SMC/SMCCC/ ? Cover letter uses SMCCC (not sure it's relevant anyway)
> s/doens't/doesn't/
>
> Rewrap comment to use ~80 columns.
>
> Seems pretty reasonable that a platform without standard ECAM might
> not have MCFG, since MCFG basically implies ECAM.
Sure, on all the above comments.
>
> Is "linux,pcie-quirk" the right property to look for? It doesn't
> sound very generic, and it doesn't sound like anything related to
> ECAM. Is it new? I don't see it in the tree yet. Should it be in
> Documentation/devicetree/bindings/pci/pci.txt so we don't get a
> different property name for every new platform?
Yes, I made it up. Someone else commented about the "linux," partially
because it should be "linux-" to conform with
https://github.com/UEFI/DSD-Guide. But also in the same context of it
being linux specific. I think that guide is where it should end up,
rather than the devicetree bindings.
I guess we can request addition to the uefi- but that seems like a
mistake this is really (hopefully?) a Linux specific properly as other
OS's will simply use the SMC. I think we could request another prefix if
we come up with a good one and think it belongs in that guide.
>
>> + */
>> + if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
>> + "linux,pcie-quirk", &soc)) {
>> + memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
>> + }
>> +
>> for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
>> if (pci_mcfg_quirk_matches(f, segment, bus_range)) {
>> if (f->cfgres.start)
>> --
>> 2.31.1
>>
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
@ 2021-08-07 0:34 ` Jeremy Linton
0 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-07 0:34 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi,
Thanks for looking at this.
On 8/6/21 5:12 PM, Bjorn Helgaas wrote:
> In subject, this or similar would match history:
>
> PCI/ACPI: Add Broadcom bcm2711 MCFG quirk
>
> On Thu, Aug 05, 2021 at 04:12:00PM -0500, Jeremy Linton wrote:
>> Now that we have a bcm2711 quirk, we need to be able to
>> detect it when the MCFG is missing. Use a namespace
>> property as an alternative to the MCFG OEM.
>
> Rewrap to use ~75 columns.
>
> Mention the DT namespace property here.
>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index 53cab975f612..7d77fc72c2a4 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
>> ALTRA_ECAM_QUIRK(1, 13),
>> ALTRA_ECAM_QUIRK(1, 14),
>> ALTRA_ECAM_QUIRK(1, 15),
>> +
>> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
>> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
>> };
>>
>> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
>> u16 segment = root->segment;
>> struct resource *bus_range = &root->secondary;
>> struct mcfg_fixup *f;
>> + const char *soc;
>> int i;
>>
>> + /*
>> + * This could be a machine with a PCI/SMC conduit,
>> + * which means it doens't have MCFG. Get the machineid from
>> + * the namespace definition instead.
>
> s/SMC/SMCCC/ ? Cover letter uses SMCCC (not sure it's relevant anyway)
> s/doens't/doesn't/
>
> Rewrap comment to use ~80 columns.
>
> Seems pretty reasonable that a platform without standard ECAM might
> not have MCFG, since MCFG basically implies ECAM.
Sure, on all the above comments.
>
> Is "linux,pcie-quirk" the right property to look for? It doesn't
> sound very generic, and it doesn't sound like anything related to
> ECAM. Is it new? I don't see it in the tree yet. Should it be in
> Documentation/devicetree/bindings/pci/pci.txt so we don't get a
> different property name for every new platform?
Yes, I made it up. Someone else commented about the "linux," partially
because it should be "linux-" to conform with
https://github.com/UEFI/DSD-Guide. But also in the same context of it
being linux specific. I think that guide is where it should end up,
rather than the devicetree bindings.
I guess we can request addition to the uefi- but that seems like a
mistake this is really (hopefully?) a Linux specific properly as other
OS's will simply use the SMC. I think we could request another prefix if
we come up with a good one and think it belongs in that guide.
>
>> + */
>> + if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
>> + "linux,pcie-quirk", &soc)) {
>> + memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
>> + }
>> +
>> for (i = 0, f = mcfg_quirks; i < ARRAY_SIZE(mcfg_quirks); i++, f++) {
>> if (pci_mcfg_quirk_matches(f, segment, bus_range)) {
>> if (f->cfgres.start)
>> --
>> 2.31.1
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
2021-08-07 0:34 ` Jeremy Linton
@ 2021-08-09 15:27 ` Rob Herring
-1 siblings, 0 replies; 42+ messages in thread
From: Rob Herring @ 2021-08-09 15:27 UTC (permalink / raw)
To: Jeremy Linton
Cc: Bjorn Helgaas, PCI, Lorenzo Pieralisi, Nicolas Saenz Julienne,
Bjorn Helgaas, Rafael J. Wysocki, Len Brown,
Krzysztof Wilczynski, Florian Fainelli,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:ACPI FOR ARM64 (ACPI/arm64),
linux-arm-kernel,
moderated list:BROADCOM BCM2835 ARM ARCHITECTURE, linux-kernel
On Fri, Aug 6, 2021 at 6:35 PM Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Hi,
>
> Thanks for looking at this.
>
> On 8/6/21 5:12 PM, Bjorn Helgaas wrote:
> > In subject, this or similar would match history:
> >
> > PCI/ACPI: Add Broadcom bcm2711 MCFG quirk
> >
> > On Thu, Aug 05, 2021 at 04:12:00PM -0500, Jeremy Linton wrote:
> >> Now that we have a bcm2711 quirk, we need to be able to
> >> detect it when the MCFG is missing. Use a namespace
> >> property as an alternative to the MCFG OEM.
> >
> > Rewrap to use ~75 columns.
> >
> > Mention the DT namespace property here.
> >
> >> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> >> ---
> >> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
> >> 1 file changed, 14 insertions(+)
> >>
> >> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> >> index 53cab975f612..7d77fc72c2a4 100644
> >> --- a/drivers/acpi/pci_mcfg.c
> >> +++ b/drivers/acpi/pci_mcfg.c
> >> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
> >> ALTRA_ECAM_QUIRK(1, 13),
> >> ALTRA_ECAM_QUIRK(1, 14),
> >> ALTRA_ECAM_QUIRK(1, 15),
> >> +
> >> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
> >> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
> >> };
> >>
> >> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> >> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
> >> u16 segment = root->segment;
> >> struct resource *bus_range = &root->secondary;
> >> struct mcfg_fixup *f;
> >> + const char *soc;
> >> int i;
> >>
> >> + /*
> >> + * This could be a machine with a PCI/SMC conduit,
> >> + * which means it doens't have MCFG. Get the machineid from
> >> + * the namespace definition instead.
> >
> > s/SMC/SMCCC/ ? Cover letter uses SMCCC (not sure it's relevant anyway)
> > s/doens't/doesn't/
> >
> > Rewrap comment to use ~80 columns.
> >
> > Seems pretty reasonable that a platform without standard ECAM might
> > not have MCFG, since MCFG basically implies ECAM.
>
>
> Sure, on all the above comments.
>
> >
> > Is "linux,pcie-quirk" the right property to look for? It doesn't
> > sound very generic, and it doesn't sound like anything related to
> > ECAM. Is it new? I don't see it in the tree yet. Should it be in
> > Documentation/devicetree/bindings/pci/pci.txt so we don't get a
> > different property name for every new platform?
No, it should not be in pci.txt. Nothing to do with DT and I don't
review ACPI bindings (someone should) if I can help it.
> Yes, I made it up. Someone else commented about the "linux," partially
> because it should be "linux-" to conform with
> https://github.com/UEFI/DSD-Guide. But also in the same context of it
> being linux specific. I think that guide is where it should end up,
> rather than the devicetree bindings.
>
> I guess we can request addition to the uefi- but that seems like a
> mistake this is really (hopefully?) a Linux specific properly as other
> OS's will simply use the SMC. I think we could request another prefix if
> we come up with a good one and think it belongs in that guide.
It's only Linux specific until it's not.
What happens when there's a second PCIe quirk here? Say what the quirk
is (and not in terms of Linux policy).
Don't you know the device here and can imply all this (like implying
off of 'compatible' in DT)? Adding properties to address issues
creates compatibility issues. Maybe not an issue in this case if the
firmware is not stable, but not a good practice to be in.
Rob
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
@ 2021-08-09 15:27 ` Rob Herring
0 siblings, 0 replies; 42+ messages in thread
From: Rob Herring @ 2021-08-09 15:27 UTC (permalink / raw)
To: Jeremy Linton
Cc: Bjorn Helgaas, PCI, Lorenzo Pieralisi, Nicolas Saenz Julienne,
Bjorn Helgaas, Rafael J. Wysocki, Len Brown,
Krzysztof Wilczynski, Florian Fainelli,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:ACPI FOR ARM64 (ACPI/arm64),
linux-arm-kernel,
moderated list:BROADCOM BCM2835 ARM ARCHITECTURE, linux-kernel
On Fri, Aug 6, 2021 at 6:35 PM Jeremy Linton <jeremy.linton@arm.com> wrote:
>
> Hi,
>
> Thanks for looking at this.
>
> On 8/6/21 5:12 PM, Bjorn Helgaas wrote:
> > In subject, this or similar would match history:
> >
> > PCI/ACPI: Add Broadcom bcm2711 MCFG quirk
> >
> > On Thu, Aug 05, 2021 at 04:12:00PM -0500, Jeremy Linton wrote:
> >> Now that we have a bcm2711 quirk, we need to be able to
> >> detect it when the MCFG is missing. Use a namespace
> >> property as an alternative to the MCFG OEM.
> >
> > Rewrap to use ~75 columns.
> >
> > Mention the DT namespace property here.
> >
> >> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> >> ---
> >> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
> >> 1 file changed, 14 insertions(+)
> >>
> >> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> >> index 53cab975f612..7d77fc72c2a4 100644
> >> --- a/drivers/acpi/pci_mcfg.c
> >> +++ b/drivers/acpi/pci_mcfg.c
> >> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
> >> ALTRA_ECAM_QUIRK(1, 13),
> >> ALTRA_ECAM_QUIRK(1, 14),
> >> ALTRA_ECAM_QUIRK(1, 15),
> >> +
> >> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
> >> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
> >> };
> >>
> >> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> >> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
> >> u16 segment = root->segment;
> >> struct resource *bus_range = &root->secondary;
> >> struct mcfg_fixup *f;
> >> + const char *soc;
> >> int i;
> >>
> >> + /*
> >> + * This could be a machine with a PCI/SMC conduit,
> >> + * which means it doens't have MCFG. Get the machineid from
> >> + * the namespace definition instead.
> >
> > s/SMC/SMCCC/ ? Cover letter uses SMCCC (not sure it's relevant anyway)
> > s/doens't/doesn't/
> >
> > Rewrap comment to use ~80 columns.
> >
> > Seems pretty reasonable that a platform without standard ECAM might
> > not have MCFG, since MCFG basically implies ECAM.
>
>
> Sure, on all the above comments.
>
> >
> > Is "linux,pcie-quirk" the right property to look for? It doesn't
> > sound very generic, and it doesn't sound like anything related to
> > ECAM. Is it new? I don't see it in the tree yet. Should it be in
> > Documentation/devicetree/bindings/pci/pci.txt so we don't get a
> > different property name for every new platform?
No, it should not be in pci.txt. Nothing to do with DT and I don't
review ACPI bindings (someone should) if I can help it.
> Yes, I made it up. Someone else commented about the "linux," partially
> because it should be "linux-" to conform with
> https://github.com/UEFI/DSD-Guide. But also in the same context of it
> being linux specific. I think that guide is where it should end up,
> rather than the devicetree bindings.
>
> I guess we can request addition to the uefi- but that seems like a
> mistake this is really (hopefully?) a Linux specific properly as other
> OS's will simply use the SMC. I think we could request another prefix if
> we come up with a good one and think it belongs in that guide.
It's only Linux specific until it's not.
What happens when there's a second PCIe quirk here? Say what the quirk
is (and not in terms of Linux policy).
Don't you know the device here and can imply all this (like implying
off of 'compatible' in DT)? Adding properties to address issues
creates compatibility issues. Maybe not an issue in this case if the
firmware is not stable, but not a good practice to be in.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
2021-08-09 15:27 ` Rob Herring
@ 2021-08-09 16:24 ` Jeremy Linton
-1 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-09 16:24 UTC (permalink / raw)
To: Rob Herring
Cc: Bjorn Helgaas, PCI, Lorenzo Pieralisi, Nicolas Saenz Julienne,
Bjorn Helgaas, Rafael J. Wysocki, Len Brown,
Krzysztof Wilczynski, Florian Fainelli,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:ACPI FOR ARM64 (ACPI/arm64),
linux-arm-kernel,
moderated list:BROADCOM BCM2835 ARM ARCHITECTURE, linux-kernel
Hi,
Thanks for looking at this.
On 8/9/21 10:27 AM, Rob Herring wrote:
> On Fri, Aug 6, 2021 at 6:35 PM Jeremy Linton <jeremy.linton@arm.com> wrote:
>>
>> Hi,
>>
>> Thanks for looking at this.
>>
>> On 8/6/21 5:12 PM, Bjorn Helgaas wrote:
>>> In subject, this or similar would match history:
>>>
>>> PCI/ACPI: Add Broadcom bcm2711 MCFG quirk
>>>
>>> On Thu, Aug 05, 2021 at 04:12:00PM -0500, Jeremy Linton wrote:
>>>> Now that we have a bcm2711 quirk, we need to be able to
>>>> detect it when the MCFG is missing. Use a namespace
>>>> property as an alternative to the MCFG OEM.
>>>
>>> Rewrap to use ~75 columns.
>>>
>>> Mention the DT namespace property here.
>>>
>>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>> ---
>>>> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
>>>> 1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>>> index 53cab975f612..7d77fc72c2a4 100644
>>>> --- a/drivers/acpi/pci_mcfg.c
>>>> +++ b/drivers/acpi/pci_mcfg.c
>>>> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
>>>> ALTRA_ECAM_QUIRK(1, 13),
>>>> ALTRA_ECAM_QUIRK(1, 14),
>>>> ALTRA_ECAM_QUIRK(1, 15),
>>>> +
>>>> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
>>>> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
>>>> };
>>>>
>>>> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>>>> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
>>>> u16 segment = root->segment;
>>>> struct resource *bus_range = &root->secondary;
>>>> struct mcfg_fixup *f;
>>>> + const char *soc;
>>>> int i;
>>>>
>>>> + /*
>>>> + * This could be a machine with a PCI/SMC conduit,
>>>> + * which means it doens't have MCFG. Get the machineid from
>>>> + * the namespace definition instead.
>>>
>>> s/SMC/SMCCC/ ? Cover letter uses SMCCC (not sure it's relevant anyway)
>>> s/doens't/doesn't/
>>>
>>> Rewrap comment to use ~80 columns.
>>>
>>> Seems pretty reasonable that a platform without standard ECAM might
>>> not have MCFG, since MCFG basically implies ECAM.
>>
>>
>> Sure, on all the above comments.
>>
>>>
>>> Is "linux,pcie-quirk" the right property to look for? It doesn't
>>> sound very generic, and it doesn't sound like anything related to
>>> ECAM. Is it new? I don't see it in the tree yet. Should it be in
>>> Documentation/devicetree/bindings/pci/pci.txt so we don't get a
>>> different property name for every new platform?
>
> No, it should not be in pci.txt. Nothing to do with DT and I don't
> review ACPI bindings (someone should) if I can help it.
That is the intention of the uefi properties registry I referred to
earlier. It has a code first model too, which allows us to review it
here and then update the document with the property and the intended
behavior.
>
>> Yes, I made it up. Someone else commented about the "linux," partially
>> because it should be "linux-" to conform with
>> https://github.com/UEFI/DSD-Guide. But also in the same context of it
>> being linux specific. I think that guide is where it should end up,
>> rather than the devicetree bindings.
>>
>> I guess we can request addition to the uefi- but that seems like a
>> mistake this is really (hopefully?) a Linux specific properly as other
>> OS's will simply use the SMC. I think we could request another prefix if
>> we come up with a good one and think it belongs in that guide.
>
> It's only Linux specific until it's not.
>
> What happens when there's a second PCIe quirk here? Say what the quirk
> is (and not in terms of Linux policy).
This is really just a stand in for the existing MCFG oem id, its an
identifier to key off, nothing else. Maybe a better name is
"linux-ecam-quirk-id" or something to that effect?
>
> Don't you know the device here and can imply all this (like implying
> off of 'compatible' in DT)? Adding properties to address issues
> creates compatibility issues. Maybe not an issue in this case if the
> firmware is not stable, but not a good practice to be in.
Yes, and no, I think there was some discussion a few years back about
using non standard HID's for these nonstandard implementations, but that
was discouraged at the time in favor of using the standard identifiers
and and keying off a Soc/platform specific ID to enable a "quirk". Given
we are a bit far down that path and the decision was made to continue
down it (rather than solving much of it with a new firmware interface)
this seems like the straightforward solution. The vast majority of these
are SoC specific, and just minor tweaks for alignment/etc. Given its an
ACPI/UEFI machine we are already hiding a lot of the platform specific
behavior for configuring the bridge/etc that might require DT like
properties in the firmware.
Thanks again.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
@ 2021-08-09 16:24 ` Jeremy Linton
0 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-09 16:24 UTC (permalink / raw)
To: Rob Herring
Cc: Bjorn Helgaas, PCI, Lorenzo Pieralisi, Nicolas Saenz Julienne,
Bjorn Helgaas, Rafael J. Wysocki, Len Brown,
Krzysztof Wilczynski, Florian Fainelli,
maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
open list:ACPI FOR ARM64 (ACPI/arm64),
linux-arm-kernel,
moderated list:BROADCOM BCM2835 ARM ARCHITECTURE, linux-kernel
Hi,
Thanks for looking at this.
On 8/9/21 10:27 AM, Rob Herring wrote:
> On Fri, Aug 6, 2021 at 6:35 PM Jeremy Linton <jeremy.linton@arm.com> wrote:
>>
>> Hi,
>>
>> Thanks for looking at this.
>>
>> On 8/6/21 5:12 PM, Bjorn Helgaas wrote:
>>> In subject, this or similar would match history:
>>>
>>> PCI/ACPI: Add Broadcom bcm2711 MCFG quirk
>>>
>>> On Thu, Aug 05, 2021 at 04:12:00PM -0500, Jeremy Linton wrote:
>>>> Now that we have a bcm2711 quirk, we need to be able to
>>>> detect it when the MCFG is missing. Use a namespace
>>>> property as an alternative to the MCFG OEM.
>>>
>>> Rewrap to use ~75 columns.
>>>
>>> Mention the DT namespace property here.
>>>
>>>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>>>> ---
>>>> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
>>>> 1 file changed, 14 insertions(+)
>>>>
>>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>>> index 53cab975f612..7d77fc72c2a4 100644
>>>> --- a/drivers/acpi/pci_mcfg.c
>>>> +++ b/drivers/acpi/pci_mcfg.c
>>>> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
>>>> ALTRA_ECAM_QUIRK(1, 13),
>>>> ALTRA_ECAM_QUIRK(1, 14),
>>>> ALTRA_ECAM_QUIRK(1, 15),
>>>> +
>>>> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
>>>> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
>>>> };
>>>>
>>>> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>>>> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
>>>> u16 segment = root->segment;
>>>> struct resource *bus_range = &root->secondary;
>>>> struct mcfg_fixup *f;
>>>> + const char *soc;
>>>> int i;
>>>>
>>>> + /*
>>>> + * This could be a machine with a PCI/SMC conduit,
>>>> + * which means it doens't have MCFG. Get the machineid from
>>>> + * the namespace definition instead.
>>>
>>> s/SMC/SMCCC/ ? Cover letter uses SMCCC (not sure it's relevant anyway)
>>> s/doens't/doesn't/
>>>
>>> Rewrap comment to use ~80 columns.
>>>
>>> Seems pretty reasonable that a platform without standard ECAM might
>>> not have MCFG, since MCFG basically implies ECAM.
>>
>>
>> Sure, on all the above comments.
>>
>>>
>>> Is "linux,pcie-quirk" the right property to look for? It doesn't
>>> sound very generic, and it doesn't sound like anything related to
>>> ECAM. Is it new? I don't see it in the tree yet. Should it be in
>>> Documentation/devicetree/bindings/pci/pci.txt so we don't get a
>>> different property name for every new platform?
>
> No, it should not be in pci.txt. Nothing to do with DT and I don't
> review ACPI bindings (someone should) if I can help it.
That is the intention of the uefi properties registry I referred to
earlier. It has a code first model too, which allows us to review it
here and then update the document with the property and the intended
behavior.
>
>> Yes, I made it up. Someone else commented about the "linux," partially
>> because it should be "linux-" to conform with
>> https://github.com/UEFI/DSD-Guide. But also in the same context of it
>> being linux specific. I think that guide is where it should end up,
>> rather than the devicetree bindings.
>>
>> I guess we can request addition to the uefi- but that seems like a
>> mistake this is really (hopefully?) a Linux specific properly as other
>> OS's will simply use the SMC. I think we could request another prefix if
>> we come up with a good one and think it belongs in that guide.
>
> It's only Linux specific until it's not.
>
> What happens when there's a second PCIe quirk here? Say what the quirk
> is (and not in terms of Linux policy).
This is really just a stand in for the existing MCFG oem id, its an
identifier to key off, nothing else. Maybe a better name is
"linux-ecam-quirk-id" or something to that effect?
>
> Don't you know the device here and can imply all this (like implying
> off of 'compatible' in DT)? Adding properties to address issues
> creates compatibility issues. Maybe not an issue in this case if the
> firmware is not stable, but not a good practice to be in.
Yes, and no, I think there was some discussion a few years back about
using non standard HID's for these nonstandard implementations, but that
was discouraged at the time in favor of using the standard identifiers
and and keying off a Soc/platform specific ID to enable a "quirk". Given
we are a bit far down that path and the decision was made to continue
down it (rather than solving much of it with a new firmware interface)
this seems like the straightforward solution. The vast majority of these
are SoC specific, and just minor tweaks for alignment/etc. Given its an
ACPI/UEFI machine we are already hiding a lot of the platform specific
behavior for configuring the bridge/etc that might require DT like
properties in the firmware.
Thanks again.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
2021-08-05 21:12 ` Jeremy Linton
@ 2021-08-10 14:31 ` Shanker R Donthineni
-1 siblings, 0 replies; 42+ messages in thread
From: Shanker R Donthineni @ 2021-08-10 14:31 UTC (permalink / raw)
To: Jeremy Linton, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi Jeremy,
On 8/5/21 4:12 PM, Jeremy Linton wrote:
> Now that we have a bcm2711 quirk, we need to be able to
> detect it when the MCFG is missing. Use a namespace
> property as an alternative to the MCFG OEM.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index 53cab975f612..7d77fc72c2a4 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
> ALTRA_ECAM_QUIRK(1, 13),
> ALTRA_ECAM_QUIRK(1, 14),
> ALTRA_ECAM_QUIRK(1, 15),
> +
> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
> };
>
> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
> u16 segment = root->segment;
> struct resource *bus_range = &root->secondary;
> struct mcfg_fixup *f;
> + const char *soc;
> int i;
>
> + /*
> + * This could be a machine with a PCI/SMC conduit,
> + * which means it doens't have MCFG. Get the machineid from
> + * the namespace definition instead.
> + */
> + if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
> + "linux,pcie-quirk", &soc)) {
> + memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
> + }
> +
Is there any specific reason for not using the firmware agnostic API to get properties?
if (!device_property_read_string(root->device, "linux,pcie-quirk", &soc)) {
memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
}
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
@ 2021-08-10 14:31 ` Shanker R Donthineni
0 siblings, 0 replies; 42+ messages in thread
From: Shanker R Donthineni @ 2021-08-10 14:31 UTC (permalink / raw)
To: Jeremy Linton, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi Jeremy,
On 8/5/21 4:12 PM, Jeremy Linton wrote:
> Now that we have a bcm2711 quirk, we need to be able to
> detect it when the MCFG is missing. Use a namespace
> property as an alternative to the MCFG OEM.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index 53cab975f612..7d77fc72c2a4 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
> ALTRA_ECAM_QUIRK(1, 13),
> ALTRA_ECAM_QUIRK(1, 14),
> ALTRA_ECAM_QUIRK(1, 15),
> +
> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
> };
>
> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
> u16 segment = root->segment;
> struct resource *bus_range = &root->secondary;
> struct mcfg_fixup *f;
> + const char *soc;
> int i;
>
> + /*
> + * This could be a machine with a PCI/SMC conduit,
> + * which means it doens't have MCFG. Get the machineid from
> + * the namespace definition instead.
> + */
> + if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
> + "linux,pcie-quirk", &soc)) {
> + memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
> + }
> +
Is there any specific reason for not using the firmware agnostic API to get properties?
if (!device_property_read_string(root->device, "linux,pcie-quirk", &soc)) {
memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
}
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
2021-08-10 14:31 ` Shanker R Donthineni
@ 2021-08-10 14:47 ` Jeremy Linton
-1 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-10 14:47 UTC (permalink / raw)
To: Shanker R Donthineni, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi,
Thanks for looking at this!
On 8/10/21 9:31 AM, Shanker R Donthineni wrote:
> Hi Jeremy,
>
> On 8/5/21 4:12 PM, Jeremy Linton wrote:
>> Now that we have a bcm2711 quirk, we need to be able to
>> detect it when the MCFG is missing. Use a namespace
>> property as an alternative to the MCFG OEM.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index 53cab975f612..7d77fc72c2a4 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
>> ALTRA_ECAM_QUIRK(1, 13),
>> ALTRA_ECAM_QUIRK(1, 14),
>> ALTRA_ECAM_QUIRK(1, 15),
>> +
>> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
>> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
>> };
>>
>> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
>> u16 segment = root->segment;
>> struct resource *bus_range = &root->secondary;
>> struct mcfg_fixup *f;
>> + const char *soc;
>> int i;
>>
>> + /*
>> + * This could be a machine with a PCI/SMC conduit,
>> + * which means it doens't have MCFG. Get the machineid from
>> + * the namespace definition instead.
>> + */
>> + if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
>> + "linux,pcie-quirk", &soc)) {
>> + memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
>> + }
>> +
>
> Is there any specific reason for not using the firmware agnostic API to get properties?
>
>
> if (!device_property_read_string(root->device, "linux,pcie-quirk", &soc)) {
> memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
> }
>
>
IIRC it was because the "device" here isn't a struct device, rather a
struct acpi_device. I think this is the normal way in this situation
since we are directly picking up the fwnode rather than finding a
generic node and then backtracking to get the fwnode.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
@ 2021-08-10 14:47 ` Jeremy Linton
0 siblings, 0 replies; 42+ messages in thread
From: Jeremy Linton @ 2021-08-10 14:47 UTC (permalink / raw)
To: Shanker R Donthineni, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi,
Thanks for looking at this!
On 8/10/21 9:31 AM, Shanker R Donthineni wrote:
> Hi Jeremy,
>
> On 8/5/21 4:12 PM, Jeremy Linton wrote:
>> Now that we have a bcm2711 quirk, we need to be able to
>> detect it when the MCFG is missing. Use a namespace
>> property as an alternative to the MCFG OEM.
>>
>> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
>> ---
>> drivers/acpi/pci_mcfg.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index 53cab975f612..7d77fc72c2a4 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
>> ALTRA_ECAM_QUIRK(1, 13),
>> ALTRA_ECAM_QUIRK(1, 14),
>> ALTRA_ECAM_QUIRK(1, 15),
>> +
>> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
>> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
>> };
>>
>> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
>> u16 segment = root->segment;
>> struct resource *bus_range = &root->secondary;
>> struct mcfg_fixup *f;
>> + const char *soc;
>> int i;
>>
>> + /*
>> + * This could be a machine with a PCI/SMC conduit,
>> + * which means it doens't have MCFG. Get the machineid from
>> + * the namespace definition instead.
>> + */
>> + if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
>> + "linux,pcie-quirk", &soc)) {
>> + memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
>> + }
>> +
>
> Is there any specific reason for not using the firmware agnostic API to get properties?
>
>
> if (!device_property_read_string(root->device, "linux,pcie-quirk", &soc)) {
> memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
> }
>
>
IIRC it was because the "device" here isn't a struct device, rather a
struct acpi_device. I think this is the normal way in this situation
since we are directly picking up the fwnode rather than finding a
generic node and then backtracking to get the fwnode.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
2021-08-10 14:47 ` Jeremy Linton
@ 2021-08-10 15:09 ` Shanker R Donthineni
-1 siblings, 0 replies; 42+ messages in thread
From: Shanker R Donthineni @ 2021-08-10 15:09 UTC (permalink / raw)
To: Jeremy Linton, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
On 8/10/21 9:47 AM, Jeremy Linton wrote:
>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>> index 53cab975f612..7d77fc72c2a4 100644
>>> --- a/drivers/acpi/pci_mcfg.c
>>> +++ b/drivers/acpi/pci_mcfg.c
>>> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
>>> ALTRA_ECAM_QUIRK(1, 13),
>>> ALTRA_ECAM_QUIRK(1, 14),
>>> ALTRA_ECAM_QUIRK(1, 15),
>>> +
>>> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
>>> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
>>> };
>>>
>>> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>>> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
>>> u16 segment = root->segment;
>>> struct resource *bus_range = &root->secondary;
>>> struct mcfg_fixup *f;
>>> + const char *soc;
>>> int i;
>>>
>>> + /*
>>> + * This could be a machine with a PCI/SMC conduit,
>>> + * which means it doens't have MCFG. Get the machineid from
>>> + * the namespace definition instead.
>>> + */
>>> + if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
>>> + "linux,pcie-quirk", &soc)) {
>>> + memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
>>> + }
>>> +
>>
>> Is there any specific reason for not using the firmware agnostic API to get properties?
>>
>>
>> if (!device_property_read_string(root->device, "linux,pcie-quirk", &soc)) {
>> memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
>> }
>>
>>
>
> IIRC it was because the "device" here isn't a struct device, rather a
> struct acpi_device. I think this is the normal way in this situation
> since we are directly picking up the fwnode rather than finding a
> generic node and then backtracking to get the fwnode.
Yes, you are right.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 3/3] PCI/ACPI: Add new quirk detection, enable bcm2711
@ 2021-08-10 15:09 ` Shanker R Donthineni
0 siblings, 0 replies; 42+ messages in thread
From: Shanker R Donthineni @ 2021-08-10 15:09 UTC (permalink / raw)
To: Jeremy Linton, linux-pci
Cc: lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh, kw,
f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
On 8/10/21 9:47 AM, Jeremy Linton wrote:
>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>> index 53cab975f612..7d77fc72c2a4 100644
>>> --- a/drivers/acpi/pci_mcfg.c
>>> +++ b/drivers/acpi/pci_mcfg.c
>>> @@ -169,6 +169,9 @@ static struct mcfg_fixup mcfg_quirks[] = {
>>> ALTRA_ECAM_QUIRK(1, 13),
>>> ALTRA_ECAM_QUIRK(1, 14),
>>> ALTRA_ECAM_QUIRK(1, 15),
>>> +
>>> + { "bcm2711", "", 0, 0, MCFG_BUS_ANY, &bcm2711_pcie_ops,
>>> + DEFINE_RES_MEM(0xFD500000, 0xA000) },
>>> };
>>>
>>> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>>> @@ -198,8 +201,19 @@ static void pci_mcfg_apply_quirks(struct acpi_pci_root *root,
>>> u16 segment = root->segment;
>>> struct resource *bus_range = &root->secondary;
>>> struct mcfg_fixup *f;
>>> + const char *soc;
>>> int i;
>>>
>>> + /*
>>> + * This could be a machine with a PCI/SMC conduit,
>>> + * which means it doens't have MCFG. Get the machineid from
>>> + * the namespace definition instead.
>>> + */
>>> + if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
>>> + "linux,pcie-quirk", &soc)) {
>>> + memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
>>> + }
>>> +
>>
>> Is there any specific reason for not using the firmware agnostic API to get properties?
>>
>>
>> if (!device_property_read_string(root->device, "linux,pcie-quirk", &soc)) {
>> memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
>> }
>>
>>
>
> IIRC it was because the "device" here isn't a struct device, rather a
> struct acpi_device. I think this is the normal way in this situation
> since we are directly picking up the fwnode rather than finding a
> generic node and then backtracking to get the fwnode.
Yes, you are right.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 0/3] CM4 ACPI PCIe quirk
2021-08-05 21:11 ` Jeremy Linton
@ 2021-08-06 11:40 ` Stefan Wahren
-1 siblings, 0 replies; 42+ messages in thread
From: Stefan Wahren @ 2021-08-06 11:40 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi Jeremy,
Am 05.08.21 um 23:11 schrieb Jeremy Linton:
> The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
> standard because its PCIe config space isn't ECAM compliant and is
> split into two parts. One part for the root port registers and a
> moveable window which points at a given device's 4K config space.
> Thus it doesn't have a MCFG (and really any MCFG provided would be
> nonsense anyway). As linux doesn't support the PCIe SMCCC standard
> we key off a linux specific host bridge _DSD to add custom ECAM
> ops and cfgres. The cfg op selects between those two regions, as
> well as disallowing problematic accesses, particularly if the link
> is down because there isn't an attached device.
i just want to inform you, that i recently submitted the inital patch
series for DT support regarding CM4 [1].
I left out anything related to PCIe (including downstream changes to
pcie-brcmstb).
Best regards
Stefan
[1] - https://marc.info/?l=linux-arm-kernel&m=162782110325813&w=2
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 0/3] CM4 ACPI PCIe quirk
@ 2021-08-06 11:40 ` Stefan Wahren
0 siblings, 0 replies; 42+ messages in thread
From: Stefan Wahren @ 2021-08-06 11:40 UTC (permalink / raw)
To: Jeremy Linton
Cc: linux-pci, lorenzo.pieralisi, nsaenz, bhelgaas, rjw, lenb, robh,
kw, f.fainelli, bcm-kernel-feedback-list, linux-acpi,
linux-arm-kernel, linux-rpi-kernel, linux-kernel
Hi Jeremy,
Am 05.08.21 um 23:11 schrieb Jeremy Linton:
> The PFTF CM4 is an ACPI platform that is following the PCIe SMCCC
> standard because its PCIe config space isn't ECAM compliant and is
> split into two parts. One part for the root port registers and a
> moveable window which points at a given device's 4K config space.
> Thus it doesn't have a MCFG (and really any MCFG provided would be
> nonsense anyway). As linux doesn't support the PCIe SMCCC standard
> we key off a linux specific host bridge _DSD to add custom ECAM
> ops and cfgres. The cfg op selects between those two regions, as
> well as disallowing problematic accesses, particularly if the link
> is down because there isn't an attached device.
i just want to inform you, that i recently submitted the inital patch
series for DT support regarding CM4 [1].
I left out anything related to PCIe (including downstream changes to
pcie-brcmstb).
Best regards
Stefan
[1] - https://marc.info/?l=linux-arm-kernel&m=162782110325813&w=2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread