Platform: Samsung Q10 backlight driver
PATCH 1/1 Platform: Samsung Q10 backlight driver
This adds backlight control on the Samsung Q10 laptop, which does not support the SABI interface. Also tested successfully on the Dell Latitude X200.
--- drivers/platform/x86/Kconfig | 8 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/samsung-q10.c | 152 ++++++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 0 deletions(-) create mode 100644 drivers/platform/x86/samsung-q10.c
The Samsung Q10 is an old model, which does not support the SABI interface, so the samsung-laptop driver does not work. Backlight control is not exposed via ACPI either. This driver works in the same way as the SMI handler triggered by pressing the brightness function keys. Please let me know any feedback towards getting this driver included.
drivers/platform/x86/Kconfig +8 -0
@@ -639,4 +639,12 @@ config XO1_RFKILL Support for enabling/disabling the WLAN interface on the OLPC XO-1 laptop. +config SAMSUNG_Q10 + tristate "Samsung Q10 Extras" + select BACKLIGHT_CLASS_DEVICE + default n + ---help--- + This driver provides support for backlight control on Samsung Q10 + and some other laptops, including Dell Latitude X200. + endif # X86_PLATFORM_DEVICES
drivers/platform/x86/Makefile +1 -0
@@ -33,3 +33,4 @@ obj-$(CONFIG_INTEL_IPS) += intel_ips.o obj-$(CONFIG_GPIO_INTEL_PMIC) += intel_pmic_gpio.o obj-$(CONFIG_XO1_RFKILL) += xo1-rfkill.o obj-$(CONFIG_IBM_RTL) += ibm_rtl.o +obj-$(CONFIG_SAMSUNG_Q10) += samsung-q10.o
drivers/platform/x86/samsung-q10.c new file +152 -0
152 lines not on this page · raw message
Quoted 80 lines · Frederick van der Wyck
This adds backlight control on the Samsung Q10 laptop, which does not support the SABI interface. Also tested successfully on the Dell Latitude X200.
Signed-off-by Frederick van der Wyck <[email protected]>--- drivers/platform/x86/Kconfig | 8 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/samsung-q10.c | 152 ++++++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 0 deletions(-) create mode 100644 drivers/platform/x86/samsung-q10.cThe Samsung Q10 is an old model, which does not support the SABI interface, so the samsung-laptop driver does not work. Backlight control is not exposed via ACPI either. This driver works in the same way as the SMI handler triggered by pressing the brightness function keys. Please let me know any feedback towards getting this driver included.
drivers/platform/x86/Kconfig +8
-0@@ -639,4 +639,12 @@ config XO1_RFKILL Support for enabling/disabling the WLAN interface on the OLPC XO-1 laptop. +config SAMSUNG_Q10 + tristate "Samsung Q10 Extras" + select BACKLIGHT_CLASS_DEVICE + default n + ---help--- + This driver provides support for backlight control on Samsung Q10 + and some other laptops, including Dell Latitude X200. + endif # X86_PLATFORM_DEVICESdrivers/platform/x86/Makefile +1
-0@@ -33,3 +33,4 @@ obj-$(CONFIG_INTEL_IPS) += intel_ips.o obj-$(CONFIG_GPIO_INTEL_PMIC) += intel_pmic_gpio.o obj-$(CONFIG_XO1_RFKILL) += xo1-rfkill.o obj-$(CONFIG_IBM_RTL) += ibm_rtl.o +obj-$(CONFIG_SAMSUNG_Q10) += samsung-q10.odrivers/platform/x86/samsung-q10.c new file +31
-0@@ -0,0 +1,152 @@ +/* +* Driver for Samsung Q10: controls the backlight +* +* Also works on Dell Latitude X200 +* +* Copyright (c) 2011 Frederick van der Wyck <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +* +*/ + +#include <linux/module.h> +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/platform_device.h> +#include <linux/backlight.h> +#include <linux/io.h> + +#define SAMSUNGQ10_BL_MAX_INTENSITY 255 +#define SAMSUNGQ10_BL_DEFAULT_INTENSITY 185 + +static int samsungq10_suspended; + +static void samsungq10_bl_send_intensity (struct backlight_device *bd) +{ +int brightness = bd->props.brightness; + +if (!samsungq10_suspended) { +while (inb (0x64 )&2 )
drivers/platform/x86/samsung-q10.c line 41
Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10
-0+ ; +outb_p (0xbe ,0x64 ); +while (inb (0x64 )&2 ) + ; +outb_p (0x89 ,0x60 ); +while (inb (0x64 )&2 ) + ; +outb_p (0x91 ,0x60 ); +while (inb (0x64 )&2 ) + ;
Potentially endless loops are not good, please timeout if you don't get the proper value over a time.
Quoted 38 lines
drivers/platform/x86/samsung-q10.c new file +38
-0+outb_p ((unsigned char )brightness,0x60 ); + }; +} + +#ifdef CONFIG_PM +static int samsungq10_suspend (struct platform_device *pdev,pm_message_t state) +{ + samsungq10_suspended =1 ; +return 0 ; +} + +static int samsungq10_resume (struct platform_device *pdev) +{ +struct backlight_device *bd =platform_get_drvdata (pdev); + samsungq10_suspended =0 ; +samsungq10_bl_send_intensity (bd); +return 0 ; +} +#else +#define samsungq10_suspend NULL +#define samsungq10_resume NULL +#endif + +static int samsungq10_bl_set_intensity (struct backlight_device *bd) +{ +samsungq10_bl_send_intensity (bd); +return 0 ; +} + +static int samsungq10_bl_get_intensity (struct backlight_device *bd) +{ +return 0 ; +} + +static const struct backlight_ops samsungq10_bl_ops = { + .get_brightness = samsungq10_bl_get_intensity, + .update_status = samsungq10_bl_set_intensity, +};
drivers/platform/x86/samsung-q10.c line 89
Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10
-0+ +static int __devinit samsungq10_probe (struct platform_device *pdev) +{ +struct backlight_properties props; +struct backlight_device *bd; + +memset (&props,0 ,sizeof (struct backlight_properties )); + props.max_brightness =SAMSUNGQ10_BL_MAX_INTENSITY ; + bd =backlight_device_register ("samsungq10bl" , &pdev->dev,NULL , + &samsungq10_bl_ops, &props);
Why the name "samsungq10bl"? Why not just "samsung"?
Quoted 20 lines
drivers/platform/x86/samsung-q10.c new file +20
-0+if (IS_ERR (bd)) +return PTR_ERR (bd); + +platform_set_drvdata (pdev, bd); + + bd->props.brightness =SAMSUNGQ10_BL_DEFAULT_INTENSITY ; +samsungq10_bl_send_intensity (bd); + +return 0 ; +} + +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_send_intensity (bd); + +backlight_device_unregister (bd); +
drivers/platform/x86/samsung-q10.c line 119
Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10
-0+return 0 ; +} + +static struct platform_driver samsungq10_driver = { + .probe = samsungq10_probe, + .remove = samsungq10_remove, + .suspend = samsungq10_suspend, + .resume = samsungq10_resume, + .driver = { + .name ="samsungq10" ,
Same with the driver name here.
Quoted 23 lines
drivers/platform/x86/samsung-q10.c new file +23
-0+ }, +}; + +static struct platform_device *samsungq10_device; + +static int __init samsungq10_init (void ) +{ +int ret; + + ret =platform_driver_register (&samsungq10_driver); +if (ret) +return ret; + samsungq10_device =platform_device_register_simple ("samsungq10" ,-1 , +NULL ,0 ); +if (IS_ERR (samsungq10_device)) { +platform_driver_unregister (&samsungq10_driver); +return PTR_ERR (samsungq10_device); + } +return 0 ; +} + +static void __exit samsungq10_exit(void) +{
drivers/platform/x86/samsung-q10.c line 152
Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10
-0+platform_device_unregister (samsungq10_device); +platform_driver_unregister (&samsungq10_driver); +} + +module_init (samsungq10_init); +module_exit (samsungq10_exit); + +MODULE_AUTHOR ("Frederick van der Wyck <[email protected]>" ); +MODULE_DESCRIPTION ("Samsung Q10 Driver" ); +MODULE_LICENSE ("GPL" );
No DMI matching to auto-load the driver and also fail to load on any non-supported hardware?
thanks,
greg k-h
Quoted 13 lines · Greg KH
Quoted 12 lines · Frederick van der Wyck
drivers/platform/x86/samsung-q10.c new file +12
-0+ +static void samsungq10_bl_send_intensity (struct backlight_device *bd) +{ +int brightness = bd->props.brightness; + +if (!samsungq10_suspended) { +while (inb (0x64 )&2 ) + ; +outb_p (0xbe ,0x64 ); +while (inb (0x64 )&2 ) + ; +outb_p (0x89 ,0x60 );
Quoted 8 lines
drivers/platform/x86/samsung-q10.c line 41
Quoted 5 lines
drivers/platform/x86/samsung-q10.c new file +5
-0+while (inb (0x64 )&2 ) + ; +outb_p (0x91 ,0x60 ); +while (inb (0x64 )&2 ) + ;Potentially endless loops are not good, please timeout if you don't get the proper value over a time.
Double not good since it is i8042 registers. Use i8042_lock_chip()/i8042_command()/i8042_unlock_chip().
Signature · 1 line
Dmitry
Hi Frederick,
Quoted 22 lines · Frederick van der Wyck
This adds backlight control on the Samsung Q10 laptop, which does not support the SABI interface. Also tested successfully on the Dell Latitude X200.
Signed-off-by Frederick van der Wyck <[email protected]>--- drivers/platform/x86/Kconfig | 8 ++ drivers/platform/x86/Makefile | 1 + drivers/platform/x86/samsung-q10.c | 152 ++++++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 0 deletions(-) create mode 100644 drivers/platform/x86/samsung-q10.cThe Samsung Q10 is an old model, which does not support the SABI interface, so the samsung-laptop driver does not work. Backlight control is not exposed via ACPI either. This driver works in the same way as the SMI handler triggered by pressing the brightness function keys. Please let me know any feedback towards getting this driver included.
drivers/platform/x86/Kconfig +0
-0
drivers/platform/x86/Kconfig config XO1_RFKILL · line 645
Quoted 7 lines
drivers/platform/x86/Kconfig +4
-0@@ -639,4 +639,12 @@ config XO1_RFKILL Support for enabling/disabling the WLAN interface on the OLPC XO-1 laptop. +config SAMSUNG_Q10 + tristate "Samsung Q10 Extras" + select BACKLIGHT_CLASS_DEVICE + default n
You do not need to specify "default n"
Quoted 34 lines
drivers/platform/x86/Kconfig +4
-0+ ---help--- + This driver provides support for backlight control on Samsung Q10 + and some other laptops, including Dell Latitude X200. + endif # X86_PLATFORM_DEVICESdrivers/platform/x86/Makefile +1
-0@@ -33,3 +33,4 @@ obj-$(CONFIG_INTEL_IPS) += intel_ips.o obj-$(CONFIG_GPIO_INTEL_PMIC) += intel_pmic_gpio.o obj-$(CONFIG_XO1_RFKILL) += xo1-rfkill.o obj-$(CONFIG_IBM_RTL) += ibm_rtl.o +obj-$(CONFIG_SAMSUNG_Q10) += samsung-q10.odrivers/platform/x86/samsung-q10.c new file +14
-0@@ -0,0 +1,152 @@ +/* +* Driver for Samsung Q10: controls the backlight +* +* Also works on Dell Latitude X200 +* +* Copyright (c) 2011 Frederick van der Wyck <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +* +*/ + +#include <linux/module.h>
drivers/platform/x86/samsung-q10.c line 24
Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10
-0+#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/platform_device.h> +#include <linux/backlight.h> +#include <linux/io.h> + +#define SAMSUNGQ10_BL_MAX_INTENSITY 255 +#define SAMSUNGQ10_BL_DEFAULT_INTENSITY 185 + +static int samsungq10_suspended;
bool?
Quoted 8 lines
drivers/platform/x86/samsung-q10.c new file +8
-0+ +static void samsungq10_bl_send_intensity (struct backlight_device *bd) +{ +int brightness = bd->props.brightness; + +if (!samsungq10_suspended) { +while (inb (0x64 )&2 ) + ;
drivers/platform/x86/samsung-q10.c line 42
Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10
-0+outb_p (0xbe ,0x64 ); +while (inb (0x64 )&2 ) + ; +outb_p (0x89 ,0x60 ); +while (inb (0x64 )&2 ) + ; +outb_p (0x91 ,0x60 ); +while (inb (0x64 )&2 ) + ; +outb_p ((unsigned char )brightness,0x60 );
As I mentioned, these registers are owned by i8042, you should not access them directly.
drivers/platform/x86/samsung-q10.c line 43
Quoted 1 line
drivers/platform/x86/samsung-q10.c new file +1
-0+ };
Stray semicolon.
drivers/platform/x86/samsung-q10.c line 49
Quoted 6 lines
drivers/platform/x86/samsung-q10.c new file +6
-0+} + +#ifdef CONFIG_PM +static int samsungq10_suspend (struct platform_device *pdev,pm_message_t state) +{ + samsungq10_suspended =1 ;
= true
drivers/platform/x86/samsung-q10.c line 55
Quoted 6 lines
drivers/platform/x86/samsung-q10.c new file +6
-0+return 0 ; +} + +static int samsungq10_resume (struct platform_device *pdev) +{ +struct backlight_device *bd =platform_get_drvdata (pdev);
Blank line before variable definition and code please.
drivers/platform/x86/samsung-q10.c line 56
Quoted 1 line
drivers/platform/x86/samsung-q10.c new file +1
-0+ samsungq10_suspended =0 ;
= false
drivers/platform/x86/samsung-q10.c line 67
Quoted 11 lines
drivers/platform/x86/samsung-q10.c new file +11
-0+samsungq10_bl_send_intensity (bd); + return 0; +} +#else +#define samsungq10_suspend NULL +#define samsungq10_resume NULL +#endif + +static int samsungq10_bl_set_intensity (struct backlight_device *bd) +{ +samsungq10_bl_send_intensity (bd);
Why do you need this? Just have samsungq10_bl_send_intensity() always return 0 (or result of i8042_command()) and use it directly.
drivers/platform/x86/samsung-q10.c line 72
Quoted 5 lines
drivers/platform/x86/samsung-q10.c new file +5
-0+return 0 ; +} + +static int samsungq10_bl_get_intensity (struct backlight_device *bd) +{
I believe you should either store last brightness and return it or not provide this method.
Quoted 19 lines
drivers/platform/x86/samsung-q10.c new file +19
-0+return 0 ; +} + +static const struct backlight_ops samsungq10_bl_ops = { + .get_brightness = samsungq10_bl_get_intensity, + .update_status = samsungq10_bl_set_intensity, +}; + +static int __devinit samsungq10_probe (struct platform_device *pdev) +{ +struct backlight_properties props; +struct backlight_device *bd; + +memset (&props,0 ,sizeof (struct backlight_properties )); + props.max_brightness =SAMSUNGQ10_BL_MAX_INTENSITY ; + bd =backlight_device_register ("samsungq10bl" , &pdev->dev,NULL , + &samsungq10_bl_ops, &props); +if (IS_ERR (bd)) +return PTR_ERR (bd);
drivers/platform/x86/samsung-q10.c line 101
Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10
-0+ +platform_set_drvdata (pdev, bd); + + bd->props.brightness =SAMSUNGQ10_BL_DEFAULT_INTENSITY ; +samsungq10_bl_send_intensity (bd); + +return 0 ; +} + +static int samsungq10_remove (struct platform_device *pdev)
__devexit
Quoted 4 lines
drivers/platform/x86/samsung-q10.c new file +4
-0+{ +struct backlight_device *bd =platform_get_drvdata (pdev); + + bd->props.brightness =SAMSUNGQ10_BL_DEFAULT_INTENSITY ;
drivers/platform/x86/samsung-q10.c line 115
Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10
-0+samsungq10_bl_send_intensity (bd); + +backlight_device_unregister (bd); + +return 0 ; +} + +static struct platform_driver samsungq10_driver = { + .probe = samsungq10_probe, + .remove = samsungq10_remove,
__devexit_p()
drivers/platform/x86/samsung-q10.c line 117
Quoted 2 lines
drivers/platform/x86/samsung-q10.c new file +2
-0+ .suspend = samsungq10_suspend, + .resume = samsungq10_resume,
Should be converted to dev_pm_ops.
drivers/platform/x86/samsung-q10.c line 119
Quoted 2 lines
drivers/platform/x86/samsung-q10.c new file +2
-0+ .driver = { + .name = "samsungq10",
.owner = THIS_MODULE,
Quoted 8 lines
drivers/platform/x86/samsung-q10.c new file +8
-0+ }, +}; + +static struct platform_device *samsungq10_device; + +static int __init samsungq10_init (void ) +{ +int ret;
drivers/platform/x86/samsung-q10.c line 137
Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10
-0+ + ret =platform_driver_register (&samsungq10_driver); +if (ret) +return ret; + samsungq10_device =platform_device_register_simple ("samsungq10" ,-1 , +NULL ,0 ); +if (IS_ERR (samsungq10_device)) { +platform_driver_unregister (&samsungq10_driver); +return PTR_ERR (samsungq10_device); + }
Consider using platform_create_bundle().
Quoted 5 lines
drivers/platform/x86/samsung-q10.c new file +5
-0+return 0 ; +} + +static void __exit samsungq10_exit(void) +{
drivers/platform/x86/samsung-q10.c line 152
Quoted 10 lines
drivers/platform/x86/samsung-q10.c new file +10
-0+platform_device_unregister (samsungq10_device); +platform_driver_unregister (&samsungq10_driver); +} + +module_init (samsungq10_init); +module_exit (samsungq10_exit); + +MODULE_AUTHOR ("Frederick van der Wyck <[email protected]>" ); +MODULE_DESCRIPTION ("Samsung Q10 Driver" ); +MODULE_LICENSE ("GPL" );
Thanks.
Signature · 1 line
Dmitry
Greg, Dmitry - thank you both for the detailed feedback. I'll make the changes you suggest and submit a new version.