| Message ID | 20200904170829.431900-1-jeremy.linton@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI/ACPI: Suppress missing MCFG message | expand |
Hi Jeremy, On 2020/9/5 1:08, Jeremy Linton wrote: > MCFG is an optional ACPI table. Given there are machines > without PCI(e) (or it is hidden) we have been receiving > queries/complaints about what this message means given > its being presented as an error. > > Lets only print an error if something is wrong with the > the given table/etc. The ACPI table list printed at boot > will continue to provide a way to detect when the table > is missing. > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> > --- > drivers/acpi/pci_mcfg.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c > index 54b36b7ad47d..0bc8c012f157 100644 > --- a/drivers/acpi/pci_mcfg.c > +++ b/drivers/acpi/pci_mcfg.c > @@ -279,6 +279,6 @@ static __init int pci_mcfg_parse(struct acpi_table_header *header) > void __init pci_mmcfg_late_init(void) > { > int err = acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse); acpi_table_parse() will return errno -ENODEV and -EINVAL, and only returns -EINVAL if the table id or the handler is NULL, which is impossible here. > - if (err) > + if (err && err != -ENODEV) So the error message below will never be printed, I would prefer update the pr_err() to pr_dbg() for optional ACPI table. > pr_err("Failed to parse MCFG (%d)\n", err); > } Thanks Hanjun
Hi, On 9/6/20 10:30 PM, Hanjun Guo wrote: > Hi Jeremy, > > On 2020/9/5 1:08, Jeremy Linton wrote: >> MCFG is an optional ACPI table. Given there are machines >> without PCI(e) (or it is hidden) we have been receiving >> queries/complaints about what this message means given >> its being presented as an error. >> >> Lets only print an error if something is wrong with the >> the given table/etc. The ACPI table list printed at boot >> will continue to provide a way to detect when the table >> is missing. >> >> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> >> --- >> drivers/acpi/pci_mcfg.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c >> index 54b36b7ad47d..0bc8c012f157 100644 >> --- a/drivers/acpi/pci_mcfg.c >> +++ b/drivers/acpi/pci_mcfg.c >> @@ -279,6 +279,6 @@ static __init int pci_mcfg_parse(struct >> acpi_table_header *header) >> void __init pci_mmcfg_late_init(void) >> { >> int err = acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse); > > acpi_table_parse() will return errno -ENODEV and -EINVAL, and only > returns -EINVAL if the table id or the handler is NULL, which is > impossible here. Right because it doesn't actually return the handler failures. > >> - if (err) >> + if (err && err != -ENODEV) > > So the error message below will never be printed, I would > prefer update the pr_err() to pr_dbg() for optional ACPI > table. Sure, that is a fine plan too. > >> pr_err("Failed to parse MCFG (%d)\n", err); >> } > > Thanks > Hanjun >
diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c index 54b36b7ad47d..0bc8c012f157 100644 --- a/drivers/acpi/pci_mcfg.c +++ b/drivers/acpi/pci_mcfg.c @@ -279,6 +279,6 @@ static __init int pci_mcfg_parse(struct acpi_table_header *header) void __init pci_mmcfg_late_init(void) { int err = acpi_table_parse(ACPI_SIG_MCFG, pci_mcfg_parse); - if (err) + if (err && err != -ENODEV) pr_err("Failed to parse MCFG (%d)\n", err); }
MCFG is an optional ACPI table. Given there are machines without PCI(e) (or it is hidden) we have been receiving queries/complaints about what this message means given its being presented as an error. Lets only print an error if something is wrong with the the given table/etc. The ACPI table list printed at boot will continue to provide a way to detect when the table is missing. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> --- drivers/acpi/pci_mcfg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)