LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] toshiba_acpi: Add a check for TOS_NOT_SUPPORTED in the sci_open function
@ 2015-01-19  2:17 Azael Avalos
  2015-01-27  2:52 ` Azael Avalos
  2015-01-29  6:19 ` Darren Hart
  0 siblings, 2 replies; 3+ messages in thread
From: Azael Avalos @ 2015-01-19  2:17 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

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
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>
---
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.
+		 */
+		return 1;
 	} else if (out[0] == TOS_NOT_PRESENT) {
 		pr_info("Toshiba SCI is not present\n");
 	}
-- 
2.2.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] toshiba_acpi: Add a check for TOS_NOT_SUPPORTED in the sci_open function
  2015-01-19  2:17 [PATCH] toshiba_acpi: Add a check for TOS_NOT_SUPPORTED in the sci_open function Azael Avalos
@ 2015-01-27  2:52 ` Azael Avalos
  2015-01-29  6:19 ` Darren Hart
  1 sibling, 0 replies; 3+ messages in thread
From: Azael Avalos @ 2015-01-27  2:52 UTC (permalink / raw)
  To: Darren Hart, platform-driver-x86, linux-kernel; +Cc: Azael Avalos

Hi Darren,

Any input on this patch?

2015-01-18 19:17 GMT-07:00 Azael Avalos <coproscefalo@gmail.com>:
> 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
> 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>
> ---
> 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.
> +                */
> +               return 1;
>         } else if (out[0] == TOS_NOT_PRESENT) {
>                 pr_info("Toshiba SCI is not present\n");
>         }
> --
> 2.2.1
>



-- 
-- El mundo apesta y vosotros apestais tambien --

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] toshiba_acpi: Add a check for TOS_NOT_SUPPORTED in the sci_open function
  2015-01-19  2:17 [PATCH] toshiba_acpi: Add a check for TOS_NOT_SUPPORTED in the sci_open function Azael Avalos
  2015-01-27  2:52 ` Azael Avalos
@ 2015-01-29  6:19 ` Darren Hart
  1 sibling, 0 replies; 3+ messages in thread
From: Darren Hart @ 2015-01-29  6:19 UTC (permalink / raw)
  To: Azael Avalos; +Cc: platform-driver-x86, linux-kernel

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-01-29  6:19 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-19  2:17 [PATCH] toshiba_acpi: Add a check for TOS_NOT_SUPPORTED in the sci_open function Azael Avalos
2015-01-27  2:52 ` Azael Avalos
2015-01-29  6:19 ` Darren Hart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).