| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20130708213456.GA5441@lat.lan>
Date: Mon, 8 Jul 2013 22:34:56 +0100
From: Frederick van der Wyck <fvanderwyck@...il.com>
To: Matthew Garrett <mjg59@...f.ucam.org>
Cc: platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org
Subject: Re: [PATCH] platform samsung-q10: use ACPI instead of direct EC
calls
On Mon, Jul 08, 2013 at 11:36:54AM -0400, Matthew Garrett wrote:
> you should be able to just use first_ec directly rather than
> probing yourself.
Thanks, I've made that change below.
> > + for (i = 0; i < SAMSUNGQ10_BL_MAX_INTENSITY; i++) {
> > + status = acpi_evaluate_object(ec_handle, "_Q63", NULL, NULL);
>
> The potential problem here is that there's no guarantee that these event
> numbers are stable, and a firmware upgrade could change them. Of course,
> that's also true of the EC registers, but we haven't had anyone complain
> about the driver suddenly breaking so I'm not hugely enthusiastic about
> replacing one fragile but seemingly working method with a fragile but
> unproven one.
That's true. On the other hand, imitating the action of the brightness keys at
this higher level seems somewhat cleaner and has the advantage that the
brightness setting is preserved on reboot (as the smi handler stores it in
nvram). I did test that 63 and 64 are the appropriate event numbers on each of
the models in the dmi table (although I only tested one firmware version per
model).
Signed-off-by: Frederick van der Wyck <fvanderwyck@...il.com>
---
drivers/platform/x86/Kconfig | 2 +-
drivers/platform/x86/samsung-q10.c | 65 +++++++++++------------------------
2 files changed, 22 insertions(+), 45 deletions(-)
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 8577261..52aed6d 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -762,7 +762,7 @@ config INTEL_OAKTRAIL
config SAMSUNG_Q10
tristate "Samsung Q10 Extras"
- depends on SERIO_I8042
+ depends on ACPI
select BACKLIGHT_CLASS_DEVICE
---help---
This driver provides support for backlight control on Samsung Q10
diff --git a/drivers/platform/x86/samsung-q10.c b/drivers/platform/x86/samsung-q10.c
index 1a90b62..9d6609d 100644
--- a/drivers/platform/x86/samsung-q10.c
+++ b/drivers/platform/x86/samsung-q10.c
@@ -14,16 +14,12 @@
#include <linux/init.h>
#include <linux/platform_device.h>
#include <linux/backlight.h>
-#include <linux/i8042.h>
#include <linux/dmi.h>
+#include <acpi/acpi_drivers.h>
-#define SAMSUNGQ10_BL_MAX_INTENSITY 255
-#define SAMSUNGQ10_BL_DEFAULT_INTENSITY 185
+#define SAMSUNGQ10_BL_MAX_INTENSITY 7
-#define SAMSUNGQ10_BL_8042_CMD 0xbe
-#define SAMSUNGQ10_BL_8042_DATA { 0x89, 0x91 }
-
-static int samsungq10_bl_brightness;
+static acpi_handle ec_handle;
static bool force;
module_param(force, bool, 0);
@@ -33,21 +29,26 @@ MODULE_PARM_DESC(force,
static int samsungq10_bl_set_intensity(struct backlight_device *bd)
{
- int brightness = bd->props.brightness;
- unsigned char c[3] = SAMSUNGQ10_BL_8042_DATA;
+ acpi_status status;
+ int i;
- c[2] = (unsigned char)brightness;
- i8042_lock_chip();
- i8042_command(c, (0x30 << 8) | SAMSUNGQ10_BL_8042_CMD);
- i8042_unlock_chip();
- samsungq10_bl_brightness = brightness;
+ for (i = 0; i < SAMSUNGQ10_BL_MAX_INTENSITY; i++) {
+ status = acpi_evaluate_object(ec_handle, "_Q63", NULL, NULL);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ }
+ for (i = 0; i < bd->props.brightness; i++) {
+ status = acpi_evaluate_object(ec_handle, "_Q64", NULL, NULL);
+ if (ACPI_FAILURE(status))
+ return -EIO;
+ }
return 0;
}
static int samsungq10_bl_get_intensity(struct backlight_device *bd)
{
- return samsungq10_bl_brightness;
+ return bd->props.brightness;
}
static const struct backlight_ops samsungq10_bl_ops = {
@@ -55,28 +56,6 @@ static const struct backlight_ops samsungq10_bl_ops = {
.update_status = samsungq10_bl_set_intensity,
};
-#ifdef CONFIG_PM_SLEEP
-static int samsungq10_suspend(struct device *dev)
-{
- return 0;
-}
-
-static int samsungq10_resume(struct device *dev)
-{
-
- struct backlight_device *bd = dev_get_drvdata(dev);
-
- samsungq10_bl_set_intensity(bd);
- return 0;
-}
-#else
-#define samsungq10_suspend NULL
-#define samsungq10_resume NULL
-#endif
-
-static SIMPLE_DEV_PM_OPS(samsungq10_pm_ops,
- samsungq10_suspend, samsungq10_resume);
-
static int samsungq10_probe(struct platform_device *pdev)
{
@@ -93,9 +72,6 @@ static int samsungq10_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, bd);
- bd->props.brightness = SAMSUNGQ10_BL_DEFAULT_INTENSITY;
- samsungq10_bl_set_intensity(bd);
-
return 0;
}
@@ -104,9 +80,6 @@ static int samsungq10_remove(struct platform_device *pdev)
struct backlight_device *bd = platform_get_drvdata(pdev);
- bd->props.brightness = SAMSUNGQ10_BL_DEFAULT_INTENSITY;
- samsungq10_bl_set_intensity(bd);
-
backlight_device_unregister(bd);
return 0;
@@ -116,7 +89,6 @@ static struct platform_driver samsungq10_driver = {
.driver = {
.name = KBUILD_MODNAME,
.owner = THIS_MODULE,
- .pm = &samsungq10_pm_ops,
},
.probe = samsungq10_probe,
.remove = samsungq10_remove,
@@ -172,6 +144,11 @@ static int __init samsungq10_init(void)
if (!force && !dmi_check_system(samsungq10_dmi_table))
return -ENODEV;
+ ec_handle = ec_get_handle();
+
+ if (!ec_handle)
+ return -ENODEV;
+
samsungq10_device = platform_create_bundle(&samsungq10_driver,
samsungq10_probe,
NULL, 0, NULL, 0);
--
1.7.3.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists