Re: [PATCH 2/5] toshiba_acpi: Fix illumination not available on certain models
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 :
> > 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
> >> ---
> >> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] toshiba_acpi: Fix illumination not available on certain models
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 dvh...@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 coproscef...@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] toshiba_acpi: Fix illumination not available on certain models
Hi there
2014-09-05 20:35 GMT-06:00 Darren Hart :
> 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
>> ---
>> 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.
>
> --
> Darren Hart
> Intel Open Source Technology Center
--
-- El mundo apesta y vosotros apestais tambien --
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] toshiba_acpi: Fix illumination not available on certain models
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
> ---
> 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?
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?
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] toshiba_acpi: Fix illumination not available on certain models
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 coproscef...@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?
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?
--
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/5] toshiba_acpi: Fix illumination not available on certain models
Hi there
2014-09-05 20:35 GMT-06:00 Darren Hart dvh...@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 coproscef...@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.
--
Darren Hart
Intel Open Source Technology Center
--
-- El mundo apesta y vosotros apestais tambien --
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
