Re: [PATCH] ACPI: bus: Avoid misleading _OSC missing-feature errors
Jonathan Cameron <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.acpi.devel |
|---|---|
| Message-ID | <20260722001452.019b3e32@jic23-huawei> |
On Tue, 21 Jul 2026 21:29:14 +0200 Saverio Miroddi <[email protected]> wrote: > The ACPI specification defines OSC_CAPABILITIES_MASK_ERROR to mean that > firmware cleared capability bits requested by the operating system. Some > firmware sets this status during the control request while returning every > requested capability unchanged. On the affected system, the requested and > returned masks were both 0x006a7eff. Just to check my understanding. When a system does this there is no error? Is it happy with the capabilities requested? So the return value is simply a firmware bug? (just one we didn't notice before). > > Commit e5322888e6bf ("ACPI: bus: Rework the handling of \_SB._OSC > platform features") made all control-response errors produce error-level > messages. Consequently, the inconsistent response above now claims that > features may be missing even though none were removed. > > Track whether the response actually clears a requested bit and omit the > error-level messages only if OSC_CAPABILITIES_MASK_ERROR is the sole > error and no capability was cleared. Keep the dynamic-debug diagnostic and > leave the negotiated capability mask and all other error cases unchanged. > > Fixes: e5322888e6bf ("ACPI: bus: Rework the handling of \_SB._OSC platform features") > Signed-off-by: Saverio Miroddi <[email protected]> > --- > drivers/acpi/bus.c | 23 ++++++++++++++++------- > 1 file changed, 16 insertions(+), 7 deletions(-) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index a30a904f6535..c13128cbc2f6 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -335,7 +335,8 @@ static int acpi_osc_handshake(acpi_handle handle, const char *uuid_str, > .length = bufsize * sizeof(u32), > }; > struct acpi_buffer output; > - u32 *retbuf, test; > + u32 *retbuf, test, errors; > + bool capabilities_masked = false; > guid_t guid; > int ret, i; > > @@ -395,16 +396,24 @@ static int acpi_osc_handshake(acpi_handle handle, const char *uuid_str, > * Clear the feature bits in capbuf[] that have not been acknowledged. > * After that, capbuf[] contains the resultant feature mask. > */ > - for (i = OSC_QUERY_DWORD + 1; i < bufsize; i++) > + for (i = OSC_QUERY_DWORD + 1; i < bufsize; i++) { > + if (capbuf[i] & ~retbuf[i]) > + capabilities_masked = true; > + > capbuf[i] &= retbuf[i]; > + } > > - if (retbuf[OSC_QUERY_DWORD] & OSC_ERROR_MASK) { > + errors = retbuf[OSC_QUERY_DWORD] & OSC_ERROR_MASK; > + if (errors) { > /* > - * Complain about the unexpected errors and print diagnostic > - * information related to them. > + * Some firmware sets OSC_CAPABILITIES_MASK_ERROR without clearing > + * any requested capability. Only complain if another error is > + * present or a capability was actually masked. Can you add a clarification here on what we think the firmware is indicating when it does this. > */ > - acpi_handle_err(handle, "_OSC: errors while processing control request\n"); > - acpi_handle_err(handle, "_OSC: some features may be missing\n"); > + if (errors != OSC_CAPABILITIES_MASK_ERROR || capabilities_masked) { > + acpi_handle_err(handle, "_OSC: errors while processing control request\n"); Related to questions above, but this print is true on any error return even if perhaps we are sure the next print is not. > + acpi_handle_err(handle, "_OSC: some features may be missing\n"); > + } > acpi_osc_error_check(handle, &guid, rev, &cap, retbuf); > } >