Re: [PATCH] platform samsung-q10: use ACPI instead of direct EC calls

2013-09-03 Thread Matthew Garrett 
Applied, thanks.
-- 
Matthew Garrett 
--
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] platform samsung-q10: use ACPI instead of direct EC calls

2013-09-03 Thread Matthew Garrett 
Applied, thanks.
-- 
Matthew Garrett matthew.garr...@nebula.com
--
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] platform samsung-q10: use ACPI instead of direct EC calls

2013-07-13 Thread Rafael J. Wysocki 
On Monday, July 08, 2013 10:34:56 PM Frederick van der Wyck wrote:
> 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).

I think we can make this change and if it breaks things for anyone, we'll
revert it.

Thanks,
Rafael


> Signed-off-by: Frederick van der Wyck 
> ---
>  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 
>  #include 
>  #include 
> -#include 
>  #include 
> +#include 
>  
> -#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

Re: [PATCH] platform samsung-q10: use ACPI instead of direct EC calls

2013-07-13 Thread Rafael J. Wysocki 
On Monday, July 08, 2013 10:34:56 PM Frederick van der Wyck wrote:
 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).

I think we can make this change and if it breaks things for anyone, we'll
revert it.

Thanks,
Rafael


 Signed-off-by: Frederick van der Wyck fvanderw...@gmail.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);
  
 -

Re: [PATCH] platform samsung-q10: use ACPI instead of direct EC calls

2013-07-08 Thread Frederick van der Wyck 
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 
---
 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 
 #include 
 #include 
-#include 
 #include 
+#include 
 
-#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,

Re: [PATCH] platform samsung-q10: use ACPI instead of direct EC calls

2013-07-08 Thread Matthew Garrett 
On Wed, 2013-07-03 at 22:27 +0100, Frederick van der Wyck wrote:

> +#define EC_HID "PNP0C09"

This is probably wrong - you should be able to just use first_ec
directly rather than probing yourself.
> + 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. 

-- 
Matthew Garrett 

--
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] platform samsung-q10: use ACPI instead of direct EC calls

2013-07-08 Thread Matthew Garrett 
On Wed, 2013-07-03 at 22:27 +0100, Frederick van der Wyck wrote:

 +#define EC_HID PNP0C09

This is probably wrong - you should be able to just use first_ec
directly rather than probing yourself.
 + 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. 

-- 
Matthew Garrett mj...@srcf.ucam.org

--
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] platform samsung-q10: use ACPI instead of direct EC calls

2013-07-08 Thread Frederick van der Wyck 
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 fvanderw...@gmail.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 = {

Re: [PATCH] platform samsung-q10: use ACPI instead of direct EC calls

2013-07-03 Thread Rafael J. Wysocki 
On Wednesday, July 03, 2013 10:09:19 PM Frederick van der Wyck wrote:
> This patch changes the Samsung Q10 backlight driver to use ACPI methods (the 
> same ones as triggered by the brightness up/down function keys) instead of 
> direct EC calls. The advantage is that the brightness setting is not lost on 
> shutdown.
> 
> Signed-off-by: Frederick van der Wyck 

Any patches with "ACPI" in the subject (or in the changelog for that matter)
should be CCed to linux-a...@vger.kernel.org.

Thanks,
Rafael


> ---
>  drivers/platform/x86/Kconfig   |2 +-
>  drivers/platform/x86/samsung-q10.c |   77 
> 
>  2 files changed, 35 insertions(+), 44 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..e7caaa3 100644
> --- a/drivers/platform/x86/samsung-q10.c
> +++ b/drivers/platform/x86/samsung-q10.c
> @@ -14,16 +14,14 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
> +#include 
>  
> -#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 }
> +#define EC_HID "PNP0C09"
>  
> -static int samsungq10_bl_brightness;
> +static acpi_handle ec_handle;
>  
>  static bool force;
>  module_param(force, bool, 0);
> @@ -33,21 +31,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 +58,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 +74,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 +82,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 +91,6 @@ static struct platform_driver samsungq10_driver = {
>   .driver = {
>   .name   = KBUILD_MODNAME,
>   .owner  = THIS_MODULE,
> - .pm = _pm_ops,
>   },
>   .probe  = samsungq10_probe,
>   .remove = samsungq10_remove,
> @@ -167,11 +141,28 @@ static struct dmi_system_id __initdata 
> samsungq10_dmi_table[] = {
>  };
>  MODULE_DEVICE_TABLE(dmi, samsungq10_dmi_table);
>  
> +static acpi_status __init acpi_handle_locate_callback(acpi_handle handle,
> + u32 level, void *context,

Re: [PATCH] platform samsung-q10: use ACPI instead of direct EC calls

2013-07-03 Thread Rafael J. Wysocki 
On Wednesday, July 03, 2013 10:09:19 PM Frederick van der Wyck wrote:
 This patch changes the Samsung Q10 backlight driver to use ACPI methods (the 
 same ones as triggered by the brightness up/down function keys) instead of 
 direct EC calls. The advantage is that the brightness setting is not lost on 
 shutdown.
 
 Signed-off-by: Frederick van der Wyck fvanderw...@gmail.com

Any patches with ACPI in the subject (or in the changelog for that matter)
should be CCed to linux-a...@vger.kernel.org.

Thanks,
Rafael


 ---
  drivers/platform/x86/Kconfig   |2 +-
  drivers/platform/x86/samsung-q10.c |   77 
 
  2 files changed, 35 insertions(+), 44 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..e7caaa3 100644
 --- a/drivers/platform/x86/samsung-q10.c
 +++ b/drivers/platform/x86/samsung-q10.c
 @@ -14,16 +14,14 @@
  #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 }
 +#define EC_HID PNP0C09
  
 -static int samsungq10_bl_brightness;
 +static acpi_handle ec_handle;
  
  static bool force;
  module_param(force, bool, 0);
 @@ -33,21 +31,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 +58,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 +74,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 +82,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 +91,6 @@ static struct platform_driver samsungq10_driver = {
   .driver = {
   .name   = KBUILD_MODNAME,
   .owner  = THIS_MODULE,
 - .pm = samsungq10_pm_ops,
   },
   .probe  = samsungq10_probe,
   .remove = samsungq10_remove,
 @@ -167,11 +141,28 @@ static struct dmi_system_id __initdata 
 samsungq10_dmi_table[] = {
  };
  MODULE_DEVICE_TABLE(dmi, samsungq10_dmi_table);
  
 +static acpi_status __init acpi_handle_locate_callback(acpi_handle handle,
 + u32 level, void *context, void **return_value)
 +{
 + *(acpi_handle