diff mbox series

[v2,3/4] PCI/ACPI: Add Broadcom bcm2711 MCFG quirk

Message ID 20210819215655.84866-4-jeremy.linton@arm.com
State Superseded
Headers show
Series CM4 ACPI PCIe quirk | expand

Commit Message

Jeremy Linton Aug. 19, 2021, 9:56 p.m. UTC 
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 <jeremy.linton@arm.com>

---
 drivers/acpi/pci_mcfg.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

-- 
2.31.1

Comments

Florian Fainelli Aug. 22, 2021, 8:53 a.m. UTC | #1 
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 <jeremy.linton@arm.com>

> ---

>   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
Jeremy Linton Aug. 24, 2021, 9:39 p.m. UTC | #2 
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 <jeremy.linton@arm.com>

>> ---

>>   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)

>>

>
Florian Fainelli Aug. 25, 2021, 4:23 p.m. UTC | #3 
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 <jeremy.linton@arm.com>

>>> ---

>>>   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
diff mbox series

Patch

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)