From: Jeremy Linton <hidden> Date: 2021-08-19 21:57:19
The PFTF CM4 is an ACPI platform that is following the Arm PCIe SMC
(DEN0115) standard because its PCIe config space isn't ECAM compliant
since it is split into two parts. One part describes the root port
registers, and another contains a moveable window pointing at a given
device's 4K config space. Thus it doesn't have an MCFG table. As
Linux doesn't support the PCI/SMC, a host bridge specific _DSD is
added and associated with custom ECAM ops and cfgres. The custom cfg
op selects between those two regions, as well as disallowing
problematic accesses.
V1->V2:
Only move register definitions to new .h file, add
include guards.
Change quirk namespace identifier.
Update Maintainers file.
A number of whitespace, grammar, etc fixes.
Jeremy Linton (4):
PCI: brcmstb: Break register definitions into separate header
PCI: brcmstb: Add ACPI config space quirk
PCI/ACPI: Add Broadcom bcm2711 MCFG quirk
MAINTAINERS: Widen brcmstb PCIe file scope
MAINTAINERS | 2 +-
drivers/acpi/pci_mcfg.c | 13 ++
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-brcmstb-acpi.c | 74 ++++++++++
drivers/pci/controller/pcie-brcmstb.c | 150 +-------------------
drivers/pci/controller/pcie-brcmstb.h | 155 +++++++++++++++++++++
include/linux/pci-ecam.h | 1 +
7 files changed, 247 insertions(+), 149 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
From: Jeremy Linton <hidden> Date: 2021-08-19 21:57:27
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 <redacted>
---
drivers/pci/controller/pcie-brcmstb.c | 150 +------------------------
drivers/pci/controller/pcie-brcmstb.h | 155 ++++++++++++++++++++++++++
2 files changed, 157 insertions(+), 148 deletions(-)
create mode 100644 drivers/pci/controller/pcie-brcmstb.h
From: Jeremy Linton <hidden> Date: 2021-08-19 21:57:30
The PFTF CM4 is an ACPI platform that isn't ECAM compliant. Its config
space is in two parts. One part is for the root port registers and a
second moveable window pointing at a device's 4K config space. Thus it
doesn't have an MCFG, and any MCFG provided would be nonsense
anyway. Instead, a Linux specific host bridge _DSD selects a custom
ECAM ops and cfgres. The cfg op picks between those two regions while
disallowing problematic accesses.
Signed-off-by: Jeremy Linton <redacted>
---
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-brcmstb-acpi.c | 74 ++++++++++++++++++++++
include/linux/pci-ecam.h | 1 +
3 files changed, 76 insertions(+)
create mode 100644 drivers/pci/controller/pcie-brcmstb-acpi.c
@@ -0,0 +1,74 @@+// SPDX-License-Identifier: GPL-2.0++/*+*ACPIquirksforBrcm2711PCIehostcontroller+*AsusedontheRaspberryPiComputeModule4+*+*Copyright(C)2021ArmLtd.+*/++#include<linux/io.h>+#include<linux/pci.h>+#include<linux/pci-ecam.h>+#include"../pci.h"+#include"pcie-brcmstb.h"++staticintbrcm_acpi_init(structpci_config_window*cfg)+{+/*+*Thisplatformdoesn'ttechnicallyhaveanythingthatcouldbecalled+*ECAM.Itsconfigregionhasrootportspecificregistersbetween+*standardPCIedefinedconfigregisters.Thustheregionsetupbythe+*genericECAMcodeneedstobeadjusted.TheHWcanaccessbus0-ff+*butthefootprintisn'tanicepowerof2(40k).Forpurposesof+*mappingtheconfigregionwearejustgoingtosquashthestandard+*andnonstandardregisterstogetherratherthanmappingthemseparately.+*/+iounmap(cfg->win);+cfg->win=pci_remap_cfgspace(cfg->res.start,resource_size(&cfg->res));+if(!cfg->win)+gotoerr_exit;++/* MSI is nonstandard as well */+pci_no_msi();++return0;+err_exit:+dev_err(cfg->parent,"PCI: Failed to remap config\n");+return-ENOMEM;+}++staticvoid__iomem*brcm_pcie_map_conf2(structpci_bus*bus,+unsignedintdevfn,intwhere)+{+structpci_config_window*cfg=bus->sysdata;+void__iomem*base=cfg->win;+intidx;+u32up;++/* Accesses to the RC go right to the RC registers if slot==0 */+if(pci_is_root_bus(bus))+returnPCI_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))+returnNULL;++if(!(up&PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK))+returnNULL;++/* For devices, write to the config space index register */+idx=PCIE_ECAM_OFFSET(bus->number,devfn,0);+writel(idx,base+PCIE_EXT_CFG_INDEX);+returnbase+PCIE_EXT_CFG_DATA+where;+}++conststructpci_ecam_opsbcm2711_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,+}+};
From: Jeremy Linton <hidden> Date: 2021-08-19 21:57:31
The brcmstb PCI hardware is now split across
multiple files. Include them in the maintainers
block.
Signed-off-by: Jeremy Linton <redacted>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Jeremy Linton <hidden> Date: 2021-08-19 21:57:33
Now that there is a bcm2711 quirk, it needs to be enabled when the
MCFG is missing. Use an ACPI namespace _DSD property
"linux-ecam-quirk-id" as an alternative to the MCFG OEM.
Signed-off-by: Jeremy Linton <redacted>
---
drivers/acpi/pci_mcfg.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
On Thu, Aug 19, 2021 at 04:56:53PM -0500, Jeremy Linton wrote:
The PFTF CM4 is an ACPI platform that isn't ECAM compliant. Its config
space is in two parts. One part is for the root port registers and a
second moveable window pointing at a device's 4K config space. Thus it
doesn't have an MCFG, and any MCFG provided would be nonsense
anyway. Instead, a Linux specific host bridge _DSD selects a custom
ECAM ops and cfgres. The cfg op picks between those two regions while
disallowing problematic accesses.
This doesn't actually say what this patch *does*.
Can you expand "PFTF CM4" somehow? Google (and the comment below, I
guess) suggests it's something to do with Raspberry Pi 4, but it would
be nice if the commit log made sense without Googling or reading the
patch.
@@ -0,0 +1,74 @@+// SPDX-License-Identifier: GPL-2.0++/*+*ACPIquirksforBrcm2711PCIehostcontroller+*AsusedontheRaspberryPiComputeModule4+*+*Copyright(C)2021ArmLtd.+*/++#include<linux/io.h>+#include<linux/pci.h>+#include<linux/pci-ecam.h>+#include"../pci.h"+#include"pcie-brcmstb.h"++staticintbrcm_acpi_init(structpci_config_window*cfg)+{+/*+*Thisplatformdoesn'ttechnicallyhaveanythingthatcouldbecalled+*ECAM.Itsconfigregionhasrootportspecificregistersbetween+*standardPCIedefinedconfigregisters.Thustheregionsetupbythe+*genericECAMcodeneedstobeadjusted.TheHWcanaccessbus0-ff+*butthefootprintisn'tanicepowerof2(40k).Forpurposesof+*mappingtheconfigregionwearejustgoingtosquashthestandard+*andnonstandardregisterstogetherratherthanmappingthemseparately.+*/+iounmap(cfg->win);+cfg->win=pci_remap_cfgspace(cfg->res.start,resource_size(&cfg->res));+if(!cfg->win)+gotoerr_exit;++/* MSI is nonstandard as well */+pci_no_msi();++return0;+err_exit:+dev_err(cfg->parent,"PCI: Failed to remap config\n");+return-ENOMEM;+}++staticvoid__iomem*brcm_pcie_map_conf2(structpci_bus*bus,+unsignedintdevfn,intwhere)+{+structpci_config_window*cfg=bus->sysdata;+void__iomem*base=cfg->win;+intidx;+u32up;++/* Accesses to the RC go right to the RC registers if slot==0 */+if(pci_is_root_bus(bus))+returnPCI_SLOT(devfn)?NULL:base+where;++/* Assure link up before sending request */
Obviously this is horribly racy, since the link may go down after you
check but before you send the request. Maybe the hardware leaves you
no choice. I'd feel a little better about it if the comment
acknowledged that (if it's so) and outlined the consequence of losing
the race (panic, recoverable error, etc).
quoted hunk
+ 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,
+ }
+};
There are already two entries that mention
drivers/pci/controller/pcie-brcmstb.c, and a patch headed for v5.14
adds a third. Do you want to update them all?
On Thu, Aug 19, 2021 at 04:56:51PM -0500, Jeremy Linton wrote:
The PFTF CM4 is an ACPI platform that is following the Arm PCIe SMC
(DEN0115) standard because its PCIe config space isn't ECAM compliant
since it is split into two parts. One part describes the root port
registers, and another contains a moveable window pointing at a given
device's 4K config space. Thus it doesn't have an MCFG table. As
Linux doesn't support the PCI/SMC, a host bridge specific _DSD is
added and associated with custom ECAM ops and cfgres. The custom cfg
op selects between those two regions, as well as disallowing
problematic accesses.
V1->V2:
Only move register definitions to new .h file, add
include guards.
Change quirk namespace identifier.
Update Maintainers file.
A number of whitespace, grammar, etc fixes.
Jeremy Linton (4):
PCI: brcmstb: Break register definitions into separate header
PCI: brcmstb: Add ACPI config space quirk
PCI/ACPI: Add Broadcom bcm2711 MCFG quirk
MAINTAINERS: Widen brcmstb PCIe file scope
MAINTAINERS | 2 +-
drivers/acpi/pci_mcfg.c | 13 ++
drivers/pci/controller/Makefile | 1 +
drivers/pci/controller/pcie-brcmstb-acpi.c | 74 ++++++++++
drivers/pci/controller/pcie-brcmstb.c | 150 +-------------------
drivers/pci/controller/pcie-brcmstb.h | 155 +++++++++++++++++++++
include/linux/pci-ecam.h | 1 +
7 files changed, 247 insertions(+), 149 deletions(-)
create mode 100644 drivers/pci/controller/pcie-brcmstb-acpi.c
create mode 100644 drivers/pci/controller/pcie-brcmstb.h
I'm fine with all of these, given the minor comments I made.
Lorenzo, if you want to pick this up after Jeremy updates it:
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Jeremy Linton <hidden> Date: 2021-08-20 20:31:50
Hi,
Thanks for looking at this.
On 8/20/21 2:06 PM, Bjorn Helgaas wrote:
On Thu, Aug 19, 2021 at 04:56:53PM -0500, Jeremy Linton wrote:
quoted
The PFTF CM4 is an ACPI platform that isn't ECAM compliant. Its config
space is in two parts. One part is for the root port registers and a
second moveable window pointing at a device's 4K config space. Thus it
doesn't have an MCFG, and any MCFG provided would be nonsense
anyway. Instead, a Linux specific host bridge _DSD selects a custom
ECAM ops and cfgres. The cfg op picks between those two regions while
disallowing problematic accesses.
This doesn't actually say what this patch *does*.
Ok.
Can you expand "PFTF CM4" somehow? Google (and the comment below, I
guess) suggests it's something to do with Raspberry Pi 4, but it would
be nice if the commit log made sense without Googling or reading the
patch.
Yes, sure, as you deduced PFTF is a community project which aims to
create a systemready UEFI/ACPI platform out of rpi4/rpi400/cm4 systems.
Its actually fairly far along in that goal, and is capable of booting a
wide range of arm64 based OS's & Hypervisors. Its acting like a proof of
concept that putting effort into some basic platform abstractions
reduces the effort to boot off the shelf OSs. It does this by moving
much of the platform specific code into firmware.
@@ -0,0 +1,74 @@+// SPDX-License-Identifier: GPL-2.0++/*+*ACPIquirksforBrcm2711PCIehostcontroller+*AsusedontheRaspberryPiComputeModule4+*+*Copyright(C)2021ArmLtd.+*/++#include<linux/io.h>+#include<linux/pci.h>+#include<linux/pci-ecam.h>+#include"../pci.h"+#include"pcie-brcmstb.h"++staticintbrcm_acpi_init(structpci_config_window*cfg)+{+/*+*Thisplatformdoesn'ttechnicallyhaveanythingthatcouldbecalled+*ECAM.Itsconfigregionhasrootportspecificregistersbetween+*standardPCIedefinedconfigregisters.Thustheregionsetupbythe+*genericECAMcodeneedstobeadjusted.TheHWcanaccessbus0-ff+*butthefootprintisn'tanicepowerof2(40k).Forpurposesof+*mappingtheconfigregionwearejustgoingtosquashthestandard+*andnonstandardregisterstogetherratherthanmappingthemseparately.+*/+iounmap(cfg->win);+cfg->win=pci_remap_cfgspace(cfg->res.start,resource_size(&cfg->res));+if(!cfg->win)+gotoerr_exit;++/* MSI is nonstandard as well */+pci_no_msi();++return0;+err_exit:+dev_err(cfg->parent,"PCI: Failed to remap config\n");+return-ENOMEM;+}++staticvoid__iomem*brcm_pcie_map_conf2(structpci_bus*bus,+unsignedintdevfn,intwhere)+{+structpci_config_window*cfg=bus->sysdata;+void__iomem*base=cfg->win;+intidx;+u32up;++/* Accesses to the RC go right to the RC registers if slot==0 */+if(pci_is_root_bus(bus))+returnPCI_SLOT(devfn)?NULL:base+where;++/* Assure link up before sending request */
Obviously this is horribly racy, since the link may go down after you
check but before you send the request. Maybe the hardware leaves you
no choice. I'd feel a little better about it if the comment
acknowledged that (if it's so) and outlined the consequence of losing
the race (panic, recoverable error, etc).
Sure, that gets at what appears to be the fundamental problem with this
bridge, mainly that the HW doesn't appear to be able to handle missing
config TLP completions in a way that can be recovered.
So, if one loses the race here (and the window is much larger in the DT
config accessor) the machine takes what is basically an unrecoverable
exception. I will update the comment to that effect.
quoted
+ 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,
+ }
+};
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 <redacted>
The PFTF CM4 is an ACPI platform that isn't ECAM compliant. Its config
space is in two parts. One part is for the root port registers and a
second moveable window pointing at a device's 4K config space. Thus it
doesn't have an MCFG, and any MCFG provided would be nonsense
anyway. Instead, a Linux specific host bridge _DSD selects a custom
ECAM ops and cfgres. The cfg op picks between those two regions while
disallowing problematic accesses.
Signed-off-by: Jeremy Linton <redacted>
Once you address Bjorn's feedback, feel free to add:
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
I do wonder if squashing patches 2 and 3 would make more sense,
otherwise we have a bcm2711_pcie_ops that is unused in patch 2.
--
Florian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Now that there is a bcm2711 quirk, it needs to be enabled when the
MCFG is missing. Use an ACPI namespace _DSD property
"linux-ecam-quirk-id" as an alternative to the MCFG OEM.
Signed-off-by: Jeremy Linton <redacted>
---
drivers/acpi/pci_mcfg.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
There are already two entries that mention
drivers/pci/controller/pcie-brcmstb.c, and a patch headed for v5.14
adds a third. Do you want to update them all?
From: Jeremy Linton <hidden> Date: 2021-08-24 21:39:25
Hi,
On 8/22/21 3:53 AM, Florian Fainelli wrote:
On 8/19/2021 11:56 PM, Jeremy Linton wrote:
quoted
Now that there is a bcm2711 quirk, it needs to be enabled when the
MCFG is missing. Use an ACPI namespace _DSD property
"linux-ecam-quirk-id" as an alternative to the MCFG OEM.
Signed-off-by: Jeremy Linton <redacted>
---
drivers/acpi/pci_mcfg.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
acpi_pci_root *root,
u16 segment = root->segment;
struct resource *bus_range = &root->secondary;
struct mcfg_fixup *f;
+ const char *soc;
int i;
+ /*
+ * This may be a machine with a PCI/SMC conduit, which means it
doesn't
+ * have an MCFG. Use an ACPI namespace definition instead.
+ */
+ if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
+ "linux-ecam-quirk-id", &soc)) {
+ memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
Being super paranoid here, can we use one of the "safe" string copy
routines here just in case?
Hmm, I went around with this a bit when I first wrote it, because the
OEM fields in the ACPI tables are fixed len and don't have null
termination. Maybe the right thing to do here is verify the string size
is at least as long as the OEM_ID_SIZE and then continue to use the memcpy.
quoted
+ }
+
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)
Now that there is a bcm2711 quirk, it needs to be enabled when the
MCFG is missing. Use an ACPI namespace _DSD property
"linux-ecam-quirk-id" as an alternative to the MCFG OEM.
Signed-off-by: Jeremy Linton <redacted>
---
drivers/acpi/pci_mcfg.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
acpi_pci_root *root,
u16 segment = root->segment;
struct resource *bus_range = &root->secondary;
struct mcfg_fixup *f;
+ const char *soc;
int i;
+ /*
+ * This may be a machine with a PCI/SMC conduit, which means it
doesn't
+ * have an MCFG. Use an ACPI namespace definition instead.
+ */
+ if (!fwnode_property_read_string(acpi_fwnode_handle(root->device),
+ "linux-ecam-quirk-id", &soc)) {
+ memcpy(mcfg_oem_id, soc, ACPI_OEM_ID_SIZE);
Being super paranoid here, can we use one of the "safe" string copy
routines here just in case?
Hmm, I went around with this a bit when I first wrote it, because the
OEM fields in the ACPI tables are fixed len and don't have null
termination. Maybe the right thing to do here is verify the string size
is at least as long as the OEM_ID_SIZE and then continue to use the memcpy.
On Thursday 19 August 2021 16:56:53 Jeremy Linton wrote:
The PFTF CM4 is an ACPI platform that isn't ECAM compliant. Its config
space is in two parts. One part is for the root port registers and a
second moveable window pointing at a device's 4K config space. Thus it
doesn't have an MCFG, and any MCFG provided would be nonsense
anyway. Instead, a Linux specific host bridge _DSD selects a custom
ECAM ops and cfgres. The cfg op picks between those two regions while
disallowing problematic accesses.
I'm not sure if Lorenzo would like this patch series...
In past there was a long discussion about ECAM compliance, MCFG quirks
and usage of ACPI (on other platform), see long thread:
https://lore.kernel.org/linux-pci/20200207183427.GA40158@google.com/
And I think it is not a good idea to extend MCFG quirks table as
according to discussion it was just temporary plaster and if platform is
not ACPI / ECAM compliant then it should use DT booting...
Lorenzo, could you put any comment on this?
@@ -0,0 +1,74 @@+// SPDX-License-Identifier: GPL-2.0++/*+*ACPIquirksforBrcm2711PCIehostcontroller+*AsusedontheRaspberryPiComputeModule4+*+*Copyright(C)2021ArmLtd.+*/++#include<linux/io.h>+#include<linux/pci.h>+#include<linux/pci-ecam.h>+#include"../pci.h"+#include"pcie-brcmstb.h"++staticintbrcm_acpi_init(structpci_config_window*cfg)+{+/*+*Thisplatformdoesn'ttechnicallyhaveanythingthatcouldbecalled+*ECAM.Itsconfigregionhasrootportspecificregistersbetween+*standardPCIedefinedconfigregisters.Thustheregionsetupbythe+*genericECAMcodeneedstobeadjusted.TheHWcanaccessbus0-ff+*butthefootprintisn'tanicepowerof2(40k).Forpurposesof+*mappingtheconfigregionwearejustgoingtosquashthestandard+*andnonstandardregisterstogetherratherthanmappingthemseparately.+*/+iounmap(cfg->win);+cfg->win=pci_remap_cfgspace(cfg->res.start,resource_size(&cfg->res));+if(!cfg->win)+gotoerr_exit;++/* MSI is nonstandard as well */+pci_no_msi();++return0;+err_exit:+dev_err(cfg->parent,"PCI: Failed to remap config\n");+return-ENOMEM;+}++staticvoid__iomem*brcm_pcie_map_conf2(structpci_bus*bus,+unsignedintdevfn,intwhere)+{+structpci_config_window*cfg=bus->sysdata;+void__iomem*base=cfg->win;+intidx;+u32up;++/* Accesses to the RC go right to the RC registers if slot==0 */+if(pci_is_root_bus(bus))+returnPCI_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))+returnNULL;++if(!(up&PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK))+returnNULL;++/* For devices, write to the config space index register */+idx=PCIE_ECAM_OFFSET(bus->number,devfn,0);+writel(idx,base+PCIE_EXT_CFG_INDEX);+returnbase+PCIE_EXT_CFG_DATA+where;+}++conststructpci_ecam_opsbcm2711_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,+}+};
From: Jeremy Linton <hidden> Date: 2021-08-30 16:11:01
Hi,
On 8/29/21 6:13 AM, Pali Rohár wrote:
On Thursday 19 August 2021 16:56:53 Jeremy Linton wrote:
quoted
The PFTF CM4 is an ACPI platform that isn't ECAM compliant. Its config
space is in two parts. One part is for the root port registers and a
second moveable window pointing at a device's 4K config space. Thus it
doesn't have an MCFG, and any MCFG provided would be nonsense
anyway. Instead, a Linux specific host bridge _DSD selects a custom
ECAM ops and cfgres. The cfg op picks between those two regions while
disallowing problematic accesses.
I'm not sure if Lorenzo would like this patch series...
That was sorta true since the arm64/ACPI/PCI patches landed. The
underlying reason is the desire for arm platforms to require less
one-off kernel patching in order to "just work". But, its become
apparent that there continue to be problems with PCIe IP and Arm
interconnect integration. So, a firmware interface was standardized
which solves most of the nonstandard ECAM issues. At that point it was
decided though that the kernel maintainers would prefer to have the
quirks visible to the kernel rather than hidden in the firmware, and
that they would be more open to merging these quirks. The Tegra patch
you listed above has been merged.
More info about this: https://lkml.org/lkml/2021/3/25/777
Thanks,
In past there was a long discussion about ECAM compliance, MCFG quirks
and usage of ACPI (on other platform), see long thread:
https://lore.kernel.org/linux-pci/20200207183427.GA40158@google.com/
And I think it is not a good idea to extend MCFG quirks table as
according to discussion it was just temporary plaster and if platform is
not ACPI / ECAM compliant then it should use DT booting...
Lorenzo, could you put any comment on this?
@@ -0,0 +1,74 @@+// SPDX-License-Identifier: GPL-2.0++/*+*ACPIquirksforBrcm2711PCIehostcontroller+*AsusedontheRaspberryPiComputeModule4+*+*Copyright(C)2021ArmLtd.+*/++#include<linux/io.h>+#include<linux/pci.h>+#include<linux/pci-ecam.h>+#include"../pci.h"+#include"pcie-brcmstb.h"++staticintbrcm_acpi_init(structpci_config_window*cfg)+{+/*+*Thisplatformdoesn'ttechnicallyhaveanythingthatcouldbecalled+*ECAM.Itsconfigregionhasrootportspecificregistersbetween+*standardPCIedefinedconfigregisters.Thustheregionsetupbythe+*genericECAMcodeneedstobeadjusted.TheHWcanaccessbus0-ff+*butthefootprintisn'tanicepowerof2(40k).Forpurposesof+*mappingtheconfigregionwearejustgoingtosquashthestandard+*andnonstandardregisterstogetherratherthanmappingthemseparately.+*/+iounmap(cfg->win);+cfg->win=pci_remap_cfgspace(cfg->res.start,resource_size(&cfg->res));+if(!cfg->win)+gotoerr_exit;++/* MSI is nonstandard as well */+pci_no_msi();++return0;+err_exit:+dev_err(cfg->parent,"PCI: Failed to remap config\n");+return-ENOMEM;+}++staticvoid__iomem*brcm_pcie_map_conf2(structpci_bus*bus,+unsignedintdevfn,intwhere)+{+structpci_config_window*cfg=bus->sysdata;+void__iomem*base=cfg->win;+intidx;+u32up;++/* Accesses to the RC go right to the RC registers if slot==0 */+if(pci_is_root_bus(bus))+returnPCI_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))+returnNULL;++if(!(up&PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK))+returnNULL;++/* For devices, write to the config space index register */+idx=PCIE_ECAM_OFFSET(bus->number,devfn,0);+writel(idx,base+PCIE_EXT_CFG_INDEX);+returnbase+PCIE_EXT_CFG_DATA+where;+}++conststructpci_ecam_opsbcm2711_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,+}+};
On Monday 30 August 2021 11:10:55 Jeremy Linton wrote:
Hi,
On 8/29/21 6:13 AM, Pali Rohár wrote:
quoted
On Thursday 19 August 2021 16:56:53 Jeremy Linton wrote:
quoted
The PFTF CM4 is an ACPI platform that isn't ECAM compliant. Its config
space is in two parts. One part is for the root port registers and a
second moveable window pointing at a device's 4K config space. Thus it
doesn't have an MCFG, and any MCFG provided would be nonsense
anyway. Instead, a Linux specific host bridge _DSD selects a custom
ECAM ops and cfgres. The cfg op picks between those two regions while
disallowing problematic accesses.
I'm not sure if Lorenzo would like this patch series...
That was sorta true since the arm64/ACPI/PCI patches landed. The underlying
reason is the desire for arm platforms to require less one-off kernel
patching in order to "just work". But, its become apparent that there
continue to be problems with PCIe IP and Arm interconnect integration. So, a
firmware interface was standardized which solves most of the nonstandard
ECAM issues. At that point it was decided though that the kernel maintainers
would prefer to have the quirks visible to the kernel rather than hidden in
the firmware, and that they would be more open to merging these quirks. The
Tegra patch you listed above has been merged.
More info about this: https://lkml.org/lkml/2021/3/25/777
Hi and thanks for pointer!
I did not know about that new discussion and a new solution.
Anyway, according to that discussion, adding a new MCFG quirk into
kernel requires adding some errata entry for documenting buggy HW. And
seems that this documentation update is not included in this patch
series...
Thanks,
quoted
In past there was a long discussion about ECAM compliance, MCFG quirks
and usage of ACPI (on other platform), see long thread:
https://lore.kernel.org/linux-pci/20200207183427.GA40158@google.com/
And I think it is not a good idea to extend MCFG quirks table as
according to discussion it was just temporary plaster and if platform is
not ACPI / ECAM compliant then it should use DT booting...
Lorenzo, could you put any comment on this?
@@ -0,0 +1,74 @@+// SPDX-License-Identifier: GPL-2.0++/*+*ACPIquirksforBrcm2711PCIehostcontroller+*AsusedontheRaspberryPiComputeModule4+*+*Copyright(C)2021ArmLtd.+*/++#include<linux/io.h>+#include<linux/pci.h>+#include<linux/pci-ecam.h>+#include"../pci.h"+#include"pcie-brcmstb.h"++staticintbrcm_acpi_init(structpci_config_window*cfg)+{+/*+*Thisplatformdoesn'ttechnicallyhaveanythingthatcouldbecalled+*ECAM.Itsconfigregionhasrootportspecificregistersbetween+*standardPCIedefinedconfigregisters.Thustheregionsetupbythe+*genericECAMcodeneedstobeadjusted.TheHWcanaccessbus0-ff+*butthefootprintisn'tanicepowerof2(40k).Forpurposesof+*mappingtheconfigregionwearejustgoingtosquashthestandard+*andnonstandardregisterstogetherratherthanmappingthemseparately.+*/+iounmap(cfg->win);+cfg->win=pci_remap_cfgspace(cfg->res.start,resource_size(&cfg->res));+if(!cfg->win)+gotoerr_exit;++/* MSI is nonstandard as well */+pci_no_msi();++return0;+err_exit:+dev_err(cfg->parent,"PCI: Failed to remap config\n");+return-ENOMEM;+}++staticvoid__iomem*brcm_pcie_map_conf2(structpci_bus*bus,+unsignedintdevfn,intwhere)+{+structpci_config_window*cfg=bus->sysdata;+void__iomem*base=cfg->win;+intidx;+u32up;++/* Accesses to the RC go right to the RC registers if slot==0 */+if(pci_is_root_bus(bus))+returnPCI_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))+returnNULL;++if(!(up&PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK))+returnNULL;++/* For devices, write to the config space index register */+idx=PCIE_ECAM_OFFSET(bus->number,devfn,0);+writel(idx,base+PCIE_EXT_CFG_INDEX);+returnbase+PCIE_EXT_CFG_DATA+where;+}++conststructpci_ecam_opsbcm2711_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,+}+};