Re: [PATCH 2/5] toshiba_acpi: Fix illumination not available on certain models

[Darren Hart <dvhart@infradead.org>]

On Fri, Sep 05, 2014 at 10:49:09PM -0600, Azael Avalos wrote:
> Hi there
> 
> 2014-09-05 20:35 GMT-06:00 Darren Hart <dvhart@infradead.org>:
> > On Fri, Sep 05, 2014 at 11:14:04AM -0600, Azael Avalos wrote:
> >> Some Toshiba models with illumination support set a different
> >> value on the returned codes, thus not allowing the illumination
> >> LED to be registered, where it should be.
> >>
> >> This patch removes a check from toshiba_illumination_available
> >> function to allow such models to register the illumination LED.
> >>
> >> Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
> >> ---
> >>  drivers/platform/x86/toshiba_acpi.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
> >> index a149bc6..4803e7b 100644
> >> --- a/drivers/platform/x86/toshiba_acpi.c
> >> +++ b/drivers/platform/x86/toshiba_acpi.c
> >> @@ -436,7 +436,7 @@ static int toshiba_illumination_available(struct toshiba_acpi_dev *dev)
> >>       if (ACPI_FAILURE(status) || out[0] == HCI_FAILURE) {
> >>               pr_err("ACPI call to query Illumination support failed\n");
> >>               return 0;
> >> -     } else if (out[0] == HCI_NOT_SUPPORTED || out[1] != 1) {
> >> +     } else if (out[0] == HCI_NOT_SUPPORTED) {
> >
> > OK, but by eliminating the check, supposedly certain models which do not support
> > illumination but do not report it via out[0], but instead via out[1], will now
> > attempt to use illumination - correct?
> 
> Oh no, the main check is out[0], which either hold success if the
> feature is supported
> or an HCI/SCI error otherwise.
> 
> >
> > The end result being user calls to an ACPI function which at best doesn't exist
> > and at worst.... does, but does something entirely different.
> >
> > I admit the potential for a problem is slight, but is it possible to check
> > something explicit for support on the newer models rather than removing an
> > existing check?
> 
> Our only resource right now is the DSDT and actual hardware to test,
> as those calls
> are not documented anywhere, and everytime the vendor decides to
> change something,
> we're on the loose end.
> 
> All the DSDTs that I previously had all set out[1] to one, so I was
> using that as an
> extra check to make sure we had illumination support, but now, recent models
> set out[1] to zero, and those models, which do happen to have illumination
> support (Qosmio X75 for example) were failing to register the LED.

OK, I've been warned about taking non-obvious changes here. However, as you were
the original author here and are effectively telling me you want to revert the
out[1] check as it breaks hardware, I'm queueing this patch.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center