Messages in this thread | | | Date | Wed, 28 Jan 2015 22:19:07 -0800 | From | Darren Hart <> | Subject | Re: [PATCH] toshiba_acpi: Add a check for TOS_NOT_SUPPORTED in the sci_open function |
| |
On Sun, Jan 18, 2015 at 07:17:12PM -0700, Azael Avalos wrote: > This was "toshiba_acpi: Change sci_open function return value" > > Some Toshiba laptops have "poorly implemented" SCI calls on their > BIOSes and are not checking for sci_{open, close} calls, therefore, > the sci_open function is failing and making some of the supported > features unavailable (kbd backlight, touchpad, illumination, etc.). > > This patch checks wheter we receive TOS_NOT_SUPPORTED and returns
^ whether
(checkpatch.pl would catch this)
Since I'm late in review, this one's on me ;-)
> 1, making the supported features work on such laptops. > > In the case that some laptops really do not support the SCI, all the > SCI dependent functions check for TOS_NOT_SUPPORTED, and thus, not > registering support for the queried feature. > > Signed-off-by: Azael Avalos <coproscefalo@gmail.com>
Queued for 3.20.
> --- > Darren, > > Hopefully this new patch eases some of the concerns of the previous > patch, and this time I added a comment block explaining a bit, but of > course, let me know if there is something else that needs change. > > Cheers > Azael > > drivers/platform/x86/toshiba_acpi.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c > index fc34a71..899ead6b 100644 > --- a/drivers/platform/x86/toshiba_acpi.c > +++ b/drivers/platform/x86/toshiba_acpi.c > @@ -389,6 +389,19 @@ static int sci_open(struct toshiba_acpi_dev *dev) > } else if (out[0] == TOS_ALREADY_OPEN) { > pr_info("Toshiba SCI already opened\n"); > return 1; > + } else if (out[0] == TOS_NOT_SUPPORTED) { > + /* Some BIOSes do not have the SCI open/close functions > + * implemented and return 0x8000 (Not Supported), failing to > + * register some supported features. > + * > + * Simply return 1 if we hit those affected laptops to make the > + * supported features work. > + * > + * In the case that some laptops really do not support the SCI, > + * all the SCI dependent functions check for TOS_NOT_SUPPORTED, > + * and thus, not registering support for the queried feature. > + */
This is great by the way, when things are just weird/broken, this is when we need really clear comments.
> + return 1; > } else if (out[0] == TOS_NOT_PRESENT) { > pr_info("Toshiba SCI is not present\n"); > }
Thanks,
-- Darren Hart Intel Open Source Technology Center
| |