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
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 <[email protected]>
---
drivers/acpi/pci_mcfg.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index 53cab975f612..4b991ee5c66c 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,18 @@ 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 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);
+ }
+
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
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 <[email protected]>
On 8/19/2021 11:56 PM, Jeremy Linton wrote:
> 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 <[email protected]>
> ---
> drivers/acpi/pci_mcfg.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index 53cab975f612..4b991ee5c66c 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,18 @@ 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 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?
> + }
> +
> 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)
>
--
Florian
Hi,
On 8/22/21 3:53 AM, Florian Fainelli wrote:
>
>
> On 8/19/2021 11:56 PM, Jeremy Linton wrote:
>> 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 <[email protected]>
>> ---
>> drivers/acpi/pci_mcfg.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index 53cab975f612..4b991ee5c66c 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,18 @@ 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 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.
>
>> + }
>> +
>> 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)
>>
>
On 8/24/2021 11:39 PM, Jeremy Linton wrote:
> Hi,
>
> On 8/22/21 3:53 AM, Florian Fainelli wrote:
>>
>>
>> On 8/19/2021 11:56 PM, Jeremy Linton wrote:
>>> 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 <[email protected]>
>>> ---
>>> drivers/acpi/pci_mcfg.c | 13 +++++++++++++
>>> 1 file changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>> index 53cab975f612..4b991ee5c66c 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,18 @@ 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 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.
Sure, sounds entirely reasonable to me.
--
Florian